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

Should 'react/react-in-jsx-scope' be disabled by default if the new JSX transform is used. #2334

Open
petetnt opened this issue Nov 12, 2020 · 9 comments

Comments

@petetnt
Copy link

petetnt commented Nov 12, 2020

A new JSX transform was introduced in React 17 and backported to React 16.14.0, 15.7.0, and 0.14.10, which enables people to use JSX features without having React in the scope. Obviously this will cause tons of react/react-in-jsx-scope errors in a new project (or that a project that used the codemod to remove the imports) that is using the airbnb config.

In CRA we use the following heuristic to catch if the new transform is available:

const hasJsxRuntime = (() => {
  try {
    require.resolve('react/jsx-runtime.js');
    return true;
  } catch (e) {
    return false;
  }
})();

My question is a) should the airbnb config use a similar heuristic or a) should the rule be disabled by default in a new major version or c) should the configuration option/issue be documented somewhere in detail as many people using the config will eventually run into it.

@ljharb
Copy link
Collaborator

ljharb commented Nov 12, 2020

If it’s truly possible to detect the use of the transform, then eslint-plugin-react itself should do that. However, the presence of the dep in no way ensures the transform is being used in the current project.

@JeanRibac
Copy link

is there a solution for this coming in the next version?

@ljharb
Copy link
Collaborator

ljharb commented Feb 1, 2021

@JeanRibac so far there’s no known solution that can automatically handle it, so atm no, it’s not coming in the next or any version.

@nickmccurdy
Copy link

nickmccurdy commented Jan 4, 2023

Why not offer a jsx-runtime config like eslint-plugin-react?

@ljharb
Copy link
Collaborator

ljharb commented Jan 4, 2023

I suppose we could, but that'd just be a re-export of the react plugin's config - why is it better for folks to extend airbnb/jsx-runtime rather than react/jsx-runtime?

@nickmccurdy
Copy link

nickmccurdy commented Jan 6, 2023

that'd just be a re-export of the react plugin's config

I don't think so, doesn't react/jsx-runtime extend the entire react/all config?

module.exports = Object.assign({}, all, {
  languageOptions: Object.assign({}, all.languageOptions, {
    parserOptions: Object.assign({}, all.languageOptions.parserOptions, {
      jsxPragma: null, // for @typescript/eslint-parser
    }),
  }),
});

https://github.com/jsx-eslint/eslint-plugin-react/blob/a5f70658d7d68a9d76c3a23b1fa2e1cfe02438ee/configs/jsx-runtime.js#L5-L15

@ljharb
Copy link
Collaborator

ljharb commented Jan 6, 2023

@nickmccurdy as i commented on the react plugin issue, it overwrites the "rules" property from that config (which doesn't enable any rules anyways), so it only turns off those two rules.

@nickmccurdy
Copy link

nickmccurdy commented Jan 6, 2023

Oh I see now, it's not actually assigning/spreading the rules property. Thanks for the explanation.

I guess my concern is more about the documentation then. Perhaps there should be a separate section in the readme for React 17 that recommends extending react/jsx-runtime. Alternatively, we could have separate configs for React ^16.8 and React >=17, extended recommended configs as needed.

@ljharb
Copy link
Collaborator

ljharb commented Jan 6, 2023

You can enable the jsx runtime in older react, and you don't have to use it in newer react - in other words, it's not tied to the react version, but to an unknowable user configuration.

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

No branches or pull requests

5 participants
@ljharb @nickmccurdy @petetnt @JeanRibac and others