-
Notifications
You must be signed in to change notification settings - Fork 27.7k
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
Comments
I have exactly the same problem with same setup. |
I have the same issue here |
A bit of additional context: React made a breaking change between I reported the upstream issue here: facebook/react#20224. Hopefully it gets fixed by way of releasing a version of cc @timneutkens who authored the dependency upgrade PR. |
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 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. |
Where can I read about the auto-installing and whether this decision in npm is final? |
Actually I'm not sure I understand.. Is it being auto-installed as a duplicate, or is installation refused? |
@gaearon see https://github.com/npm/rfcs/blob/latest/implemented/0025-install-peer-deps.md Funny that they call this out:
It seems like packages can opt-out of the auto-installing peer dep behavior by using {
"peerDependencies": {
"react": "17.x.x"
},
"peerDependenciesMeta": {
"react": {
"autoinstall": false
}
}
} |
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.
The installation is refused. In
The decision seems final. It was proposed as an RFC in March (npm/rfcs#43) and implemented shortly after. It was released in the |
@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 |
Say we have |
@gaearon FYI here's an example of what happens if you try an install that would result in multiple versions of React:
|
Ah, I see. OP's issue is misleading here:
OP makes it sound like npm defaults to installing two copies of React. npm 7 by default will refuse to install:
|
Btw the context for #18518 (comment) is that we'd like to make the requirement stricter and enforce that That's unrelated to the |
Using
And doesn't result in a duplicated React version. |
@Timer that's a bug in the current version of npm that should be resolved soon. See npm/arborist#180 and npm/cli#2123. |
Would |
No, when I was using npm v7 to install without any special flags it resulted in two versions of |
@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. |
@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. |
@jaydenseric that doesn't seem right. npm 7 will outright refuse to run the installation, not even populating node_modules. |
@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.)
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. 💙 |
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
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 People have gotten used to |
@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.
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. |
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
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. |
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. |
@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 IMO it's hard to argue that this is worth it, on balance. |
The OP problem should be resolved with |
Thanks for the patch @gaearon ! Now we run into a long time frustration about next.js/packages/next/package.json Line 117 in 3c33794
|
Fixed in |
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. |
Bug report
Describe the bug
next
v10.0.0 forcesreact
v17 to be installed, even if the project hasreact
v16 in dependencies.This is because
next
v10.0.0 has ause-subscription
v1.5.0 dependency pinned:next.js/packages/next/package.json
Line 117 in 118ab79
use-subscription
v1.5.0 has areact
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 theuse-subscription
peer dependency onreact
is not satisfied, but it would not installreact
v17. While this may seem ok, it probably is not becauseuse-subscription
declaredreact
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, andreact
v17 to fulfill theuse-subscription
peer dependency. The result is that multiple versions ofreact
get installed, and this can cause a React render error that multiple instances of React are being used with hooks.To Reproduce
npm ls react
to see the multiple versions installed. This in itself is bad.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 ofreact
installed in the user's project, and not cause multiple versions ofreact
to be installed.Screenshots
System information
The text was updated successfully, but these errors were encountered: