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

v14 --fix rewrites `a 'b' c` to 'a \'b\' c' instead of "a 'b' c" #1380

Open
wmhilton opened this issue Aug 19, 2019 · 4 comments

Comments

@wmhilton
Copy link

commented Aug 19, 2019

What version of this package are you using?
v14.0.0

What operating system, Node.js, and npm version?
node v10.15.3

What happened?
standard --fix rewrote my template strings (containing single quotes) into single quote strings (containing escaped single quotes):

e.g.

-    expect(await fs.exists(`${dir}/a.txt`)).toBe(true, `'a.txt' exists`)
+    expect(await fs.exists(`${dir}/a.txt`)).toBe(true, '\'a.txt\' exists')

What did you expect to happen?
I didn't expect that. Like... I'm a little sad that it rewrote my template strings at all, but short of reverting #838, at worst --fix should rewrite them using double quotes like,

+    expect(await fs.exists(`${dir}/a.txt`)).toBe(true, "'a.txt' exists")

Are you willing to submit a pull request to fix this bug?
Maybe? If someone can point me at what file does the --fix behavior shouldn't be too hard for me to figure out.

@feross

This comment has been minimized.

Copy link
Member

commented Aug 19, 2019

I agree with you that this fix is not ideal. Double quotes would be better. The fix for this would need to go into ESLint in this file: https://github.com/eslint/eslint/blob/master/lib/rules/quotes.js It looks like we'd need to take into account the value of the avoidEscape rule setting whenever converting between quote types in --fix. This seems like a reasonable change.

@wmhilton As a next step, would you like to open this as an issue on ESLint to see what they think of the idea?

@wmhilton

This comment has been minimized.

Copy link
Author

commented Aug 20, 2019

As a next step, would you like to open this as an issue on ESLint to see what they think of the idea?

done

@feross

This comment has been minimized.

Copy link
Member

commented Aug 22, 2019

Cool, let's wait to see how that issue discussion goes: eslint/eslint#12129

@feross

This comment has been minimized.

Copy link
Member

commented Aug 23, 2019

@wmhilton FYI, there's a new issue opened with a proposed solution to this and other problems with the quotes rule: eslint/eslint#12156

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
You can’t perform that action at this time.