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

no-unescaped-entities: Add exception for children content #1263

Closed
alexilyaev opened this issue Jun 16, 2017 · 13 comments
Closed

no-unescaped-entities: Add exception for children content #1263

alexilyaev opened this issue Jun 16, 2017 · 13 comments
Labels

Comments

@alexilyaev
Copy link
Contributor

alexilyaev commented Jun 16, 2017

The rule no-unescaped-entities warns on these characters inside JSX:

>
"
'
}

But it looks like Babel handles them pretty well when used as children content:
https://babeljs.io/repl/#?babili=false&evaluate=true&lineWrap=false&presets=es2015%2Creact&targets=&browsers=&builtIns=false&debug=false&code_lz=DwEwlgbgBAxgNgQwM5IHIILYFMC8AiACyzjgHs8A-AKCigEkAXAciSgHNSA7ThKAIyxReHUiCggEATygNRUgIQ0oFKACUwbAgwD8SgL5QkAVwBOgsKzwIA7llIYkWPFWAB6cBGpA&experimental=true&loose=false&spec=false

I've got a bunch of text I copy paste all the time (note I wrote I've, that's what ppl are used to).
I would like to have warnings when I probably did an error, but in children content these characters are ok and refactoring them all the time is painful.

Right now the team just disabled this rule because of it.

Can we add an exception for children content (either an option or built in)?

Related #894

@ljharb
Copy link
Member

ljharb commented Jun 16, 2017

"This rule prevents characters that you may have meant as JSX escape characters from being accidentally injected as a text node in JSX statements." - it's not because those entities are invalid, it's because it's impossible to know from looking at it if you meant it, or made a mistake.

In other words, they're not OK, because the programmer's intention is not clear.

@alexilyaev
Copy link
Contributor Author

alexilyaev commented Jun 18, 2017

I see, more complicated than I thought.

Can we add an exceptions or ignore property?

This way I could for example ignore ', since we only use " for attributes, and ' is too common in our texts.

@ljharb
Copy link
Member

ljharb commented Jun 18, 2017

Why are you using straight quotes in your text? That's typographically invalid - you should only be using “ ” ‘ ’.

@alexilyaev
Copy link
Contributor Author

@ljharb 2 reasons:

  1. Texts I receive from external resources (content creators, product manager, marketing)
  2. I write a ton every day, if I had to press an unfamiliar key combination for every ' and " I would go insane. It's true that for code I could make the exception, but point 1 still stands.

@ljharb
Copy link
Member

ljharb commented Jun 18, 2017

If external resources provided you with text that had spelling and grammar errors, wouldn't you want to fix those? Using straight quotes in text is a typography error.

Re point 2, you can trivially set up your computer to autocorrect straight quotes to curly quotes in non-code contexts.

In general, I think that if you want exceptions to this rule, you'd be better off just disabling it.

@alexilyaev
Copy link
Contributor Author

alexilyaev commented Jun 18, 2017

Just clicked the Terms link at the bottom of this page, are these ' proper quotes or just '?
I see Google Privacy does use proper quotes.

I do agree with you, just trying to cope with reality.
Even if I changed my system, I can't change all my client's systems, too many unrelated people involved.

I would very much want to catch > and } as these often are mistakes.
I'm willing to PR if the option is acceptable, and with some guidance of course.

And thanks for the package in general, I really appreciate the work you guys are doing!
👍💯

@ljharb
Copy link
Member

ljharb commented Jun 19, 2017

Indeed, lots of sites use straight quotes when they shouldn't - linters are for those who want to do the right thing :-)

I'd definitely not want an option that allowed for generic configurability, because many of the characters the rule blocks would be dangerous to allow - but I wouldn't block an option that specifically allowed straight quotes, if other collabs are inclined to add it.

@aij
Copy link

aij commented Oct 25, 2017

programmer's [...] shouldn't [...] I'd [...] wouldn't

@ljharb Which direction are you claiming those ' should be curled?

@ljharb
Copy link
Member

ljharb commented Oct 25, 2017

@aij ' should be curly quotes in all prose contexts where it's not referring to "minutes" (as in, DMS notation).

@mkhazov
Copy link

mkhazov commented Dec 19, 2017

There is a "forbid" option, so we can easily configure the rule:
"react/no-unescaped-entities": [2, { "forbid": [">", "}"] }],

@alexilyaev
Copy link
Contributor Author

@mkhazov That worked!
Suits our needs.

forbid is not documented on the rule doc though.

@ljharb
Copy link
Member

ljharb commented Dec 19, 2017

A PR to improve the docs is always appreciated :-)

alexilyaev added a commit to alexilyaev/eslint-plugin-react that referenced this issue Dec 24, 2017
@alexilyaev
Copy link
Contributor Author

@ljharb PR'ed :-)

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

No branches or pull requests

4 participants