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: prefer-string-raw #2048

Closed
fisker opened this issue Mar 10, 2023 · 8 comments · Fixed by #2339
Closed

Rule proposal: prefer-string-raw #2048

fisker opened this issue Mar 10, 2023 · 8 comments · Fixed by #2339

Comments

@fisker
Copy link
Collaborator

fisker commented Mar 10, 2023

Description

To avoid escape \.

Fail

console.log('"\\" found.');
const directory = "C:\\windows\\style\\path";
new RegExp('foo\\.bar')

Pass

console.log(String.raw`"\" found.`);
const directory = String.raw`C:\windows\style\path`;
new RegExp(String.raw`foo\.bar`)
const foo = '\\ `';
// Use `String.raw` will need escape `
const foo = '\\ ${foo}';
// Use `String.raw` will need escape $ or `{`
// Places can't use a tagged template

"A directive\\"

import foo from "./foo\\bar.js";

const foo = {
	'\\': 'a non-computed key'
}

This rule can also check when building a tagged function like this, but it's hard to detect such pattern.

@silverwind
Copy link
Contributor

silverwind commented Mar 30, 2023

Seems useful sometimes. I think the rule should not trigger when the string in question contains other escape sequences besides \\ because using String.raw on that would break those other sequences, for example:

console.log(`"\\" found. \nok`);

@fisker
Copy link
Collaborator Author

fisker commented May 8, 2024

@sindresorhus Thoughts?

@sindresorhus
Copy link
Owner

Accepted

@DavidAnson
Copy link

@fisker @sindresorhus, is this rule compelling enough to have in the “recommended” set? I normally try to abide by plugin defaults, but I’m struggling here. While C#/Python syntax for raw is short, JavaScript syntax is long and ugly. This is personal preference, but the examples in the documentation feel to me like they are worse after the recommended change. Also, if the reader is not familiar with template literals, the JS syntax looks wrong/invalid. According to mdn, raw is the “only built-in template literal tag” which makes me think it won’t be familiar to a lot of people. To be clear, I don’t object to the rule itself, I’m just questioning whether it should be recommended?

@fisker
Copy link
Collaborator Author

fisker commented May 11, 2024

the examples in the documentation feel to me like they are worse after the recommended change

Why is that? Escaped \\ is ugly to me.

@DavidAnson
Copy link

The console.log and RegExp examples are almost twice as long after adding String.raw - and the first/most important parameter to both functions is partially obscured because it's prefixed by String.raw.

@silverwind
Copy link
Contributor

I tired to enable this rule, but ended up disabling it because the only cases where I use backslashes is in RegExp strings like "[^\\/]", and I found the readability reduced because String.raw is not well known and the intend of avoiding to escape a single character is not obvious.

@ElPrudi
Copy link

ElPrudi commented May 21, 2024

Should not be in recommended. This kills performance and VSCode syntax highlighting doesn't know how to handle it:

console.time('before')
for (let i = 0; i < 100000; i++) 'C:\\windows\\style\\path'
console.timeEnd('before') // 1.935 ms
console.time('after')
for (let i = 0; i < 100000; i++) String.raw`C:\windows\style\path`
console.timeEnd('after') // 47.016 ms
// Around: 25x slower

kevinoid added a commit to kevinoid/eslint-config-kevinoid that referenced this issue Jun 17, 2024
Although `String.raw` may be useful in some situations, requiring it is
problematic for several reasons:

- It often makes the combined literal longer.
  (e.g. `"foo\\bar"` (10 chars) vs ``String.raw`foo\bar``` (19 chars))
- It's much less performant.
  sindresorhus/eslint-plugin-unicorn#2048 (comment)
- It can make code harder to read, due to being uncommon.
  I'm used to seeing double backslashes.  When they are missing I
  double-take.
- Literals are inconsistent: Strings with escape sequences can't use
  String.raw and strings without backslahes don't need to use it, so
  string literals vary making adjacent literals inconsistent and harder
  to read:
  ```js
  const strings = [
    "foo/bar",
    "foo\\bar\n",
    String.raw`foo\bar`,
    "foo_bar",
  ];
  ```

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
kevinoid added a commit to kevinoid/eslint-config-kevinoid that referenced this issue Jun 17, 2024
Although `String.raw` may be useful in some situations, requiring it is
problematic for several reasons:

- It often makes the combined literal longer.
  (e.g. `"foo\\bar"` (10 chars) vs ``String.raw`foo\bar``` (19 chars))
- It's much less performant.
  sindresorhus/eslint-plugin-unicorn#2048 (comment)
- It can make code harder to read, due to being uncommon.
  I'm used to seeing double backslashes.  When they are missing I
  double-take.
- Literals are inconsistent: Strings with escape sequences can't use
  String.raw and strings without backslahes don't need to use it, so
  string literals vary making adjacent literals inconsistent and harder
  to read:
  ```js
  const strings = [
    "foo/bar",
    "foo\\bar\n",
    String.raw`foo\bar`,
    "foo_bar",
  ];
  ```

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
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.

5 participants