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

Enforce curly braces for JSX props with React elements #3184

Closed
neutraali opened this issue Jan 21, 2022 · 8 comments · Fixed by #3191
Closed

Enforce curly braces for JSX props with React elements #3184

neutraali opened this issue Jan 21, 2022 · 8 comments · Fixed by #3191

Comments

@neutraali
Copy link

Hey there! I've dug around for a few days now, whenever I had a spare moment to dedicate to it, but I couldn't find a rule that would enforce curly braces for React elements as JSX props. I've tried using a combo of jsx-no-literals (which was mostly for strings) and curly-brace-presence but it didn't work (using --fix) for React element props. Here's some examples using jsx-curly-brace-presence with [1, "always"] as the value:

Initial input: <ExampleComponent text='Hello' component=<p>World!</p> />

Expected output: <ExampleComponent text={'Hello'} component={<p>World!</p>} />

Actual output: <ExampleComponent text={'Hello'} component=<p>World!</p> />

... The problem with not enclosing the JSX element props with curly braces is that it breaks certain parsers, for example the one esbuild happens to be using.

Libraries:

"babel-eslint": "10.1.0"
"eslint": "6.8.0"
"eslint-config-prettier": "6.10.0"
"eslint-config-recommended": "4.1.0"
"eslint-plugin-flowtype": "4.6.0"
"eslint-plugin-prettier": "3.1.2"
"eslint-plugin-react": "7.18.3"

Environment:

nodejs@16.13.0
npm@6.14.15
@ljharb
Copy link
Member

ljharb commented Jan 21, 2022

It should break every parser - it’s simply invalid jsx to omit the curly braces, and there’s no eslint rules that operate on invalid syntax.

@ljharb
Copy link
Member

ljharb commented Jan 21, 2022

You’re on eslint 6, which is quite old - have you tried eslint 8?

@neutraali
Copy link
Author

Thanks for the tips, @ljharb! Will check it out once I'm back in the saddle.

@ljharb
Copy link
Member

ljharb commented Jan 21, 2022

I'll close this for now, but will be happy to reopen if there's more to discuss.

@ljharb ljharb closed this as completed Jan 21, 2022
@neutraali
Copy link
Author

neutraali commented Jan 27, 2022

So, to recap, the JSX syntax actually does support passing in elements directly - That just isn't documented at all.

Apparently Babel, for example, has allowed this for quite some time now, but I guess that's just developers being developers.

@ljharb
Copy link
Member

ljharb commented Jan 27, 2022

Wow, that’s horrific.

If it parses, we can handle it, but i think it should have its own rule, since nobody should ever be omitting these curly braces.

@ljharb
Copy link
Member

ljharb commented Jan 27, 2022

I decided this is indeed the appropriate rule, and I've added an option in #3191.

@neutraali can you confirm that that PR meets your needs?

ljharb added a commit to ljharb/eslint-plugin-react that referenced this issue Jan 27, 2022
ljharb added a commit to ljharb/eslint-plugin-react that referenced this issue Jan 27, 2022
@neutraali
Copy link
Author

@ljharb Lookin' good! 👍🏻

@ljharb ljharb closed this as completed in ed63c01 Jan 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants