Update Node.js to v16 and update frontend dependencies #519
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
also changed shell to bash We use nvm to install Node.js as nodesource distributions somehow always install Node.js 18 instead (see nodesource/distributions#1583)
Also alias the default version If you want to see the full output during Docker build, you can use the "--progress=plain" option.
|
With this, I am getting (after I nuked all existing volumes and images and rebuilt everything) the following error from webpacker: A similar message is also in the logs for experimental. |
|
Thanks, just stumbled on this too. We still need |
|
After updating This is probably related to rails/webpacker#2969 |
This is because we still use webpacker v4 currently. We will upgrade this depenedency when we switch from @rails/webpacker to webpack in the future.
This comment was marked as off-topic.
This comment was marked as off-topic.
This is so that Node.js is not installed again, which would result in v18, which we don't want at the moment.
This ensures that the app user can access node that was previously installed with nvm in /root/.nvm
|
Hey @christian-heusel, it'd be great if you could take a look at these changes and give a review as you have created the original Dockerfile ☺ |
christian-heusel
suggested changes
Jun 22, 2023
There was a problem hiding this comment.
Hey @christian-heusel, it'd be great if you could take a look at these changes and give a review as you have created the original Dockerfile
☺️
That is not correct, I have just heavily modified the setup!
4b5ca69 adds the dockerfile and was not done by me 😊
Anyways, I have left some comments about stuff that could be improved in my opinion, although I fear I am of no much help here 😄
Review is based on source code only, no actual testing has been done by me (yet).
Co-authored-by: Christian Heusel <christian@heusel.eu>
Oh, sorry ;) @christian-heusel Thanks a lot for your feedback!
Definitely not the case |
- Replaced Yarn installation with "corepack", a new binary shipped with Node.js starting with v16. This also means that Yarn is upgraded from v1 to v2 with a different CLI, e.g. we now use "yarn workspaces focus" instead of "yarn install" to install the dependencies for the workspace in the current working directory. - Update GPG key handling during PostgreSQL installation - Added some more comments to the Dockerfile - Moved copying of Node.js up in the Dockerfile More yarn-related changed in subsequent commits.
- Added a fix so that corepack really installs the version we want and not some other old version - with Yarn2+, building curerntly fails, so we switch back to Yarn1 and deal with an upgrade of Yarn in the future
Yarn1 does not generate this folder
"unnecessary" means dependencies that were added when trying out Yarn2+
christian-heusel
suggested changes
Jun 26, 2023
Co-authored-by: Christian Heusel <christian@heusel.eu>
(in prod Dockerfile)
christian-heusel
approved these changes
Jun 26, 2023
|
@christian-heusel Thanks a lot for reviewing this PR 👍 |
Splines
added a commit
that referenced
this pull request
Jun 27, 2023
* Use npm audit to update npm packages * Use Node.js version 16 in Docker development also changed shell to bash We use nvm to install Node.js as nodesource distributions somehow always install Node.js 18 instead (see nodesource/distributions#1583) * Use node after installation * Split installation of Node.js to multiple RUN commands Also alias the default version If you want to see the full output during Docker build, you can use the "--progress=plain" option. * Uniy sass packages and update yarn lock * Reflect changes in production and test Dockerfiles * Add comments to Dockerfile and tighten run commands * Add sass-loader back as dependency * Switch back to old version of sass-loader This is because we still use webpacker v4 currently. We will upgrade this depenedency when we switch from @rails/webpacker to webpack in the future. * Use "-no-install-recommends" for yarn install in prod This is so that Node.js is not installed again, which would result in v18, which we don't want at the moment. * Use consistent ${} syntax in Dockerfile & fix Node path * Copy Node.js over to /usr/local This ensures that the app user can access node that was previously installed with nvm in /root/.nvm * Apply quote arguments suggestion (in code review) Co-authored-by: Christian Heusel <christian@heusel.eu> * Quote arguments in other Dockerfiles as well * Replace deprecated apt-key and improve Dockerfiles - Replaced Yarn installation with "corepack", a new binary shipped with Node.js starting with v16. This also means that Yarn is upgraded from v1 to v2 with a different CLI, e.g. we now use "yarn workspaces focus" instead of "yarn install" to install the dependencies for the workspace in the current working directory. - Update GPG key handling during PostgreSQL installation - Added some more comments to the Dockerfile - Moved copying of Node.js up in the Dockerfile More yarn-related changed in subsequent commits. * Upgrade to Yarn 2 and add packages for non-error webpack build * Copy Yarn to usr folder alongside other Node.js tooling * Activate Yarn for app user * Only copy node over to usr folder * Go back to Yarn1 and explicitly set Yarn version - Added a fix so that corepack really installs the version we want and not some other old version - with Yarn2+, building curerntly fails, so we switch back to Yarn1 and deal with an upgrade of Yarn in the future * Remove ".yarn" from .gitignore Yarn1 does not generate this folder * Remove unnecessary dependencies from package.json "unnecessary" means dependencies that were added when trying out Yarn2+ * Add back missing "--production=false" to "yarn install" * Use "ruby:3.1.4-bullseye" to have PostgreSQL available * Add missing `apt update` in dev and test Dockerfiles Co-authored-by: Christian Heusel <christian@heusel.eu> * Group `update` and `install` in one RUN statement (in prod Dockerfile) * Remove superfluous newline * Make Node.js version logging one RUN statement * Explicitly set `--production=true` during `yarn install` --------- Co-authored-by: Christian Heusel <christian@heusel.eu>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Build in production suddenly failed with
and Node.js version was
Node.js v18.13.0although we explicitly specified v14. This might be related to this issue. We now install Node.js v16 as the EOL of v14 was already reached in April this year. v16 is only maintained until the end of September 2023 (see here), so we should upgrade to v18 eventually, which is currently the LTS. We switched tonvmto install Node.js as v18 was somehow installed although we specified v16, see the already mentioned issue.from here