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

Revisit dependency pinning #6756

Open
iansu opened this issue Apr 4, 2019 · 15 comments
Open

Revisit dependency pinning #6756

iansu opened this issue Apr 4, 2019 · 15 comments

Comments

@iansu
Copy link
Contributor

iansu commented Apr 4, 2019

Pinning all of our dependencies causes issues with the preflight check whenever a new version of a nested dependency is released. babel-jest is one example of this. When a new version is released and used by another dependency the preflight check fails until we update our version to match.

We should consider unpinning certain dependencies like babel-jest and updating the preflight check to work with carrot versions like ^24.7.1. It currently only works with exact version numbers.

@ianschmitz
Copy link
Contributor

Here's an example we get a preflight warning in our CI. We noticed this quite a bit the last few days as Jest has released a few new versions in quick succession. Each new release would break CI.

This is an example npm ls babel-jest output when everything works nicely:

test-app@0.1.0 /c/projects/test-app
└─┬ react-scripts@3.0.0-next.b0cbf2ca
  ├── babel-jest@24.5.0
  └─┬ jest@24.5.0
    └─┬ jest-cli@24.5.0
      └─┬ jest-config@24.5.0
        └── babel-jest@24.5.0  deduped

However when a new version of jest comes out, let's say 24.6.0, even though we pinned jest@24.5.0 and babel-jest@24.5.0, jest references jest-cli: ^24.5.0.

What would happen during npm install is we get jest-cli@24.6.0, which brings in jest-config@24.6.0 and babel-jest@24.6.0 which ends up in the root of node_modules, while putting babel-jest@24.5.0 inside node_modules/react-scripts/node_modules/babel-jest, and then when running CRA we get the preflight warning.

@minesaner
Copy link

The preflight check folder is <rootDir>/node_modules. Maybe <scriptsVersion>/node_modules should be check first.

@yuchuan1
Copy link

yuchuan1 commented May 5, 2019

Jest 24.8.0 breaks it again.

When I run npx create-react-app someapp and then run yarn start. It gave me the following error.

yarn run v1.15.2
$ react-scripts start

There might be a problem with the project dependency tree.
It is likely not a bug in Create React App, but something you need to fix locally.

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

"babel-jest": "24.7.1"

Don't try to install it manually: your package manager does it automatically.
However, a different version of babel-jest was detected higher up in the tree:

/Users/eddiechen/github/someapp/node_modules/babel-jest (version: 24.8.0)

Manually installing incompatible versions is known to cause hard-to-debug issues.

If you would prefer to ignore this check, add SKIP_PREFLIGHT_CHECK=true to an .env file in your project.
That will permanently disable this message but you might encounter other issues.

To fix the dependency tree, try following the steps below in the exact order:

  1. Delete package-lock.json (not package.json!) and/or yarn.lock in your project folder.
  2. Delete node_modules in your project folder.
  3. Remove "babel-jest" from dependencies and/or devDependencies in the package.json file in your project folder.
  4. Run npm install or yarn, depending on the package manager you use.

In most cases, this should be enough to fix the problem.
If this has not helped, there are a few other things you can try:

  1. If you used npm, install yarn (http://yarnpkg.com/) and repeat the above steps with it instead.
    This may help because npm has known issues with package hoisting which may get resolved in future versions.

  2. Check if /Users/eddiechen/github/someapp/node_modules/babel-jest is outside your project directory.
    For example, you might have accidentally installed something in your home folder.

  3. Try running npm ls babel-jest in your project folder.
    This will tell you which other package (apart from the expected react-scripts) installed babel-jest.

If nothing else helps, add SKIP_PREFLIGHT_CHECK=true to an .env file in your project.
That would permanently disable this preflight check in case you want to proceed anyway.

P.S. We know this message is long but please read the steps above :-) We hope you find them helpful!

error Command failed with exit code 1.

The error went away after I changed "babel-jest": "24.7.1" to "babel-jest": "^24.7.1" in package.json of create-react-app.

@icyJoseph
Copy link

Suffering from the very same issue. Any CRA made this morning fails to start because of 24.8.0 in babel-jest. Same output as @yuchuan1

@abisuq
Copy link

abisuq commented May 6, 2019

add a version resolutions into package.json temporarily

"resolutions": {
    "babel-jest": "24.7.1"
  },

@russellr922
Copy link

Ran into same problem today.

@jdarling
Copy link

jdarling commented May 6, 2019

Resolutions didn't work for me. Any other ideas on a work around? I'm using Yarn so that might have something to do with it, but I've tried with npm as well and that just hangs at starting before it finally dies with an allocation failure.

@eddiemonge
Copy link
Contributor

Also getting this with webpack

@ericvera
Copy link

ericvera commented May 7, 2019

@jdarling To run the server, as a workaround, I've been using the following command. My guess is that any command that fails you can just add SKIP_PREFLIGHT_CHECK=true before it for now.

SKIP_PREFLIGHT_CHECK=true yarn start

@jdarling
Copy link

jdarling commented May 7, 2019

Everything mentioned above results in:

Starting the development server...


<--- Last few GCs --->

[3450:0x33a7100]    96265 ms: Mark-sweep 1370.9 (1424.0) -> 1369.9 (1424.0) MB, 1980.0 / 0.0 ms  (average mu = 0.098, current mu = 0.001) allocation failure scavenge might not succeed
[3450:0x33a7100]    98223 ms: Mark-sweep 1371.1 (1424.5) -> 1370.1 (1424.5) MB, 1957.0 / 0.0 ms  (average mu = 0.052, current mu = 0.001) allocation failure scavenge might not succeed


<--- JS stacktrace --->

==== JS stack trace =========================================

    0: ExitFrame [pc: 0x16c69525be1d]
Security context: 0x37af7ed9e6e9 <JSObject>
    1: SourceMapGenerator_addMapping [0x148b9ae37b89] [/home/jdarling/su360/gateway/src/public/node_modules/@babel/generator/node_modules/source-map/lib/source-map-generator.js:~94] [pc=0x16c695bcfa18](this=0x37925ba82201 <SourceMapGenerator map = 0x1cd3fe538759>,aArgs=0x0d1e0617c339 <Object map = 0x25e847ee1e71>)
    2: arguments adaptor frame: 3->1
    3: ...

FATAL ERROR: Ineffective mark-compacts near heap limit Allocation failed - JavaScript heap out of memory
 1: 0x8dc510 node::Abort() [/home/jdarling/.nvm/versions/node/v10.15.3/bin/node]
 2: 0x8dc55c  [/home/jdarling/.nvm/versions/node/v10.15.3/bin/node]
 3: 0xad9b5e v8::Utils::ReportOOMFailure(v8::internal::Isolate*, char const*, bool) [/home/jdarling/.nvm/versions/node/v10.15.3/bin/node]
 4: 0xad9d94 v8::internal::V8::FatalProcessOutOfMemory(v8::internal::Isolate*, char const*, bool) [/home/jdarling/.nvm/versions/node/v10.15.3/bin/node]
 5: 0xec7bf2  [/home/jdarling/.nvm/versions/node/v10.15.3/bin/node]
 6: 0xec7cf8 v8::internal::Heap::CheckIneffectiveMarkCompact(unsigned long, double) [/home/jdarling/.nvm/versions/node/v10.15.3/bin/node]
 7: 0xed3dd2 v8::internal::Heap::PerformGarbageCollection(v8::internal::GarbageCollector, v8::GCCallbackFlags) [/home/jdarling/.nvm/versions/node/v10.15.3/bin/node]
 8: 0xed4704 v8::internal::Heap::CollectGarbage(v8::internal::AllocationSpace, v8::internal::GarbageCollectionReason, v8::GCCallbackFlags) [/home/jdarling/.nvm/versions/node/v10.15.3/bin/node]
 9: 0xed7371 v8::internal::Heap::AllocateRawWithRetryOrFail(int, v8::internal::AllocationSpace, v8::internal::AllocationAlignment) [/home/jdarling/.nvm/versions/node/v10.15.3/bin/node]
10: 0xea07f4 v8::internal::Factory::NewFillerObject(int, bool, v8::internal::AllocationSpace) [/home/jdarling/.nvm/versions/node/v10.15.3/bin/node]
11: 0x114018e v8::internal::Runtime_AllocateInNewSpace(int, v8::internal::Object**, v8::internal::Isolate*) [/home/jdarling/.nvm/versions/node/v10.15.3/bin/node]
12: 0x16c69525be1d 
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

Or some variant for me. Last week same code no problem, this week everything is broke.

@jdarling
Copy link

jdarling commented May 7, 2019

Ok, if anyone else is still suffering make sure you delete your node_modules folder, yarn.lock (or package-lock.json) file, and clear your cache. yarn cache clear (in my case). Then do a clean install. Unfortunately for those of us on 3rd world internet connections that can mean hours of waiting.

Thanks for forcing jest down our throats and not locking versions down so it could screw us.

@ianschmitz
Copy link
Contributor

Thanks for forcing jest down our throats and not locking versions down so it could screw us.

That is a very unfair and unproductive statement. Most of our users are very happy with Jest. You are also free to use your own testing framework and/or eject if needed.

We are locking dependency versions down in CRA in almost all cases. However we can't control how our dependencies reference other dependency versions, and in some cases we end up in these scenarios (which we are addressing).

Also keep in mind that this project is mostly maintained by the community. We contribute to this project outside of our day jobs as it is something we're passionate about and love doing. Please keep the comments polite and constructive. Thanks!

@jdarling
Copy link

jdarling commented May 7, 2019

Inflammatory yes, very unfair and unproductive, not at all. Can I install or use create-react-app without having Jest installed even though I don't use Jest, nope (statement is accurate and thus fair). On more than one occasion has Jest broke backward compatibility by not following semantic versioning rules, yes (statement is backed by historic evidence, provokes a response and thus is productive).

The testing package should be a dependency that acts as an addon like everything else, not a default.

My statement is completely accurate and I stand by it.

@henryruhs
Copy link

henryruhs commented May 7, 2019

It is not unfair or unproductive - it just puts the finger in the wound. Jest and React are developed by Facebook and I strongly recommend an inhouse NPM registry powered CI pipeline to run against ALL projects such as Yarn, Jest, React etc. before you guys deploy to the public registry and break thousands of projects and builds.

I did a little "kind of stupid" math but it shows the scope:

4676703 downloads per week (react)
 668100 downloads per day (react)

1h ~ average time to research and fix an issue like this one

668100h = 76years

36 years ~ average time of a career

congrats, 2 developers have been killed today

@iansu
Copy link
Contributor Author

iansu commented May 7, 2019

This project isn't Jest or React and none of the maintainers of Create React App work at Facebook. We are all volunteers working on this project in our spare time.

@facebook facebook locked as too heated and limited conversation to collaborators May 7, 2019
@ianschmitz ianschmitz unpinned this issue Jun 22, 2019
@ianschmitz ianschmitz modified the milestone: 3.x Oct 5, 2019
@ianschmitz ianschmitz added this to the 3.3 milestone Oct 5, 2019
@iansu iansu modified the milestones: 3.3, 4.0 Oct 16, 2019
@ianschmitz ianschmitz modified the milestones: 4.0, 4.x May 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.