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

Upgrade Node.js to v20 (LTS) #578

Merged
merged 4 commits into from
Jan 5, 2024
Merged

Upgrade Node.js to v20 (LTS) #578

merged 4 commits into from
Jan 5, 2024

Conversation

Splines
Copy link
Member

@Splines Splines commented Jan 4, 2024

Upgrade Node.js to v20.10.0 (LTS until April 2026).

I tested this on the experimental branch and the run was successful (see Dozzle). Also check out mampf-experimental. Nope, that's what I hoped, but of course the old error keeps hunting us.

And we are not alone, the question for this error was viewed 2.5million times on Stack overflow here! I will fix this! maybe useful

Finally fixed

In 8f67ce9, I finally fixed this issue after several failed attempts. See the comment in the code for further details. The experimental branch currently also points to this commit and you can see in the logs that it's working. mampf-experimental also shows the monotiles on the landing page.

@Splines Splines added the dependencies Pull requests that update a dependency file label Jan 4, 2024
@Splines Splines self-assigned this Jan 4, 2024
@Splines Splines changed the title Upgrade NOde.js to v20 (LTS) Upgrade Node.js to v20 (LTS) Jan 4, 2024
Instead of the insecure MD4, we use SHA256.
The solution was proposed here:
https://stackoverflow.com/a/72219174/
@Splines Splines marked this pull request as ready for review January 4, 2024 23:40
@Splines Splines requested a review from fosterfarrell9 January 4, 2024 23:41
@Splines Splines mentioned this pull request Jan 4, 2024
1 task
fosterfarrell9
fosterfarrell9 previously approved these changes Jan 5, 2024
Copy link
Collaborator

@fosterfarrell9 fosterfarrell9 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works like a charm. Is /docker/run_cypress_tests/Dockerfile excluded from the change because it is currently excluded from the pipeline anyway?

We do this even though we currently don't use this Dockerfile.
@Splines
Copy link
Member Author

Splines commented Jan 5, 2024

Oh no, thanks I simply forgot about it 🙈 It's not used right now, that's true. I've still updated it to keep the symmetry. I need another approval then.

@Splines Splines merged commit 533c427 into mampf-next Jan 5, 2024
2 checks passed
@Splines Splines deleted the deps/upgrade-node branch January 5, 2024 22:20
Splines added a commit that referenced this pull request Jan 6, 2024
* Upgrade Node.js to v20 (LTS)

see the release schedule here:
https://raw.githubusercontent.com/nodejs/Release/main/schedule.svg

* Use secure hash algorithm for webpack

Instead of the insecure MD4, we use SHA256.
The solution was proposed here:
https://stackoverflow.com/a/72219174/

* Refactor: extract variable

* Upgrade node version also for cypress tests

We do this even though we currently don't use this Dockerfile.
@Splines Splines mentioned this pull request Jan 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants