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

Unpin eslint version in react-scripts #5618

Closed
wants to merge 1 commit into from

Conversation

chadfawcett
Copy link

I've run into issues with eslint being the wrong exact version a few times now. Is there a reason fir it being pinned to an exact version?

I have a custom eslint config, so eslint is a peer dependency of my project. Since it's an approximate version, it resolves to 5.8.0. react-scripts then throws a warning when I try to run tests.

The react-scripts package provided by Create React App requires a dependency:

  "eslint": "5.6.0"

@chadfawcett
Copy link
Author

The test are failing with a similar error.

The react-scripts package provided by Create React App requires a dependency:
  "eslint": "^5.6.0"
Don't try to install it manually: your package manager does it automatically.
However, a different version of eslint was detected higher up in the tree:
  /home/travis/build/facebook/create-react-app/packages/react-scripts/node_modules/eslint (version: 5.8.0)

Shouldn't 5.8.0 satisfy ^5.6.0? https://docs.npmjs.com/misc/semver#advanced-range-syntax

@Timer
Copy link
Contributor

Timer commented Oct 29, 2018

This package is purposely pinned to prevent unwanted changes coming into the project. Please use 5.6.0 in your project.

@Timer Timer closed this Oct 29, 2018
@chadfawcett
Copy link
Author

Wouldn't semver prevent unwanted changes?

@chadfawcett chadfawcett deleted the unpin-eslint branch October 29, 2018 19:41
@chadfawcett
Copy link
Author

Also is there any documentation on the pinned versions? I couldn't find anything and it would be nice to have an explanation so people don't end up spending time on PRs in the future.

@Timer
Copy link
Contributor

Timer commented Oct 29, 2018

Semver is just a recommendation, there's no guarantees. A patch or minor could be a breaking change.
We also try to vet our packages before upgrading so users don't get viruses.

We have no formal documentation, but we have always pinned package versions (to be exact).

@chadfawcett
Copy link
Author

Ah okay. My apologize. I took a look back at v1 of the scripts to check what version you had for eslint and saw ^4.4.1. So I thought the pinning was new to v2. However I looked at package.json instead of packages/react-scripts/package.json.

I see the root version is a devDependency, so I assume the approximate version is okay here.

Thanks for clarifying.

@hulkish
Copy link

hulkish commented Nov 29, 2018

versions for published packages should not be pinned.. it creates more complex node_modules install trees - since each pinned version must be satisfied. Instead, it would be far more useful if, for example react-dev-utils had a postinstall script which warned for versions above what is known to work.

@lock lock bot locked and limited conversation to collaborators Jan 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants