Skip to content

Enhancement: [restrict-template-expressions] allowURLSearchParams option #5323

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
4 tasks done
mmkal opened this issue Jul 7, 2022 · 3 comments
Closed
4 tasks done

Enhancement: [restrict-template-expressions] allowURLSearchParams option #5323

mmkal opened this issue Jul 7, 2022 · 3 comments
Labels
duplicate This issue or pull request already exists enhancement: plugin rule option New rule option for an existing eslint-plugin rule package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin

Comments

@mmkal
Copy link

mmkal commented Jul 7, 2022 •
edited
Loading

Before You File a Proposal Please Confirm You Have Done The Following...

My proposal is suitable for this project

  • I believe my proposal would be useful to the broader TypeScript community (meaning it is not a niche proposal).

Link to the rule's documentation

https://github.com/typescript-eslint/typescript-eslint/blob/v4.28.2/packages/eslint-plugin/docs/rules/restrict-template-expressions.md

Description

Having to do this is a little annoying:

const href = `https://example.com/foo/bar/baz?${new URLSearchParams({ abc: 'xyz' }).toString()}`

When the version of the above without .toString() is always safe and less verbose. URLSearchParams is supported in pretty much every browser, nodejs, and deno.

I read the rationale for closing the broader issue #3538: #3538 (comment) and I don't think it applies here. This is specific to URLSearchParams not any-class-which-implements-toString which is less safe and harder to implement.

Fail

const href = 'https://example.com/foo/bar/baz?${{ abc: 'xyz' }}`

Pass

const href = 'https://example.com/foo/bar/baz?${new URLSearchParams({ abc: 'xyz' })}`

Additional Info

The option should work just like allowNumber, allowBoolean, etc.

I'd be happy to contribute a PR if it'd be accepted.
@mmkal mmkal added enhancement: plugin rule option New rule option for an existing eslint-plugin rule package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for team members to take a look labels Jul 7, 2022
@bradzacher
Copy link
Member

The reasoning in the comment still applies here - just because this type has a toString method doesn't mean it's what you want to be using in all cases.

I'm not sure I understand why there should be a specific exception made for this type over any other type. Why should we add an option for URLSearchParams and not Array, or <insert type name here>?

@bradzacher bradzacher added awaiting response Issues waiting for a reply from the OP or another party and removed triage Waiting for team members to take a look labels Jul 7, 2022
@mmkal
Copy link
Author

mmkal commented Jul 7, 2022 •
edited
Loading

Because URLSearchParams has a toString method which is particularly useful for an extremely ubiquitous, standardised format (URL query strings), unlike Array. The format is familiar to humans everywhere, not even just developers. Anyone who's used Google has come across it. If <insert type name here> is supported in all engines and outputs a similarly ubiquitous and well spec'd format, maybe it should be considered too. I can't think of any examples right now.

@JoshuaKGoldberg JoshuaKGoldberg mentioned this issue Oct 17, 2022
Closed
4 tasks
@JoshuaKGoldberg
Copy link
Member

Because URLSearchParams has a toString method which is particularly useful for an extremely ubiquitous, standardised format (URL query strings), unlike Array.

In that case, the real issue is with TypeScript not adding a mention of that overridden .toString() in its lib.dom.d.ts: microsoft/TypeScript#38347. Dup of #4999. Thanks for mentioning!

@JoshuaKGoldberg JoshuaKGoldberg added duplicate This issue or pull request already exists and removed awaiting response Issues waiting for a reply from the OP or another party labels Oct 17, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 18, 2022
@JoshuaKGoldberg JoshuaKGoldberg closed this as not planned Won't fix, can't repro, duplicate, stale Aug 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
duplicate This issue or pull request already exists enhancement: plugin rule option New rule option for an existing eslint-plugin rule package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@JoshuaKGoldberg @bradzacher @mmkal