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

Rule proposal: no-useless-interpolation #1261

Open
karlhorky opened this issue May 11, 2021 · 16 comments
Open

Rule proposal: no-useless-interpolation #1261

karlhorky opened this issue May 11, 2021 · 16 comments

Comments

@karlhorky
Copy link

Hi there! 👋 First of all, thanks again for this plugin, super helpful, the things available in this plugin.

I wanted to propose a new rule to address a pattern that I have seen from a lot of our students at @upleveled - writing overly complex patterns with unnecessary interpolations in template strings (eg. a single string variable, or a string literal, or multiple string literals).

Fail

const withString = `${str}`;
const withStringLiteral = `${'abc'}`;
const withMultipleStringLiterals = `${'abc'}${'def'}`;

Pass

const withString = str;
const withStringLiteral = 'abc';
const withMultipleStringLiterals = 'abcdef';

Implementation ideas

Looking into the ASTExplorer, I guess the simple version of the first failure case below would be:

  • only 1 element in TemplateLiteral.expressions (an Identifier)
  • only 2 elements in TemplateLiteral.quasis, all empty strings

For the literals, I guess no limit to the quasis, but every element in expressions should be a Literal.

@karlhorky
Copy link
Author

Oh, I just realized this may be difficult to implement without variable type information.

What is the position of the project on using type information (eg. TypeScript, JSDoc) in rules?

@karlhorky
Copy link
Author

karlhorky commented May 11, 2021

🤦‍♂️ Ohh I know why this rule seemed so familiar to me, I proposed something similar over at @typescript-eslint/eslint-plugin some time ago:

typescript-eslint/typescript-eslint#2846

If you think this is a better place for this (instead of here) then I will close this.

@karlhorky
Copy link
Author

karlhorky commented May 11, 2021

Ah and background of a similar proposal in the ESLint repo: eslint/eslint#10798

@fisker
Copy link
Collaborator

fisker commented May 11, 2021

Looking into the ASTExplorer, I guess the simple version of the first failure case below would be:

This is not right they are the same, one expression plus two empty quasis.

@fisker
Copy link
Collaborator

fisker commented May 11, 2021

Oh, I just realized this may be difficult to implement without variable type information.

Actually, not really nessary, if it's unknown type, we can assume it's type-casting, const foo = String(bar) still better than template.

@fisker
Copy link
Collaborator

fisker commented May 11, 2021

The rule could be two parts

  1. forbid string literal as expression.
  2. No expression-only template.

Update:

String concatenation should fobid too

`before${ "head" + expression + "tail" }after`

@fisker
Copy link
Collaborator

fisker commented May 11, 2021

I think we can accept it.

@fisker
Copy link
Collaborator

fisker commented May 11, 2021

One note, taged templates should not reported.

@sindresorhus
Copy link
Owner

This is now accepted.

@EvgenyOrekhov
Copy link
Contributor

Note: the `${str}` case is already covered by the disallowTemplateShorthand option of the core no-implicit-coercion rule.

@dimaMachina
Copy link
Contributor

I will work on it!

@fisker
Copy link
Collaborator

fisker commented Dec 22, 2021

I started few month ago, but I can't remember the progress...

https://github.com/fisker/eslint-plugin-unicorn/commits/no-useless-interpolation

@damiangreen
Copy link

is anything happening on this?

@KurtPreston
Copy link

This rule to be implemented by eslint-plugin-lit:
https://github.com/43081j/eslint-plugin-lit/blob/master/docs/rules/no-useless-template-literals.md

@rentalhost
Copy link

Some progress on that? I could includes functions like ${chalk.bold("Hello")} to be just chalk.bold("Hello")?

@EvgenyOrekhov
Copy link
Contributor

Looks like this was implemented in @typescript-eslint/no-useless-template-literals

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants