-
Notifications
You must be signed in to change notification settings - Fork 81
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
Add support for NPM 8, 9, 10 and PNPM 8, remove support for NPM 6 #235
Add support for NPM 8, 9, 10 and PNPM 8, remove support for NPM 6 #235
Conversation
dfef485
to
2e9839f
Compare
# unfortunataly, npm >= 7 ignores dependencies of local dependencies to handle them the same way as normal | ||
# dependencies we require to use install-links which will create a tar ball for all packages and install | ||
# it like a normal dependency with its dependencies | ||
install-links=true | ||
|
||
# pnpm does not install dependencies of local dependencies correctly by default but if use the shamefully-hoist | ||
# configuration it also installs them correctly and can so be correctly build via pnpm | ||
shamefully-hoist=true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This are the main changes required for newer NPM version and for using PNPM
bdec5d9
to
f908c40
Compare
@alexander-schranz how will development work change with the missing watch task? Always reinstall and build? Can that be done with some kind of automation? |
Developing in core would be easier for most when just work in the Developing of third party bundles is more an issue, normally npm install # install all dependencies
rm -rf node_modules/sulu-admin-bundle
ln -s @sulu/vendor/sulu/sulu/src/Sulu/Bundle/AdminBundle/Resources/js node_modules/sulu-admin-bundle We could provide a script doing this mechanism something like |
f908c40
to
c9bf9d8
Compare
mysql-version: '8.0' | ||
php-extensions: 'ctype, iconv, intl, mysql, pdo_mysql, php_fileinfo, imagick' | ||
php-extensions: 'ctype, iconv, intl, mysql, pdo_mysql, php_fileinfo, gd, sodium' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's in this PR?
Add support for NPM 8, 9, 10 and PNPM 8, remove support for NPM 6
Why?
Node 16 is going for end of life: https://nodejs.org/en/about/previous-releases
Example Usage
BC Breaks/Deprecations
No support for NPM 6 / symlinked local packages.
Disadvantages
No watch task over vendor package developments as install requires
install-links=true
which ends not longer uses symlinks. Instead a tarball is created and install, but it installs correctly dependencies of local dependencies. Which does not work forfile:
packages.Why this opens a wider range for support of modern npm and pnpm versions.
Yarn
is still a problem and not supported by sulu. This includes not only yarn 1 but also 2,3,4.