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

Next.js v10.0.0 forces React v17 to be installed #18518

Closed
jaydenseric opened this issue Oct 30, 2020 · 30 comments · Fixed by #19087
Closed

Next.js v10.0.0 forces React v17 to be installed #18518

jaydenseric opened this issue Oct 30, 2020 · 30 comments · Fixed by #19087
Assignees
Labels
Upstream Related to using Next.js with a third-party dependency. (e.g., React, UI/icon libraries, etc.).
Milestone

Comments

@jaydenseric
Copy link
Contributor

Bug report

Describe the bug

next v10.0.0 forces react v17 to be installed, even if the project has react v16 in dependencies.

This is because next v10.0.0 has a use-subscription v1.5.0 dependency pinned:

"use-subscription": "1.5.0",

use-subscription v1.5.0 has a react peer dependency of ^17.0.0 (v16 not supported):

https://unpkg.com/use-subscription@1.5.0/package.json

With a project that has a react v16 dependency, npm v6 at install time would emit a warning that the use-subscription peer dependency on react is not satisfied, but it would not install react v17. While this may seem ok, it probably is not because use-subscription declared react v16 is not supported, so who knows if it will have problems in it's function.

With npm v7, which automatically installs peer dependencies as needed, this results in react v16 being installed to fulfill the project dependency, and react v17 to fulfill the use-subscription peer dependency. The result is that multiple versions of react get installed, and this can cause a React render error that multiple instances of React are being used with hooks.

To Reproduce

  1. In a hello-world Next.js project, ensure React v16 is specified in the package dependencies.
  2. Use npm v7 to install the project.
  3. Run npm ls react to see the multiple versions installed. This in itself is bad.
  4. At this point, you could try to run npm run dev to see if you get the same React render error I do in my project about hooks and multiple React instances, but I'm not 100% sure if it will happen out of the box or if you need to use some particular hooks to see the issue.

Expected behavior

When used with npm v6 or v7, next should respect the version of react installed in the user's project, and not cause multiple versions of react to be installed.

Screenshots

Screen Shot 2020-10-30 at 2 36 21 pm

System information

  • OS: macOS 10.15.7
  • Version of Next.js: 10.0.0
  • Version of Node.js: Any compatible with npm v6, v7
  • Version of npm: v7 especially
@adammockor
Copy link

I have exactly the same problem with same setup.

@sungsong88
Copy link

I have the same issue here

@billyjanitsch
Copy link

A bit of additional context: React made a breaking change between use-subscription@1.4.1 and use-subscription@1.5.0 (updating the React peer dependency from ^16.8.0 to ^17.0.0). Next.js upgraded this dependency in #18199, noting that it would cause a peer deps issue for React 16 users, but ideally it wouldn't cause issues for either major version of React.

I reported the upstream issue here: facebook/react#20224. Hopefully it gets fixed by way of releasing a version of use-subscription that peer-depends on ^16.8.0 || ^17.0.0 such that Next.js can upgrade to it and retain support for both major versions of React.

cc @timneutkens who authored the dependency upgrade PR.

@gaearon
Copy link

gaearon commented Nov 11, 2020

React made a breaking change

While I understand the frustration, I don't think there is a common consensus that changing a peer dependency in any way constitutes a breaking change by itself. (In part because they have been not considered in the algorithm for a very long time.)

It is news to me that npm v7 has changed the behavior to consider peer dependencies in the algorithm. This is actually pretty worrying for our use case because react must be a singleton (it's used for dependency injection), and we would rather refuse the installation than have two copies in the tree.

I think this npm design might force our hand to simply remove the peerDependencies and let the user figure out the matching versions themselves. Because the auto-installing seems much worse.

@gaearon
Copy link

gaearon commented Nov 11, 2020

Where can I read about the auto-installing and whether this decision in npm is final?

@gaearon
Copy link

gaearon commented Nov 11, 2020

Actually I'm not sure I understand.. Is it being auto-installed as a duplicate, or is installation refused?

@Timer
Copy link
Member

Timer commented Nov 11, 2020

@gaearon see https://github.com/npm/rfcs/blob/latest/implemented/0025-install-peer-deps.md

Funny that they call this out:

The main issue is that, because the use of peerDependencies has gotten so popular in the React community, and because React is extremely popular among front-end developers who are somewhat new to npm, the hazards of the current approach affect them the most profoundly, and they are the least able to know what to do when faced with the error.

It seems like packages can opt-out of the auto-installing peer dep behavior by using peerDependenciesMeta:

{
  "peerDependencies": {
    "react": "17.x.x"
  },
  "peerDependenciesMeta": {
    "react": {
      "autoinstall": false
    }
  }
}

@billyjanitsch
Copy link

I don't think there is a common consensus that changing a peer dependency in any way constitutes a breaking change by itself.

I know there's some debate, but, from what I've seen, the ecosystem overwhelmingly adheres to this. Widening a peer dependency range isn't considered breaking, but almost all packages will bump major versions when changing a peer dep such that a previously-valid resolution may no longer valid.

we would rather refuse the installation than have two copies in the tree

The installation is refused. In npm@7, peer dependencies are automatically installed, but the semantics are otherwise similar. Suppose a depends on c@1 and b depends on c@2. If you try to npm install a b, npm will refuse to install. (It's now an error rather than a warning to have an invalid peer resolution, and the install won't complete.) So there shouldn't be any need for React to change any of its versioning strategy in general.

Where can I read about the auto-installing and whether this decision in npm is final?

The decision seems final. It was proposed as an RFC in March (npm/rfcs#43) and implemented shortly after. It was released in the npm@7 beta in August, and npm@7.0.0 final was released in October. You can read more about the rationale here and the specific algorithm changes here.

@billyjanitsch
Copy link

Funny that they call this out:

The main issue is that, because the use of peerDependencies has gotten so popular in the React community, and because React is extremely popular among front-end developers who are somewhat new to npm, the hazards of the current approach affect them the most profoundly, and they are the least able to know what to do when faced with the error.

@Timer to be clear, they're presenting that as a reason to prefer the new behavior of stricter validation. ("The main issue" with preserving the npm@6 behavior.) Because invalid peer deps are just a console warning in npm@6, it's easy for developers to miss that and end up with an invalid installation. npm@7 makes this a clear error by refusing to install. (The error message offers a workaround of running npm i --force if you're sure that you want an invalid tree.)

@gaearon
Copy link

gaearon commented Nov 11, 2020

Say we have react-dom@17.0.0 that peer-depends on exact react@17.0.0. Then we release react-dom@17.0.1 that peer-depends on exact react@17.0.1. Is this valid or does it constitute a breaking change?

@billyjanitsch
Copy link

@gaearon FYI here's an example of what happens if you try an install that would result in multiple versions of React:

❯ npm i react@16 use-subscription@1.5
npm ERR! code ERESOLVE
npm ERR! ERESOLVE unable to resolve dependency tree
npm ERR! 
npm ERR! While resolving: undefined@undefined
npm ERR! Found: react@16.14.0
npm ERR! node_modules/react
npm ERR!   react@"16" from the root project
npm ERR! 
npm ERR! Could not resolve dependency:
npm ERR! peer react@"^17.0.0" from use-subscription@1.5.0
npm ERR! node_modules/use-subscription
npm ERR!   use-subscription@"1.5" from the root project
npm ERR! 
npm ERR! Fix the upstream dependency conflict, or retry
npm ERR! this command with --force, or --legacy-peer-deps
npm ERR! to accept an incorrect (and potentially broken) dependency resolution.
npm ERR! 
npm ERR! See /Users/billy/.npm/eresolve-report.txt for a full report.

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/billy/.npm/_logs/2020-11-11T22_58_03_210Z-debug.log

@Timer
Copy link
Member

Timer commented Nov 11, 2020

Ah, I see. OP's issue is misleading here:

With npm v7, which automatically installs peer dependencies as needed, this results in react v16 being installed to fulfill the project dependency, and react v17 to fulfill the use-subscription peer dependency. The result is that multiple versions of react get installed, and this can cause a React render error that multiple instances of React are being used with hooks.

OP makes it sound like npm defaults to installing two copies of React.

npm 7 by default will refuse to install:

❯ npm i
npm ERR! code ERESOLVE
npm ERR! ERESOLVE unable to resolve dependency tree
npm ERR! 
npm ERR! Found: react@16.14.0
npm ERR! node_modules/react
npm ERR!   react@"16.14.0" from the root project
npm ERR!   peer react@"^16.6.0 || ^17" from next@10.0.1
npm ERR!   node_modules/next
npm ERR!     next@"10.0.1" from the root project
npm ERR!   3 more (react-dom, @next/react-dev-overlay, styled-jsx)
npm ERR! 
npm ERR! Could not resolve dependency:
npm ERR! peer react@"^17.0.0" from use-subscription@1.5.0
npm ERR! node_modules/next/node_modules/use-subscription
npm ERR!   use-subscription@"1.5.0" from next@10.0.1
npm ERR!   node_modules/next
npm ERR!     next@"10.0.1" from the root project
npm ERR! 
npm ERR! Fix the upstream dependency conflict, or retry
npm ERR! this command with --force, or --legacy-peer-deps
npm ERR! to accept an incorrect (and potentially broken) dependency resolution.

--force errors too:

❯ npm i --force                                                                              ✘ 1 
npm WARN using --force Recommended protections disabled.
npm ERR! code ERESOLVE
npm ERR! ERESOLVE unable to resolve dependency tree
npm ERR! 
npm ERR! Found: react@16.14.0
npm ERR! node_modules/react
npm ERR!   react@"16.14.0" from the root project
npm ERR!   peer react@"^16.6.0 || ^17" from next@10.0.1
npm ERR!   node_modules/next
npm ERR!     next@"10.0.1" from the root project
npm ERR!   3 more (react-dom, @next/react-dev-overlay, styled-jsx)
npm ERR! 
npm ERR! Could not resolve dependency:
npm ERR! peer react@"^17.0.0" from use-subscription@1.5.0
npm ERR! node_modules/next/node_modules/use-subscription
npm ERR!   use-subscription@"1.5.0" from next@10.0.1
npm ERR!   node_modules/next
npm ERR!     next@"10.0.1" from the root project
npm ERR! 
npm ERR! Fix the upstream dependency conflict, or retry
npm ERR! this command with --force, or --legacy-peer-deps
npm ERR! to accept an incorrect (and potentially broken) dependency resolution.

@gaearon
Copy link

gaearon commented Nov 11, 2020

Btw the context for #18518 (comment) is that we'd like to make the requirement stricter and enforce that react and react-dom are both updated in the lockstep. Potentially making it a hard runtime error in the future if they mismatch. Because it's already leading to subtle breakages in practice.

That's unrelated to the use-subscription problem but I want to understand the consensus there.

@Timer
Copy link
Member

Timer commented Nov 11, 2020

Using --legacy-peer-deps works:

❯ npm i --legacy-peer-deps    
❯ npm ls react
test@ /Users/joe/Desktop/scratch2/test
├─┬ next@10.0.1
│ ├─┬ @next/react-dev-overlay@10.0.1
│ │ └── react@16.14.0 deduped
│ ├── react@16.14.0 deduped
│ ├─┬ styled-jsx@3.3.1
│ │ └── react@16.14.0 deduped
│ └─┬ use-subscription@1.5.0
│   └── react@16.14.0 deduped invalid
├─┬ react-dom@16.14.0
│ └── react@16.14.0 deduped
└── react@16.14.0

And doesn't result in a duplicated React version.

@billyjanitsch
Copy link

--force errors too:

@Timer that's a bug in the current version of npm that should be resolved soon. See npm/arborist#180 and npm/cli#2123. --force, once that issue is fixed, would generally the preferred approach over --legacy-peer-deps.

@Timer
Copy link
Member

Timer commented Nov 11, 2020

Would --force result in a duplicated copy of react being installed? Or forced deduping like legacy peer deps?

@Timer Timer added Upstream Related to using Next.js with a third-party dependency. (e.g., React, UI/icon libraries, etc.). and removed type: needs investigation labels Nov 11, 2020
@Timer Timer modified the milestones: iteration 12, 10.x.x Nov 11, 2020
@jaydenseric
Copy link
Contributor Author

@Timer

Ah, I see. OP's issue is misleading here

OP makes it sound like npm defaults to installing two copies of React.

No, when I was using npm v7 to install without any special flags it resulted in two versions of react in node_modules. Otherwise, how could I take this screenshot?

Screen Shot 2020-10-30 at 2 36 21 pm

@billyjanitsch
Copy link

@Timer forced deduping, preferring the version that is declared higher up. In this case that would generally be whatever version of React appears in package.json.

@Timer
Copy link
Member

Timer commented Nov 11, 2020

@jaydenseric what version of npm 7 was that on? I cannot reproduce it with 7.0.10 and any combination of flags. I figured you used a special command to get it to duplicate.

@billyjanitsch
Copy link

No, when I was using npm v7 to install without any special flags it resulted in two versions of react in node_modules. Otherwise, how could I take this screenshot?

@jaydenseric that doesn't seem right. npm 7 will outright refuse to run the installation, not even populating node_modules.

@billyjanitsch
Copy link

billyjanitsch commented Nov 11, 2020

Say we have react-dom@17.0.0 that peer-depends on exact react@17.0.0. Then we release react-dom@17.0.1 that peer-depends on exact react@17.0.1. Is this valid or does it constitute a breaking change?

@gaearon That's a great question. I haven't seen a lot of packages do this so I don't have a sense for the ecosystem convention around it. But I see why you'd want this for React. It might be a sufficiently grey area where the rules are unclear and React could just trailblaze? (To be clear, not speaking as an authority here at all, just relaying my sense of the ecosystem.)

React made a breaking change
While I understand the frustration

Btw, I'm sorry that my tone implied frustration; I absolutely didn't mean that. My understanding was that there was consensus so I didn't think to caveat that statement, but, in hindsight, I wish I'd written "React made what I consider a breaking change". I appreciate you looking at this and don't mean for it to be a source of stress. 💙

@jaydenseric
Copy link
Contributor Author

jaydenseric commented Nov 11, 2020

@Timer

@jaydenseric what version of npm 7 was that on? I cannot reproduce it with 7.0.10 and any combination of flags. I figured you used a special command to get it to duplicate.

I have been using every version of npm v7 as they come out, and can't remember which patch version I used when raising this issue. Maybe v7.0.3? I just tried v7.0.9 and got the same results you did. This doesn't reduce the urgency of a solution to this issue however, it makes it more critical to fix since npm install react@16 react-dom@16 next@10 won't work at all now.

@gaearon @billyjanitsch

Say we have react-dom@17.0.0 that peer-depends on exact react@17.0.0. Then we release react-dom@17.0.1 that peer-depends on exact react@17.0.1. Is this valid or does it constitute a breaking change?

For sure it's a breaking change. Just think about it; if a user does this (notice dependencies are pinned to exact versions without using ^):

 {
   "dependencies": {
     "react": "17.0.0",
-    "react-dom": "17.0.0"
+    "react-dom": "17.0.1"
   }
 }

Their project would break. But, it should be perfectly safe to bump a patch version of a single dependency.

In this hypothetical situation, the correct semver thing would have been to release react v17.0.1, and react-dom v18.0.0 that contains an updated react peer dependency of ^17.0.1. Realistically such a situation would not be expected for a react patch release, but is expected for a minor release with a new feature that react-dom starts using.

People have gotten used to react and react-dom sharing the same version numbers, but there are technical reasons that approach doesn't make sense.

@billyjanitsch
Copy link

For sure it's a breaking change. [...] Realistically such a situation would not be expected for a react patch release

@jaydenseric I think that's the dilemma that @gaearon is raising. It's less a question of "in the absolute strictest terms, does this meet the criteria for a breaking change?" and more like "is this grey enough that React could reasonably do it to avoid ridiculous numbers of major releases?" which IMO is less clear-cut.

But, it should be perfectly safe to bump a patch version of a single dependency.

I believe the idea is that this is not safe in general with React. They want to be able to do lockstep releases knowing that the relationship between the two packages might change, but expecting the consumer to keep them in sync. The alternative is the scenario you mentioned of requiring a major bump for nearly every change, which is ... not great.

@jaydenseric
Copy link
Contributor Author

@billyjanitsch

They want to be able to do lockstep releases knowing that the relationship between the two packages might change, but expecting the consumer to keep them in sync.

I hope for a future where all npm packages follow semver, instead of expecting users to be intimately familiar with their versioning idiosyncrasies. As far as I can see, the React docs doesn't even document the caveat that react and react-dom must be maintained in dependencies/lockfiles with exactly matching versions down to the patch.

The alternative is the scenario you mentioned of requiring a major bump for nearly every change, which is ... not great.

If its a major change, publish a major version! That is semver, and that is great. The alternative is a broken ecosystem, like has resulted right now in the issue we are discussing. No one cares if your major version becomes double digits, they just want stable packages that follow ecosystem standards.

@gaearon
Copy link

gaearon commented Nov 11, 2020

I think we need to be careful not to step into semver pedantism here. Semver is a social contract, not a technical one. It is always possible to write code that would technically break in every patch. That’s not a reason to label every release as major.

@billyjanitsch
Copy link

No one cares if your major version becomes double digits

@jaydenseric I don't think that's a fair characterization of the downside of this approach. Consider how long it will take all third-party packages to add ^17.0.0 to their peer dep ranges and cut a release. And consider that with npm@7 you can't update React until all of your third-party dependents have done this. Now imagine this whole process needing to happen, say, 20-50x as often.

IMO it's hard to argue that this is worth it, on balance.

@gaearon
Copy link

gaearon commented Nov 12, 2020

The OP problem should be resolved with use-subscription@1.5.1. Thanks @billyjanitsch and @jaydenseric for reporting.

@jaydenseric
Copy link
Contributor Author

Thanks for the patch @gaearon !

Now we run into a long time frustration about next; it is one of the few packages published to npm that pins all the dependencies to exact versions (an antipattern), so we can't benefit from any upstream fixes until a new version of next is published with updated dependencies:

"use-subscription": "1.5.0",

@Timer
Copy link
Member

Timer commented Nov 12, 2020

Fixed in ^10.0.2-canary.11!

@balazsorban44
Copy link
Member

This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@vercel vercel locked as resolved and limited conversation to collaborators Jan 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Upstream Related to using Next.js with a third-party dependency. (e.g., React, UI/icon libraries, etc.).
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants