Javascript setup is not compatible with npm 7 / 8

[niklasnatter]

The javascript setup in the assets/admin folder uses the file:... syntax to require Sulu packages such as the sulu-admin-bundle: https://github.com/sulu/skeleton/blob/c9640ee7dae09f09a84d83fce0b5143d52fc4a84/assets/admin/package.json#L15-L28

Dependencies that are required like this will be symlinked into the assets/admin/node_moules directory. Unfortunately, npm v7 does not install the dependencies of these dependencies at the moment: https://github.com/npm/cli/issues/2339

Because of this, the dependencies of the Sulu bundles are not installed and the build fails with a lot of error messages like this:

ERROR in /sulu-skeleton/vendor/sulu/sulu/src/Sulu/Bundle/AdminBundle/Resources/js/components/Tabs/Tabs.js
Module not found: Error: Can't resolve 'classnames' in '/sulu-skeleton/vendor/sulu/sulu/src/Sulu/Bundle/AdminBundle/Resources/js/components/Tabs'
 @ /sulu-skeleton/vendor/sulu/sulu/src/Sulu/Bundle/AdminBundle/Resources/js/components/Tabs/Tabs.js 54:0-36 347:22-32
 @ /sulu-skeleton/vendor/sulu/sulu/src/Sulu/Bundle/AdminBundle/Resources/js/components/Tabs/index.js
 @ /sulu-skeleton/vendor/sulu/sulu/src/Sulu/Bundle/AdminBundle/Resources/js/components/index.js
 @ /sulu-skeleton/vendor/sulu/sulu/src/Sulu/Bundle/ContactBundle/Resources/js/components/ContactDetails/Phone.js
 @ /sulu-skeleton/vendor/sulu/sulu/src/Sulu/Bundle/ContactBundle/Resources/js/index.js
 @ ./index.js

[niklasnatter]

Fixed by https://github.com/sulu/sulu/pull/5859 and https://github.com/sulu/skeleton/pull/96 🎉
To understand why the fix works, the following comment that was added in https://github.com/sulu/sulu/pull/5859 in the symlink-vendor-directory.js might be helpful:

// the sulu/skeleton uses the assets/admin directory as root directory for the administration interface javascript
// application. this application imports the sulu/sulu code by requiring the bundles from the the vendor directory
// in its package.json.
//
// unfortunately this setup causes various problems in the javascript ecosystem because the assets/admin directory
// is not an ancestor of the vendor/sulu/sulu directory which is used to require the code of bundles. for example,
// npm 7 does not install the dependencies of the bundle packages (https://github.com/npm/cli/issues/2339) and older
// npm versions are not able to dedupe packages that are required by a bundle and in the assets/admin directory.
// because of this, packages that were not deduped might be included in the webpack build multiple times. this
// increases the size of the build and will lead to an error in case of the @ckeditor packages:
// http://ckeditor.com/docs/ckeditor5/latest/framework/guides/support/error-codes.html#error-ckeditor-duplicated-modules
//
// to prevent these problems, this file creates a assets/admin/node_modules/node_modules/@sulu/vendor symlink and we
// use the symlinked vendor directory to require the bundles in the assets/admin/package.json.
// this makes the directory that is used for requiring the bundles a descendant of the assets/admin directory and
// allows npm to correctly dedupe the installed packages.

[alexander-schranz]

Damn seems like it doesn't actually work. Really not sure why but currently tested by me and @chirimoya and the packages of the subpackages where not installed. Not sure why or if there was maybe again a change in npm or node, but with current:

[ 0s064 | Mär 10 06:52pm ] [ 0 ]
~/D/S/s/a/admin (2.x|✚5…) $ npm --version
7.6.2

[ 0s881 | Mär 10 06:52pm ] [ 0 ]
~/D/S/s/a/admin (2.x|✚5…) $ node --version
v15.11.0

it didn't work for me 😢 😠 😡 . So it seems like I did only fix the ckeditor issue and windows build not the npm 😞

[niklasnatter]

Thanks for reopening - thats really sad to hear 😢 Maybe we need to add the symlink outside of the node_modules folder for npm7 compatibility? I could imagine that npm7 does not install the dependencies if a package is inside of the node_modules folder 🤔

[alexander-schranz]

Did test it yesterday but outside node_modules didn't work for me. Not sure why it did work sometime 🙈

[niklasnatter]

Okay thats a pity - I guess we need to hope that https://github.com/npm/cli/issues/2339 gets resolved soon then 😕

[madwizard-thomas]

For me it does seem to work if the symlink is outside node_modules, but inside the admin dir. So using ../../vendor/sulu/.. does not resolve dependencies, neither does the now default symlink in node_modules/@sulu/vendor/...

However if I symlink assets/admin/sulu to ../../vendor/sulu/sulu and use file:sulu/src/Sulu/Bundle'... as dependency paths it does work.

For npm 7 it would also work to just add the list of sulu module directories as an array to the workspaces section of package.json and remove them from the dependencies section (still requires a symlink in trhe admin dir though). This will also resolve the dependencies. It will also break on incompatible react version peer dependencies (npm 7 installs peer dependencies by default and will complain about conflicts). That may need to be resolved but with npm install --legacy-peer-deps it does work.

[madwizard-thomas]

One issue I'm still having with this setup is that if the node_modules directory in the root exists, the symlinked package will resolve modules from this directory. The build will succeed but the code won't actually work (wrong version of path-to-regexp causes errors for example). If it doesn't exists everything works fine. I think this is caused by npm using the resolved path (realpath) of the symlinks to search the modules.

[niklasnatter]

Hey, thanks a lot for spending your time on this. We really appreciate your help here!

For me it does seem to work if the symlink is outside node_modules, but inside the admin dir. So using ../../vendor/sulu/.. does not resolve dependencies, neither does the now default symlink in node_modules/@sulu/vendor/...

However if I symlink assets/admin/sulu to ../../vendor/sulu/sulu and use file:sulu/src/Sulu/Bundle'... as dependency paths it does work.

We tried this initially, but we decided against a symlink outside of the node_modules folder because of DX. When we tested this, it broke the autocompletion/refactoring capabilities of our IDE because the IDE indexed the content of the assets/admin/sulu symlink as part of the source code. Placing the symlink in the node_modules solved this because the IDE treated the symlinked file as dependency code.

For npm 7 it would also work to just add the list of sulu module directories as an array to the workspaces section of package.json and remove them from the dependencies section (still requires a symlink in trhe admin dir though). This will also resolve the dependencies.

This is really good to know, thanks! Unfortunately this setup will not work with npm 6, right? I think I would prefer such a setup as soon as there is a Node.js LTS release that includes npm 7 per default.

One issue I'm still having with this setup is that if the node_modules directory in the root exists, the symlinked package will resolve modules from this directory. The build will succeed but the code won't actually work (wrong version of path-to-regexp causes errors for example). If it doesn't exists everything works fine.

Do you mean the root of the project? Who creates this node_modules folder in the root? I think this problem exists with the current setup in the repository too. As far as I understand, the problem is related to how webpack resolves dependencies (relative to the file that is processed). Unfortunately, I think that this is a quirk with our setup/project structure and we cannot do anything here.

[madwizard-thomas]

We tried this initially, but we decided against a symlink outside of the node_modules folder because of DX.

I see!

Unfortunately this setup will not work with npm 6, right?

Yes

Do you mean the root of the project? Who creates this node_modules folder in the root?

Yes, this is the node_modules used for the website itself (webpack/encore).

As far as I understand, the problem is related to how webpack resolves dependencies (relative to the file that is processed). Unfortunately, I think that this is a quirk with our setup/project structure and we cannot do anything here.

It is the module resolution that node use, there doesn't seem to be much to influence that. I can't see any workarounds either other than building the admin separately (or with docker for example).

[niklasnatter]

Yes, this is the node_modules used for the website itself (webpack/encore). It is the module resolution that node use, there doesn't seem to be much to influence that. I can't see any workarounds either other than building the admin separately (or with docker for example).

I think you are right 😕
We configure webpack/encore to use the assets/website folder instead of the project root in our projects. There is a small guide in the documentation how to do this (hopefully it is still up-to-date, havent used this in the last months): https://docs.sulu.io/en/2.2/cookbook/webpack-encore.html

[alexander-schranz]

Sadly custom build still don't work under npm 7/8. The related issue https://github.com/npm/cli/issues/2339 still exists.

[AndreasA]

@alexander-schranz Will NPM actually fix this because I am not that sure. Maybe you should implement a separate solution instead after all the issue is over a year old and it makes development more tricky due to admin/website might require different NPM versions

[alexander-schranz]

@AndreasA there is already in discussion to use module federation or ddl builds so every bundle provides build version of the code itself and not a project need to compile also the code of the bundles.

See also https://github.com/sulu/sulu/issues/6343#issuecomment-999633060

To test that out we first need upgrade to webpack 5. Currently ckeditor was the blocking point there. They fixed that with the latest release 32.0.0 release.

At current state if you need to switch npm versions you maybe want to create .nvmrc in your project to achieve that the correct version is used.

[alexander-schranz]

There was an update on the linked issue: https://github.com/npm/cli/issues/2339#issuecomment-1111228605

So it is now possible to install the dependencies with npm 8.8.0. Sadly there seems to be no way to support npm 6 and npm 8.8.

NPM 6 requires that we include our dependencies over a local child:

• file:node_modules/@sulu/vendor/sulu/sulu/src/Sulu/Bundle/AdminBundle/Resources/js

NPM 8.8 seems not to like that and requires an include over:

• file:../../vendor/sulu/sulu/src/Sulu/Bundle/AdminBundle/Resources/js

One change which effects our development process NPM 6 did install the dependencies as symlinks and make so development easier. NPM 8.8 will pack the dependency and install a copy of it, which will make for us the sulu development harder.

Still it seems the package installation work. Node 18 or 17 seems not be compatible with webpack 4, both errors in:

node:internal/crypto/hash:67 this[kHandle] = new _Hash(algorithm, xofLen);

> build
> webpack --mode production

node:internal/crypto/hash:67
  this[kHandle] = new _Hash(algorithm, xofLen);
                  ^

Error: error:0308010C:digital envelope routines::unsupported
    at new Hash (node:internal/crypto/hash:67:19)
    at Object.createHash (node:crypto:133:10)
    at module.exports (/Users/alexanderschranz/Documents/Projects/sulu-develop.localhost/assets/admin/node_modules/webpack/lib/util/createHash.js:135:53)
    at NormalModule._initBuildHash (/Users/alexanderschranz/Documents/Projects/sulu-develop.localhost/assets/admin/node_modules/webpack/lib/NormalModule.js:417:16)
    at handleParseError (/Users/alexanderschranz/Documents/Projects/sulu-develop.localhost/assets/admin/node_modules/webpack/lib/NormalModule.js:471:10)
    at /Users/alexanderschranz/Documents/Projects/sulu-develop.localhost/assets/admin/node_modules/webpack/lib/NormalModule.js:503:5
    at /Users/alexanderschranz/Documents/Projects/sulu-develop.localhost/assets/admin/node_modules/webpack/lib/NormalModule.js:358:12
    at /Users/alexanderschranz/Documents/Projects/sulu-develop.localhost/assets/admin/node_modules/loader-runner/lib/LoaderRunner.js:373:3
    at iterateNormalLoaders (/Users/alexanderschranz/Documents/Projects/sulu-develop.localhost/assets/admin/node_modules/loader-runner/lib/LoaderRunner.js:214:10)
    at iterateNormalLoaders (/Users/alexanderschranz/Documents/Projects/sulu-develop.localhost/assets/admin/node_modules/loader-runner/lib/LoaderRunner.js:221:10)
    at /Users/alexanderschranz/Documents/Projects/sulu-develop.localhost/assets/admin/node_modules/loader-runner/lib/LoaderRunner.js:236:3
    at context.callback (/Users/alexanderschranz/Documents/Projects/sulu-develop.localhost/assets/admin/node_modules/loader-runner/lib/LoaderRunner.js:111:13)
    at /Users/alexanderschranz/Documents/Projects/sulu-develop.localhost/assets/admin/node_modules/babel-loader/lib/index.js:59:71 {
  opensslErrorStack: [ 'error:03000086:digital envelope routines::initialization error' ],
  library: 'digital envelope routines',
  reason: 'unsupported',
  code: 'ERR_OSSL_EVP_UNSUPPORTED'
}

Node.js v18.0.0

Node 16 (current LTS) with npm 8.8 seems to work.

Solutions for Node 17 / 18 problems seems to update to webpack 5 or use the workaround here: https://github.com/webpack/webpack/issues/13572#issuecomment-923736472 which I tested and did work when adding that lines to sulu webpack config. The major change here is that it will change the hash algorithm and so generate a completely new files also for all static content (fonts, images, ...) which did not change.

[alexander-schranz]

I tried NPM 9 where the --install-links is now default.

Still it seems the package installation work. Node 18 or 17 seems not be compatible with webpack 4, both errors in: node:internal/crypto/hash:67 this[kHandle] = new _Hash(algorithm, xofLen);

This issue still exist but, there seems currently workaround for this when doing:

export NODE_OPTIONS=--openssl-legacy-provider

Still npm 8.8 and 9 does not create symlinks which make developing sulu itself only possible inside the sulu/sulu package itself not outside of it. Like written above it is not possible to be compatible to npm 6, 7, 8 and 8.8, 9. So we would force that versions.

So best solution still would be update to webpack 5 and introducing the webpack 5 module federations. Which would also remove the whole dependencies symlink issues.

[alexander-schranz]

I forgot to add I also tried https://github.com/mweststrate/relative-deps package from mweststrate the creator of mobx. Sadly this does also not work as it seems require yarn, versions in package + that the package is published on npm registry. Which make it not usable for us to work with local dependencies.