10000 Document rationale for allowNumber: false in restrict-template-expressions · Issue #9311 · typescript-eslint/typescript-eslint · GitHub
[go: up one dir, main page]
Skip to content

Document rationale for allowNumber: false in restrict-template-expressions #9311

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
2 tasks done
danvk opened this issue Jun 8, 2024 · 3 comments · Fixed by #9870
Closed
2 tasks done

Document rationale for allowNumber: false in restrict-template-expressions #9311

danvk opened this issue Jun 8, 2024 · 3 comments · Fixed by #9870
Labels
accepting prs Go ahead, send a pull request that resolves this issue documentation Documentation ("docs") that needs adding/updating locked due to age Please open a new issue if you'd like to say more. See https://typescript-eslint.io/contributing.

Comments

@danvk
Copy link
Collaborator
danvk commented Jun 8, 2024

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

Description

The strict-type-checked configuration sets allowNumber: false for the restrict-template-expressions rule. This prohibits code like the following:

const userId = 123;
const url = `/users/${userId}`;

There is nothing wrong with this code. The official MDN docs on template literals contain code just like it. The typescript-eslint docs for this rule offer no rationale of why interpolating numbers into a string in this way is problematic.

Impacted Configurations

  • strict
  • strict-type-checked

Additional Info

I'm totally bought into all the other options that strict enables for restrict-template-expressions (allowAny, allowArray, allowBoolean, allowNullish, allowNever). These seem potentially problematic since they could result in your inadvertently showing debug code to users. But allowNumber? I'm having trouble seeing any rationale for banning that.

My proposal would be to either change the default setting or add a clear explanation of why interpolating numbers into strings is dangerous.

Possibly related:
@danvk danvk added package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin preset config change Proposal for an addition, removal, or general change to a preset config triage Waiting for team members to take a look labels Jun 8, 2024
@Josh-Cena
Copy link
Member
Josh-Cena commented Jun 8, 2024 •
edited
Loading

I think the rationale is not integers but floating point. For example `The result is ${0.1 + 0.2}` is not going to be user friendly. You should probably use toFixed(). The MDN example is fine because we are printing IDs which are known to be integers. As a lint rule we can't make that assumption.

@danvk
Copy link
Collaborator Author
danvk commented Jun 9, 2024

That's a good point @Josh-Cena. If you're working with integers, this rule is pure noise. If you're working with floats, it's a helpful reminder. For the first time, I will say: it would be nice if TypeScript had an int type! :)

So I can see the rationale for this rule, though I think for most code it's going to be quite noisy compared to the other types that are disallowed by restrict-template-expressions. Including a rationale in the docs seems like a clear win, though.

@Josh-Cena
Copy link
Member

Agreed!

@Josh-Cena Josh-Cena added documentation Documentation ("docs") that needs adding/updating accepting prs Go ahead, send a pull request that resolves this issue and removed package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for team members to take a look preset config change Proposal for an addition, removal, or general change to a preset config labels Jun 9, 2024
@Josh-Cena Josh-Cena changed the title Configs: Change allowNumber: false → true for restrict-template-expressions in strict and strict-type-checked configs Document rationale for allowNumber: false in restrict-template-expressions Jun 9, 2024
@JoshuaKGoldberg JoshuaKGoldberg mentioned this issue 7AA8 Aug 23, 2024
Merged
3 tasks
@github-actions github-actions bot added the locked due to age Please open a new issue if you'd like to say more. See https://typescript-eslint.io/contributing. label Sep 1, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
accepting prs Go ahead, send a pull request that resolves this issue documentation Documentation ("docs") that needs adding/updating locked due to age Please open a new issue if you'd like to say more. See https://typescript-eslint.io/contributing.
Projects
None yet
Milestone
No milestone
2 participants
@danvk @Josh-Cena
0