search

sulu / skeleton

Project template for starting your new project based on the Sulu content management system
https://sulu.io
MIT License
261 stars 81 forks source link

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

Closed lubomirfiala closed 3 years ago

lubomirfiala commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 https://github.com/sulu/sulu/pull/6201). Thanks for creating this issue!