decaporg / decap-cms

A Git-based CMS for Static Site Generators
https://decapcms.org
MIT License
17.84k stars 3.04k forks source link

netlify-cms-app fails to install with react 17 as version 6 of react-aria-menubutton in incompatible with react 17 #5376

Closed something-something-something closed 1 month ago

something-something-something commented 3 years ago

Describe the bug

When installing netlify-cms-app with react 17 the install fails with the following error:

npm ERR! code ERESOLVE
npm ERR! ERESOLVE unable to resolve dependency tree
npm ERR! 
npm ERR! Found: react@17.0.2
npm ERR! node_modules/react
npm ERR!   peer react@"17.0.2" from react-dom@17.0.2
npm ERR!   node_modules/react-dom
npm ERR!     react-dom@"^17.0.2" from the root project
npm ERR!     peer react-dom@"^16.8.4 || ^17.0.0" from netlify-cms-app@2.15.6
npm ERR!     node_modules/netlify-cms-app
npm ERR!       netlify-cms-app@"*" from the root project
npm ERR!     8 more (netlify-cms-widget-date, ...)
npm ERR!   react@"^17.0.2" from the root project
npm ERR!   38 more (netlify-cms-app, @emotion/core, @emotion/styled, ...)
npm ERR! 
npm ERR! Could not resolve dependency:
npm ERR! peer react@"0.14.x || ^15.0.0 || ^16.0.0" from react-aria-menubutton@6.3.0
npm ERR! node_modules/netlify-cms-ui-default/node_modules/react-aria-menubutton
npm ERR!   react-aria-menubutton@"^6.0.0" from netlify-cms-ui-default@2.13.1
npm ERR!   node_modules/netlify-cms-ui-default
npm ERR!     netlify-cms-ui-default@"^2.13.1" from netlify-cms-app@2.15.6
npm ERR!     node_modules/netlify-cms-app
npm ERR!       netlify-cms-app@"*" from the root project
npm ERR!     23 more (netlify-cms-backend-azure, ...)
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!

It seems like react-aria-menubutton in netlify-cms-ui-default needs to be upgraded to version 7.0.1 see: https://github.com/davidtheclark/react-aria-menubutton/issues/140

To Reproduce

Run npm init -y && npm i react react-dom && npm i netlify-cms-app in an empty directory. See Error

Expected behavior

netlify-cms-app installs without having to use --legacy-peer-deps

Screenshots

NA

Applicable Versions:

CMS configuration

NA

Additional context

Seems like a dependency that didn't get updated while fixing https://github.com/netlify/netlify-cms/issues/5111

erezrokah commented 3 years ago

Thanks @something-something-something, this is also an issue with react-split-pane: https://github.com/tomkp/react-split-pane/pull/681 https://netlifycms.slack.com/archives/CPRR0PQ9E/p1620817591329300

erezrokah commented 3 years ago

Blocked by https://github.com/davidtheclark/react-aria-menubutton/pull/133

shirshendubhowmick commented 3 years ago

Blocked by davidtheclark/react-aria-menubutton#133

This is now complete

erezrokah commented 3 years ago

Woo @shirshendubhowmick, this is amazing. Thank you for the quick follow up ❤️

paulobarcelos commented 3 years ago

It seems the react-codemirror nested dependency could also be blocking this? This is the error I get when trying to update to React 17:

npm ERR! code ERESOLVE
npm ERR! ERESOLVE unable to resolve dependency tree
npm ERR!
npm ERR! Found: react@17.0.2
npm ERR! node_modules/react
npm ERR!   react@"^17.0.2" from the root project
npm ERR!   peer react@">=16.8 || ^17.0.0" from framer-motion@4.1.17
npm ERR!   node_modules/framer-motion
npm ERR!     framer-motion@"^4.1.17" from the root project
npm ERR!   45 more (react-awesome-styled-grid, react-dnd, react-dom, ...)
npm ERR!
npm ERR! Could not resolve dependency:
npm ERR! peer react@">=15.5 <=16.x" from react-codemirror2@7.2.1
npm ERR! node_modules/netlify-cms-widget-code/node_modules/react-codemirror2
npm ERR!   react-codemirror2@"^7.0.0" from netlify-cms-widget-code@1.3.2
npm ERR!   node_modules/netlify-cms-widget-code
npm ERR!     netlify-cms-widget-code@"^1.3.2" from netlify-cms-app@2.15.11
npm ERR!     node_modules/netlify-cms-app
npm ERR!       netlify-cms-app@"^2.15.11" from the root project
npm ERR!
erezrokah commented 3 years ago

Thanks @paulobarcelos, that's helpful. Looks like there's a PR for it https://github.com/scniro/react-codemirror2/pull/224, but no comment from the maintainer.

paulobarcelos commented 3 years ago

Thanks @paulobarcelos, that's helpful. Looks like there's a PR for it scniro/react-codemirror2#224, but no comment from the maintainer.

@erezrokah, by the looks of it, seems that project may be dead. Wonder if it would be a good idea for Netlifly CMS to change to the proposed updated fork while the original one doesn't change (if it ever will). Feels like halting support for React 17 is a bigger issue than dealing with the annoyance of having to migrate to an unofficial fork of the dependency.

erezrokah commented 3 years ago

That fork doesn't seem to be updating frequently too and I don't think we want to install directly from GitHub.

For React 17 + npm@7 support there's a workaround by passing the --legacy-peer-deps flag, which is clearly stated in the error.

Not sure if we should find a completely different solution to our Code widget.

Since this seems to be an issue with more that one dependency, we might have to think of a solution that doesn't involve chasing down maintainers.

I wonder if there's a way we can update dependencies package.json files on the CMS side to allow users to install it.

LuisOsta commented 2 years ago

I get a similar error when trying to install the package netlify-cms-app:

npm ERR! Could not resolve dependency:
npm ERR! peer react@"^16.0.0-0" from react-split-pane@0.1.92
npm ERR! node_modules/netlify-cms-app/node_modules/react-split-pane
npm ERR!   react-split-pane@"^0.1.85" from netlify-cms-core@2.54.0
npm ERR!   node_modules/netlify-cms-app/node_modules/netlify-cms-core
npm ERR!     netlify-cms-core@"^2.54.0" from netlify-cms-app@2.15.58
npm ERR!     node_modules/netlify-cms-app
npm ERR!       netlify-cms-app@"*" from the root project
npm ERR! 

It seems that react-split-plane may be dead or at least poorly maintained as the last commit was in May. There's a highly commented issue for the react 17 upgrade - https://github.com/tomkp/react-split-pane/issues/713 and a PR for it - https://github.com/tomkp/react-split-pane/pull/681. With the maintainer having left no comments on either.

Is there a way we can move forward without using --legacy-peer-deps, as that often causes its own headaches that I'd love to avoid

JeremyGrieshop commented 2 years ago

It seems that react-split-plane may be dead or at least poorly maintained as the last commit was in May. There's a highly commented issue for the react 17 upgrade - tomkp/react-split-pane#713 and a PR for it - tomkp/react-split-pane#681. With the maintainer having left no comments on either.

After waiting for months, I realized the whole project was only a couple of MIT-licensed files, so I just copied them into my project and was done with it.

LuisOsta commented 2 years ago

@JeremyGrieshop How did you integrate the copied files into the package?

JeremyGrieshop commented 2 years ago

@JeremyGrieshop How did you integrate the copied files into the package?

I don't have a public repository to share at this moment, but I just copied these 4 files into a local directory, called SplitPane in my project and imported from it instead of react-split-pane:

https://github.com/tomkp/react-split-pane/tree/master/src

It does, at this time, require a polyfil, so you'll need to add react-lifecycles-compat as a dependency for now. If these are ever converted over to class-less components/hooks, then it won't be necessary. It also required react-style-proptype, but I just removed that code from the appropriate classes, since I didn't care about the prop types. Otherwise, you'd need to add that dependency as well.

LuisOsta commented 2 years ago

@erezrokah would it be possible to follow what this user had commented on the PR in react-split-pane - https://github.com/tomkp/react-split-pane/pull/681#issuecomment-1000543520 and simply copy the needed components into the internals of Netlify CMS with the right license?

JeremyGrieshop commented 2 years ago

Here's an example I quickly pushed to my personal repository that someone could copy into their own SplitPane folder. I converted over some of the older class dependencies and made them functional components that use hooks instead:

https://github.com/JeremyGrieshop/react-split-pane

erezrokah commented 2 years ago

Hi everyone 👋 At this point I think we will accept a contribution that inlines react-split-pane as it seems there won't be a any progress with https://github.com/tomkp/react-split-pane/pull/681

LuisOsta commented 2 years ago

@erezrokah Sounds great! I'm working on that right now in this fork https://github.com/LuisOsta/netlify-cms based on the version created by @JeremyGrieshop.

Btw Jeremy let me know if the attribution I included in the commit looks good to you (I included the README and a link to your repository)

LuisOsta commented 2 years ago

@erezrokah If you could take a look when you have the time that'd be much appreciated

LuisOsta commented 2 years ago

Aside from react-split-pane are there any remaining packages that are causing React 17 compatibility issue? What's left for us to be able to definitely close this issue?

JeremyGrieshop commented 2 years ago

Btw Jeremy let me know if the attribution I included in the commit looks good to you (I included the README and a link to your repository)

Looks great! I hope you find it useful and that I didn't leave anything out of the original work.

martinjagodic commented 1 month ago

This seems to be fixed