[New] add rule to enforce fragment syntax #1994
Conversation
README.md
Outdated
| @@ -159,6 +159,7 @@ Enable the rules that you would like to use. | |||
| * [react/jsx-one-expression-per-line](docs/rules/jsx-one-expression-per-line.md): Limit to one expression per line in JSX | |||
| * [react/jsx-curly-brace-presence](docs/rules/jsx-curly-brace-presence.md): Enforce curly braces or disallow unnecessary curly braces in JSX | |||
| * [react/jsx-pascal-case](docs/rules/jsx-pascal-case.md): Enforce PascalCase for user-defined JSX components | |||
| * [react/jsx-prefer-fragment-shorthand](docs/rules/jsx-prefer-fragment-shorthand.md): Enforce standard or shorthand syntax for React fragments | |||
There was a problem hiding this comment.
maybe “prefer” shouldn’t be in the rule name, since that’s only one of the settings
There was a problem hiding this comment.
jsx-fragment-syntax of either 'shorthand' or 'standard'? (Also not sure what’s the better default)
There was a problem hiding this comment.
Technically React.Fragment isn’t syntax, it’s api - maybe jsx-fragments with either “syntax” or “element” or something.
I’d say the default should probably be “syntax” even though that requires babel 7.
There was a problem hiding this comment.
Ah, I see what you mean 👍
|
|
||
| ### `always` mode | ||
|
|
||
| This is the default mode. It will enforce the shorthand syntax for React fragments, with one exception. [Keys or attributes are not supported by the shorthand syntax][short_syntax], so the rule will not warn on standard-syntax fragments that use those. |
There was a problem hiding this comment.
What attributes besides “key” are possible here?
There was a problem hiding this comment.
Currently none, but the docs hint at event handlers in the future: https://reactjs.org/docs/fragments.html#keyed-fragments
| return { | ||
| JSXElement(node) { | ||
| const openingEl = node.openingElement; | ||
| if (elementType(openingEl) === `${reactPragma}.${fragmentPragma}`) { |
There was a problem hiding this comment.
What if someone already has <Fragment? I’d expect to warn on that, if it was destructured from React.
There was a problem hiding this comment.
Oh. That’s what I was trying to figure out in #1661, but I took #1661 (comment) to mean we don’t have to handle that for now?
There was a problem hiding this comment.
I don’t think we have to handle it in the autofixer - but i think we do need to handle it in terms of warning.
There was a problem hiding this comment.
Ok then - here's my current understanding:
- when autofixing
<>to "element" form, it always goes to<React.Fragment> - but we should be able to determine that we need to warn on
<Fragment>(and it's then trivial to autofix it to<>)
For the latter, I'm thinking of
const { Fragment } = React;import { Fragment } from 'react';and maybe
const Fragment = React.Fragment;?
There was a problem hiding this comment.
Also
const { Fragment } = require('react');
| function getFixerToLong(jsxFragment) { | ||
| return function(fixer) { | ||
| let source = sourceCode.getText(); | ||
| source = replaceNode(source, jsxFragment.closingFragment, closeFragLong); |
There was a problem hiding this comment.
Btw, this would be trivially handled by returning an array of two fixer calls (one for opening and one for closing element), but this is not supported in ESLint 3 😞
alexzherdev
force-pushed
the
1661-fragments-rule
branch
from
33f5880 to
45019af
Compare
ljharb
force-pushed
the
1661-fragments-rule
branch
from
301a489 to
2e60d0e
Compare
| if (!versionUtil.testReactVersion(context, '16.2.0')) { | ||
| context.report({ | ||
| node, | ||
| message: 'Fragments are only supported starting from React v16.2' |
There was a problem hiding this comment.
Could we add something along the lines of:
Please disable the `jsx-fragments` rule or upgrade your version of React.
I think it's always a good idea to give users an action to do to help them resolve the error/warning.
There was a problem hiding this comment.
That sounds like a great improvement.
ghost
mentioned this pull request
Resolves #1661.