Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

Set a more recent React version for eslint-plugin-react #1155

Closed
edmorley opened this issue Oct 7, 2018 · 6 comments
Closed

Set a more recent React version for eslint-plugin-react #1155

edmorley opened this issue Oct 7, 2018 · 6 comments
Assignees
Milestone

Comments

@edmorley
Copy link
Member

edmorley commented Oct 7, 2018

The eslint-plugin-react plugin takes a version option, which describes the React version used by a project, which helps rules decide which React features are available.

By default, no version is specified, which results in a warning:
https://github.com/yannickcr/eslint-plugin-react/blob/8f2663ed42860bb477d4720335674cf892ea65c5/lib/util/version.js#L16-L18

However when using via the AirBnB React config, then the version defaults to 16.0:
https://github.com/airbnb/javascript/blob/24da5bb540de12117bcdfa7b6d8ce3c0ff4b6504/packages/eslint-config-airbnb/rules/react.js#L462-L465

This suppresses the "no version set warning" and means that the rules are less strict than if set at a more recent version of React, such as 16.5. eg:
https://github.com/yannickcr/eslint-plugin-react/blob/7dce90a4f570847b33eaefddb8d9bb7d6eca9dc5/lib/rules/no-unsafe.js#L29-L32
https://github.com/yannickcr/eslint-plugin-react/blob/f27285f994291b450796303d461558ee91fffe47/lib/rules/no-deprecated.js#L80

I think that Neutrino's AirBnb preset should either:

  1. Set version to the latest stable React major version (ie 16.5) out of the box (and mention in the docs)
  2. Warn if people haven't set their own version, but continue as though they'd specified 16.5
  3. Warn if people haven't set their own version, but continue with the 16.0 default
  4. Error if people haven't set their own version

@eliperelman, thoughts?

@edmorley edmorley added this to the v9 milestone Oct 7, 2018
@edmorley
Copy link
Member Author

edmorley commented Oct 9, 2018

I'm leaning towards (1) and listing the version setting in the @neutrinojs/airbnb README, which should make it pretty clear. The only downside I can see is that we presumably won't be able to bump the version to say 16.6 outside of a Neutrino major version bump, so once a new React minor version comes out, people will have to set the version themselves anyway.

@eliperelman
Copy link
Member

As mentioned in IRC, another option would be to read the version of react in dependencies or devDependencies, stripping off any semantic versioning range identifiers.

@edmorley
Copy link
Member Author

A 6th option (hehe), would be to do what CRA does, and set a version of 999.999.999, which basically means "latest":
https://github.com/facebook/create-react-app/blob/5fecfee2373ebd1eda0c405b82a1c2d24afae6a1/packages/react-scripts/config/webpack.config.dev.js#L191-L194
https://github.com/facebook/create-react-app/blob/5fecfee2373ebd1eda0c405b82a1c2d24afae6a1/packages/react-scripts/config/webpack.config.prod.js#L267-L270

We could then update the @neutrinojs/airbnb docs to list the defaults as:

  use: [
    ['@neutrinojs/airbnb', {
      eslint: {
        settings: {
          react: {
            // Default to the enabling rules for the latest version of React
            version: '999.999.999',
          }
        },
      }
    }]
  ]

@eliperelman
Copy link
Member

So far option 6 seems like the best choice, thoughts?

@edmorley
Copy link
Member Author

Yeah I agree (6) seems best.

However fixing this is blocked on fixing the issues mentioned in #382 (comment), since it's not currently possible to set settings in a way that works for both eslint-loader and eslintrc, due to bugs in @neutrinojs/eslint. I have a branch locally that fixes this, but it's a significant refactor and I need to finish up tests/docs.

@edmorley edmorley self-assigned this Oct 19, 2018
@edmorley
Copy link
Member Author

I have the branch for this ready locally, but it's blocked on the review/merge of #1182.

edmorley added a commit that referenced this issue Oct 22, 2018
Several of the `eslint-plugin-react` rules use the specified React
version to determine what warnings/errors to show (for example only
showing deprecation warnings for certain features if using a newer
version of React). Not setting a version, or setting one that's older
than the version of React being used means that valid warnings/errors
may not actually be shown.

Whilst `eslint-plugin-react` helpfully outputs a warning if an explicit
React version has not been specified, several popular presets (such
as `eslint-config-airbnb` and `eslint-config-standard-react`) set a
default version of `16.0`, which results in many rules being disabled.

As such, we're using the trick used by CRA, of setting the version to
`999.999.999`, which ends up being interpreted as "latest version"
by `eslint-plugin-react`'s rules.

Fixes #1155.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

No branches or pull requests

2 participants