Skip to content
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.

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

Enhancement: [restrict-template-expressions] More permissive type check #8722

Open
4 tasks done
abrahamguo opened this issue Mar 19, 2024 · 6 comments · May be fixed by #8556
Open
4 tasks done

Enhancement: [restrict-template-expressions] More permissive type check #8722

abrahamguo opened this issue Mar 19, 2024 · 6 comments · May be fixed by #8556
Labels
accepting prs Go ahead, send a pull request that resolves this issue enhancement: plugin rule option New rule option for an existing eslint-plugin rule package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin

Comments

@abrahamguo
Copy link
Contributor

abrahamguo commented Mar 19, 2024

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://typescript-eslint.io/rules/restrict-template-expressions/

Description

Related: #6279

I have added the allowArray option to restrict-template-expressions in #8389.

Now, I'd like to discuss other things that may be useful to put in a string. The most common case for me is stringifying URLSearchParams, Date and Buffers, but there could be potential for others as well.
Some questions for discussion:

Fail

declare const maybeParams: URLSearchParams | undefined;

fetch(`https://myapi.com?${maybeParams}`);

Pass

declare const definitelyParams: URLSearchParams;

fetch(`https://myapi.com?${definitelyParams}`);

Additional Info

No response

@abrahamguo abrahamguo 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 maintainers to take a look labels Mar 19, 2024
@bradzacher
Copy link
Member

bradzacher commented Mar 19, 2024

Quick response:

Do we add a new option ignoredTypeNames?

Nope. We do not want loose "type name a string" options.
See #6017

TL;DR - if you configure ignoreTypeNames: ["Date"] how does that differentiate between

declare const x: Date;
`${x}` // will result in '<..date time string here..>'

and

interface Date { prop: string }
declare const x: Date;
`${x}` // will result in '[object Object]'

If we add a general purpose option - it needs to follow the same spec as described in #6017 and implemented in the related PRs in other rules.

@abrahamguo
Copy link
Contributor Author

@bradzacher Does this mean that ignoredTypeNames in no-base-to-string is also on the chopping block to be removed, because of the same issue?

@bradzacher
Copy link
Member

At some point in the future - sure. It's just low on the list of list of priorities given its already implemented and would be a large breaking change for users to change.

@JoshuaKGoldberg JoshuaKGoldberg added evaluating community engagement we're looking for community engagement on this issue to show that this problem is widely important and removed triage Waiting for maintainers to take a look labels Mar 25, 2024
@abrahamguo
Copy link
Contributor Author

abrahamguo commented Mar 27, 2024

What do you guys think about the comments on #6279?

(which, really, I don't think should be closed, since it hasn't really been fixed?)

In that issue, it seems like @bradzacher supports such an option.

@JoshuaKGoldberg
Copy link
Member

I'm personally in favor of adding the standard format of "allow specifically these types" to restrict-template-expressions (#8722 (comment)). Would like to see more input from @typescript-eslint/triage-team before making any decisions.

@JoshuaKGoldberg
Copy link
Member

A 👍 from another team member and a past "I'm ok with adding" from #6279 (comment). Marking as accepting PRs!

@JoshuaKGoldberg JoshuaKGoldberg added accepting prs Go ahead, send a pull request that resolves this issue and removed evaluating community engagement we're looking for community engagement on this issue to show that this problem is widely important labels Jun 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepting prs Go ahead, send a pull request that resolves this issue 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
Development

Successfully merging a pull request may close this issue.

3 participants