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

npm 7 fails while resolving dependencies before validating supported engines #133

Closed
lubomirfiala opened this issue Aug 18, 2021 · 5 comments

Comments

@lubomirfiala
Copy link

I had to build custom JS for admin with sulu:admin:update-build but dependency error was thrown.
After some debuging I found that my version of NPM wasn't compatible.
sulu:admin:update-build should check for engines compatibility before building dependencies and throw error.

@alexander-schranz
Copy link
Member

alexander-schranz commented Aug 18, 2021

@lubomirfiala The engines compatibility are not required for running sulu:admin:update-build. If you have a normal sulu setup the sulu:admin:update-build will update files in the assets/admin directory and then download the build from the tagged and tested version from github into your public/build/admin directory. Only when there are customization done which we only know after answering multiple questions it will ask you if you want to do a manual build and only there npm is needed and only there an error will then thrown. So there is no need to check the compatiblity before as it is not required to have a compatible npm version installed before the manual build.

@alexander-schranz
Copy link
Member

In your case you can also skip the manual build by answering that question with n and then us docker as an alternative way to build your admin js with node if you really have done js customizations: https://docs.sulu.io/en/2.3/cookbook/build-admin-frontend.html#solution-2-build-manually-with-docker

@alexander-schranz
Copy link
Member

dependency error was thrown.

If you are getting a dependency error before the engines are checked this sounds more like an issue on the NPM side that they are checking first dependency before they are checking the defined engines.

@niklasnatter
Copy link
Contributor

I just tested the sulu:admin:update-build command on a machine with npm 7. Unfortunately, it looks like npm treis to resolve the dependency tree before validating the engines section of the pacakge.json 😕
Because of this, I see the following error when running bin/console sulu:admin:update-build:

npm ERR! code ERESOLVE
npm ERR! ERESOLVE unable to resolve dependency tree
npm ERR! 
npm ERR! While resolving: sulu-skeleton@undefined
npm ERR! Found: react@17.0.2
npm ERR! node_modules/react
npm ERR!   react@"^17.0.0" from the root project
npm ERR! 
npm ERR! Could not resolve dependency:
npm ERR! peer react@"^0.13.0 || ^0.14.0 || ^15.0.0 || ^16.0.0" from mobx-react@5.4.4
npm ERR! node_modules/mobx-react
npm ERR!   mobx-react@"^5.0.0" 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.

When I add the --legacy-peer-deps option to the npm install command, I see the expected error message again:

npm ERR! code EBADENGINE
npm ERR! engine Unsupported engine
npm ERR! engine Not compatible with your version of node/npm: undefined
npm ERR! notsup Not compatible with your version of node/npm: undefined
npm ERR! notsup Required: {"node":">=12","npm":">=6 <7"}
npm ERR! notsup Actual:   {"npm":"7.21.1","node":"v14.17.0"}

@niklasnatter
Copy link
Contributor

We decided to enable the legacy-peer-deps setting in the assets/admin/.npmrc file per default to prevent the dependency tree error message when using npm 7 (see sulu/sulu#6201). Thanks for creating this issue!

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

3 participants