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

pnpm appears to be pulling in multiple versions of React when used with NextJS 9.5.0+ #2743

Closed
rfestag opened this issue Aug 1, 2020 · 19 comments

Comments

@rfestag
Copy link

rfestag commented Aug 1, 2020

This issue was originally raised with NextJS (vercel/next.js#15616). Upon investigation, they determined it was a pnpm bug causing pnpm to bring in multiple versions of React, which is why I'm opening this ticket.

I am able to work around the issue by using a pnpmfile.js as referenced below, then re-installing next when I am not using a workspace. However, even if I have the pnpmfile.js in the root of a monorepo and I see messages indicating it is removing the dependencies, I still get the same error:
#2695

Any help identifying the cause (whether it is a bug in pnpm or something in NextJS's latest releases) would be greatly appreciated. I just don't know how to investigate this further.

pnpm version:

5.4.9

Code to reproduce the issue:

The issue is most readily apparent when creating a new NextJS application. This still install NextJS 9.5.1

First, use create-next-app to install (this will use NPM by default). Note that it works as expected with NPM:

pnpm init next-app test
cd test
npm run dev

Then switch to using pnpm instead. Note that navigating to the server shows the standard Error: Invalid hook call error.

rm -rf node_modules package-lock.json
pnpm install
pnpm run dev

However if you install the last from the 9.4.x series, it runs as expected:

pnpm install next@9.4.4
pnpm run dev

Considering there is no code change between the various scenarios, it looks like pnpm is somehow pulling in multiple versions of React.

Expected behavior:

I don't expect any complaints about React hooks. It looks like, whatever NextJS changed in their 9.5.0 baseline, they seem to have made some pnpm bug apparent.

Actual behavior:

I get the following error message:

Error: Invalid hook call. Hooks can only be called inside of the body of a function component. This could happen for one of the following reasons:
1. You might have mismatching versions of React and the renderer (such as React DOM)
2. You might be breaking the Rules of Hooks
3. You might have more than one copy of React in the same app
See https://fb.me/react-invalid-hook-call for tips about how to debug and fix this problem.

Additional information:

  • node -v prints: 14.3.0
  • Windows, OS X, or Linux?: Linux (Manjaro)
@zkochan
Copy link
Member

zkochan commented Aug 4, 2020

This is not a pnpm bug. As I mentioned in the linked issue, pnpm does not install multiple versions of react. Those are just symlinks to the same directory.

This seems like a bug in React. In this issue I see they mention it appears on lerna monorepos as well.

A possible workaround would be to not install react as a dependency in any of the workspace projects, except the root project of the workspace.

@aparajita
Copy link
Contributor

This is a well-known problem with React which is unrelated to pnpm in particular:

facebook/react#13991
https://reactjs.org/warnings/invalid-hook-call-warning.html#duplicate-react

Seems like React should just fix its package structure so this doesn't happen.

A possible workaround would be to not install react as a dependency in any of the workspace projects, except the root project of the workspace.

The react package has no binaries, so you should be able to install it at the root of the workspace. The other thing you can do is use syncpack to sync the versions across the workspace.

@zkochan I think we can close this.

@rfestag
Copy link
Author

rfestag commented Aug 5, 2020

Yes, the error is a well known issue. However, I do think there is something at least odd within pnpm (particularly with the fact that the workspaces end up behaving fundamentally differently). It may be that a different issue should be opened, or that this simply be tracked through #2695 instead. It does seem odd that the behavior of the pnpmfile is different between a standard repository and a monorepo.

I don't think syncpack will solve the issue. In my particular case I know for a fact that I am only using the exact same version of react. Again, I wasn't really convinced it was really a pnpm issue to begin with, but they pointed fingers at pnpm and I wasn't sure how to dig further into the issue.

@aparajita
Copy link
Contributor

they pointed fingers at pnpm

Those who structure their packages in a brittle way always blame it on the package managers.

Did you try all of the workarounds people mentioned here?

@bitfrost
Copy link

bitfrost commented Aug 5, 2020

As suggested by @zkochan :

A possible workaround would be to not install react as a dependency in any of the workspace projects, except the root project of the workspace.

^^ doing this via pnpmfile.js did just work for me in a workspace based project using next.js 9.5.1

pnpfile.js

function readPackage(pkg, context) {

if (pkg.peerDependencies && pkg.peerDependencies.react) {
// fixes react error "Error: Invalid hook call."
// with multiple copy of react in the node_modules folder
context.log([${pkg.name}] Removing react as a peer dependency (https://bit.ly/3jmD8J6).)
delete pkg.peerDependencies.react;
}

if (pkg.dependencies && pkg.dependencies.react) {
// fixes react error "Error: Invalid hook call."
// with multiple copy of react in the node_modules folder
context.log([${pkg.name}] Removing react as dependency (https://bit.ly/3jmD8J6).)
delete pkg.dependencies.react;
}

return pkg;
}

module.exports = {
hooks: { readPackage }
};

@rfestag
Copy link
Author

rfestag commented Aug 5, 2020

@aparajita - No, but honestly I'm not sure which of the various workarounds in there really applies in my particular case. Some fixes reference yarn, some reference adding alias to some undefined file, others recommend moving React out of the bundle (not applicable in this case, as the problem is not in a component library).

That said, I can also confirm @zkochan's resolution (moving react up to the root of a monorepo) works in conjunction with a pnpmfile that removes react from the peerDependencies of all dependencies.

Personally, my only issue with this solution is it means my NextJS app in the monorepo doesn't explicitly list its expected dependencies. Any of my component packages only use React as a dev dependency, so pulling it out of that and only listing it in the peerDependencies is fine (and consistent with how I'm using my monorepo).

My question is this: Does the fact that I have to move that dependency up to the root indicate a potential bug (or point of improvement) in pnpm? I have less of an issue using the pnpmfile (that is intended to resolve issues with how dependencies are packaged, and the consensus is that the fundamental bug initially reported lies outside of pnpm). However, it feels wrong that we must both use the pnpmfile and move the dependency up to the root (where, in some cases, that dependency may be where we don't expect it). Before this ticket closes out, can anyone shed some light on this?

@bitfrost
Copy link

bitfrost commented Aug 5, 2020

A possible workaround would be to not install react as a dependency in any of the workspace projects, except the root project of the workspace.

This (placing react as root dependency of he mono repo / package.json) plus removing react as a dependency of other packages via pnpmfile.js I was able to get a next.js 9.5.1 app working in monorepo with workspaces enabled @rfestag. I agree doesn't feel right, but I think in one of the issues you had been looking for next.js 9.5 + pnpm + workspace mono repo to work, so thought I would mention it.

@aparajita
Copy link
Contributor

Does the fact that I have to move that dependency up to the root indicate a potential bug (or point of improvement) in pnpm?

Not at all, to me it's an advantage of using a monorepo that you can hoist common packages up in the directory tree — they don't necessarily have to be in the root.

@rfestag
Copy link
Author

rfestag commented Aug 6, 2020

@bitfrost - I'm aware, I was actually confirming that I got the same result as you when I said above that "I can also confirm @zkochan's resolution". But thanks for double-checking that I saw that, I obviously wasn't clear in my statement above.

@aparajita - I agree in the case of devDependencies and peerDependencies, but I'm not sure I agree in the case of dependencies. Let's take React and NextJS out of the picture. Say I have a monorepo with a bunch of packages, all of which have a runtime dependency on package foo (i.e., it is a "common" package). Say I decide that it is inappropriate for that dependency to be a peerDependency. As I understand your argument, because it is "common", it makes sense to pull up to the root. However, that would fundamentally break the NPM package itself (since it is a runtime dependency, it must be either a peerDependency or a dependency). Furthermore, let's say I do need to use a pnpmfile.js to modify foo (perhaps I need to force a dependency version, or I need to inject a dependency because the owner of foo did it wrong...any of the use cases the pnpmfile.js is intended to help resolve). As I'm understanding current state of things in pnpm, I can't do that. The pnpmfile.js doesn't seem to properly affect dependencies in packages installed below the root; the only way I can have it modify them is to move that dependency up to the root. However, in this case, I can't do that because it needs to be in the dependencies.

This is a little bit of a contrived example, but it seems reasonable. Again, I may well be wrong. Please correct me if I'm missing something. I'm mostly concerned by the inconsistency of how the pnpmfile.js works in monorepos, and that it indicates an issue that should be addressed. Maybe this is a low priority issue; that's a fine response. Maybe it is a non-issue; if so, I'm hoping to understand why. If you think it is more appropriate to close this issue and for a different (more properly phrased issue) to be created to discuss further, I'm happy to do so.

@zkochan
Copy link
Member

zkochan commented Aug 6, 2020

The pnpmfile fix doesn't work in a workspace due to the same symlinking issue. The workaround works in a single project because when the peers are removed, only one symlink is created to react, in the root of node_modules. However, in a workspace, if several projects have react in dependencies, each of the projects will have a symlink in its node_modules. Hence, the React issue will appear.

So I don't see any inconsistency in how pnpmfile works in a monorepo vs single project.

@rfestag
Copy link
Author

rfestag commented Aug 6, 2020

@zkochan - Thanks for the explanation. I think I'm still missing something based on the description above. What about the case where I only have one app/package depending on React? In my test examples, I created a monorepo with just a NextJS app (i.e., nowhere else in the monorepo was React a dependency, devDependency, or peerDependency. If the problem is that there are several projects with React dependencies (and the symlinks are treated as distinct rather than the same), how does this situation arise? One example I described in the other issue was taking a working NextJS app, moving it into an empty workspace (and moving the pnpmfile.js up to the root from the app), and I still had the same issue.

@konsumer
Copy link

konsumer commented Aug 11, 2020

I get this issue with even a very simple next project in the pnpm workspace.

Here is an example project that only has one package, which is a next frontend, and the simplest possible nextjs site (a single page that just says "It works".)

In order to recreate issue, I clone, run pnpm i in root, then pnpm start inside packages/frontend

I get this:

Error: Invalid hook call. Hooks can only be called inside of the body of a function component. This could happen for one of the following reasons:
1. You might have mismatching versions of React and the renderer (such as React DOM)
2. You might be breaking the Rules of Hooks
3. You might have more than one copy of React in the same app
See https://fb.me/react-invalid-hook-call for tips about how to debug and fix this problem.
    at resolveDispatcher (webpack-internal:///../../node_modules/.pnpm/react@16.13.1/node_modules/react/cjs/react.development.js:1465:13)
    at useContext (webpack-internal:///../../node_modules/.pnpm/react@16.13.1/node_modules/react/cjs/react.development.js:1473:20)
    at Html (webpack-internal:///../../node_modules/.pnpm/next@9.5.2_react-dom@16.13.1+react@16.13.1/node_modules/next/dist/pages/_document.js:158:29)
    at processChild (/home/konsumer/Desktop/next-workspace/node_modules/.pnpm/react-dom@16.13.1_react@16.13.1/node_modules/react-dom/cjs/react-dom-server.node.development.js:3043:14)
    at resolve (/home/konsumer/Desktop/next-workspace/node_modules/.pnpm/react-dom@16.13.1_react@16.13.1/node_modules/react-dom/cjs/react-dom-server.node.development.js:2960:5)
    at ReactDOMServerRenderer.render (/home/konsumer/Desktop/next-workspace/node_modules/.pnpm/react-dom@16.13.1_react@16.13.1/node_modules/react-dom/cjs/react-dom-server.node.development.js:3435:22)
    at ReactDOMServerRenderer.read (/home/konsumer/Desktop/next-workspace/node_modules/.pnpm/react-dom@16.13.1_react@16.13.1/node_modules/react-dom/cjs/react-dom-server.node.development.js:3373:29)
    at renderToStaticMarkup (/home/konsumer/Desktop/next-workspace/node_modules/.pnpm/react-dom@16.13.1_react@16.13.1/node_modules/react-dom/cjs/react-dom-server.node.development.js:4004:27)
    at renderDocument (/home/konsumer/Desktop/next-workspace/node_modules/.pnpm/next@9.5.2_react-dom@16.13.1+react@16.13.1/node_modules/next/dist/next-server/server/render.js:3:624)
    at renderToHTML (/home/konsumer/Desktop/next-workspace/node_modules/.pnpm/next@9.5.2_react-dom@16.13.1+react@16.13.1/node_modules/next/dist/next-server/server/render.js:50:72)

For testing, I do this to ensure the thing works, elswhere:

cd packages/frontend
rm -rf node_modules
npm i
npm start

And it's fine.

What do I need to do to make it work? If this is a fragile module-layout (that follows the pnpm docs, as far as I can tell) how should I structure it, instead?

@remorses
Copy link

This is a issue with latest nextjs, it will be fixed by vercel/next.js#16369

@skivol
Copy link

skivol commented Oct 24, 2020

Next.js fix is available in v9.5.6-canary.0 (vercel/next.js#17279 (comment))
Can be already used as next@canary (vercel/next.js#17279 (comment))

@aparajita
Copy link
Contributor

This is a issue with latest nextjs, it will be fixed by vercel/next.js#16369

Like we've been saying, it wasn't pnpm's fault. 😁 Usually dependency problems arise from one of two things:

  • Packages that do not declare (or document) all of their dependencies, but get away with it (consciously or unconsciously) in npm or yarn 1 due to the flattened node_modules they use.

  • Packages that dynamically find other packages not following symlinks or expecting node_modules to be flat.

@bitfrost
Copy link

@zkochan the release was fixed in next.js a while ago, this probably can be closed

@rhyek
Copy link

rhyek commented Jul 22, 2021

Had this problem with a workspace with two apps both depending on different versions of react. Installed the same version for react and react-dom in both apps and problem was fixed.

@leovigna
Copy link

leovigna commented Aug 7, 2021

For me the issue was more than just different versions. In my Lerna workspace I had the same React version installed in a library package and my frontend react package. As @aparajita mentioned it's not pnpm's fault, but the issue does seem to arise due to how React handles the symlinks created by pnpm.

@zkochan solution was insufficient for me as I needed React as a dependency for my library's testing. My solution was to use package hoisting as described https://github.com/lerna/lerna/blob/main/doc/hoist.md This solved the issue as only one symlink is created at the root of your workspace as opposed to per-package. I use Lerna with pnpm (npmClient: pnpm) for workspace management so this was a good solution for me.

sjdemartini added a commit to sjdemartini/mui-tiptap that referenced this issue May 17, 2023
This still doesn't work properly, due to issues with duplicate React
instances being installed as described in
pnpm/pnpm#2743 and
https://legacy.reactjs.org/warnings/invalid-hook-call-warning.html#duplicate-react
sjdemartini added a commit to sjdemartini/mui-tiptap that referenced this issue May 17, 2023
This still doesn't work properly, due to issues with duplicate React
instances being installed as described in
pnpm/pnpm#2743 and
https://legacy.reactjs.org/warnings/invalid-hook-call-warning.html#duplicate-react
@zkochan
Copy link
Member

zkochan commented Feb 9, 2024

Tested with pnpm v9.0.0-alpha.4
It works now.

@zkochan zkochan closed this as completed Feb 9, 2024
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

9 participants