create-react-app: verifyPackageTree() errors when wrong version dependency is installed in any parent directory

@Timer I don’t fully understand the concern with hoisting based on your example here, assuming I am not misunderstanding how yarn’s hoisting works.

I recreated the example in a new branch. You were right that since webpack@3 is required more than webpack@4, webpack@3 is hoisted to the root directory. However, webpack@4 is still installed “locally” in the node_modules directory for the one package/workspace that depends on it.

Based on this hoisting behavior, create-react-app should not need to traverse and validate versions of the dependencies in all parent directories. It should only need to validate versions for the first occurrence of each dependency when traversing up the directory hierarchy (in order to mimic node’s package resolution behavior).

Let me know if I’m misunderstanding the problem we are trying to avoid/protect end users from.

Edit: tl;dr; Are we sure we cannot (1) Trust yarn to hoist dependencies properly in a way that does not impact create-react-app, and (2) Change verifyPackageTree() validation logic to only validate the dependencies that would actually be resolved by node’s package resolution logic?

I agree we can make this smarter – maybe we can try to detect when in a workspace/monorepo and bail out once we hit the root of the workspace.

@nicojs as mentioned above, this is the desired behavior – not a bug.

I just faced this when trying out react-script@2 for the first time. It definitely seems like a bug.

My project structure:

• project
  • node_modules (jest 23, for rn 0.56 and typescript)
  • android
  • ios
  • web
    • node_modules (jest 22)

Created an .env file with SKIP_PREFLIGHT_CHECK=true for now to ignore this.

All tests pass just fine when running yarn test.

So it isn’t a problem when the parent directory is not part of a monorepo or when you decide to not hoist the dependencies? If so, we could maybe build in this check? Right now, it’s pretty much all or nothing.

Side note: this hoisting business seems like a bad idea to me if it can break the dependency system.

I have this issue as well. If it’s intended behavior, is there a workaround other than putting the parent project in a sibling directory instead?

The problem would generally showcase in a monorepo when combined with hoisting:

Say you have a package abc that relies on foo and webpack@4, but a package def and ghi that relies on webpack@3.

The package manager hoists webpack@3 to the root because it’s used 2x, where webpack@4 is only used once. foo is also hoisted because there’s only one instance of it across your project.

Now, when foo requires webpack, it receives webpack@3 instead of webpack@4. This check is designed to prevent this behavior, and needs to be overly eager because we don’t know the list of packages which may require webpack.

/node_modules/webpack@3
/node_modules/foo
/packages/abc/node_modules/webpack@4
/packages/def/node_modules/<empty>
/packages/ghi/node_modules/<empty>

This same problem exists when a user self-installs webpack within a single project, causing webpack nesting in the tree.

@newoga, your example misses one crucial detail - the foo package mentioned by @Timer.

What will happen if you install a foo package that requires webpack@4 as a dependency?

According to this blog post - https://yarnpkg.com/blog/2018/04/18/dependencies-done-right/ - webpack@4 is installed in node_modules of foo package making your point valid.

But what will happen if you install a package that requires webpack@4 as a peerDependency?

According to the blog post above, it will target webpack@3 leading to a potential failure.

This is what @Timer tried to explain.

In my opinion, react-scripts shouldn’t prevent the build if a different version of the dependency is available higher in the tree. I understand that it is a simple safe solution but it leads to more confusion than it should.

That’s because it could prevent a build when everything is fine.

It sounds like the hoisting mechanism in yarn workspaces should make sure that dependencies that require peer dependencies are placed as close as to the required version of peer dependencies.

Edit

According to this issue - https://github.com/facebook/create-react-app/issues/1795 - the reality is not that simple. It looks like some common dependencies aren’t complying with the node module resolution strategy, what is very common in the front end world, or npm makes strange decisions when it comes to placing dependencies.

I have noticed that verifyPackageTree.js - https://github.com/facebook/create-react-app/blob/master/packages/react-scripts/scripts/utils/verifyPackageTree.js#L24 - checks only specific dependencies.

For monorepo users, as a temporary solution, I’d suggest setting a nohoist rule for packages mentioned in the link above.

Edit 2

The nohoist that works for me is:

    "nohoist": [
      "**/babel-eslint",
      "**/babel-jest",
      "**/babel-loader",
      "**/eslint",
      "**/jest",
      "**/webpack",
      "**/webpack-dev-server",
      "**/babel-eslint/**",
      "**/babel-jest/**",
      "**/babel-loader/**",
      "**/eslint/**",
      "**/jest/**",
      "**/webpack/**",
      "**/webpack-dev-server/**"
    ]

Edit 3

The nohoist above caused eslint to stop working completely in some workspaces in my project. Consider it a non-ideal solution.

@Timer thanks for the example, but I still believe something isn’t quite right with how the hoisting is working. E.g. in my case, the only dependency requiring babel is actually CRA (tried to remove the eslint package to check) but still the build check fails for what I shown before, preventing the production build.

Disabling the check and let the build go produces a perfectly working build tho, so no problem with deps resolving.

My tiny brain has problems to understand why it would be a problem, but if it is desired behavior an e2e case would help to explain why. The 4 steps to resolve the issue don’t help for this issue as well. A mention of this issue with the reason why would be greatly appreciated.

@Timer Okay, thanks for the quick response. Interested to see why but I will wait for the E2E test case. Feel free to close this issue or use is as a “reminder” for the E2E test case.

This is the intentional behavior. We’re going to make an E2E test case to showcase why.

I am also running into this in a lerna monorepo with npm (not yarn). ("react-scripts": "^2.0.0-next.3e165448")

We have one version of eslint installed for running lint-staged at the root of our monorepo and then within our CRA app’s package, it installs the react-scripts eslint.

This definitely seems like CRA trying to be too strict in its enforcement. Instead of checking every folder in the node module resolution path, wouldn’t it be enough to check require.resolve(<moduleName>) which would find the closest node_modules version?

I’ve got a similar use case to @brunolemos. It seems like a common project structure that we intend on supporting. (The proxy feature in CRA, for example, supports this project structure).

I’m similarly using SKIP_PREFLIGHT_CHECK=true to work around the issue.

@Timer Any news on the E2E test case to showcase why you mentioned?

Yes same error here. Quick steps to reproduce:

mkdir ~/tmp
cd ~/tmp
npm i -D webpack@4.16.5
npx create-react-app foo
cd foo
npm t

@Experiment8 I am facing the same error when I am building the react apps. I tested that with the basic template. But, still getting this same error as you. Did you happen to rectify this error in your application?

Just some additional context, I ran into this issue while testing and experimenting with the monorepo/yarn workspaces support in the next version. The side effect of having a create-react-app based project in a monorepo in this way is that all packages must share the same version of the handful of dependencies that create-react-app enforces.