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

chore(node): Upgrade to node lts #5394

Merged
merged 4 commits into from
Oct 31, 2024

Conversation

bashbreakpoint
Copy link
Contributor

Upgrading the docker build and nvm to node 20.18, which is the latest supported, stable LTS version. Node 12 and 16 have been EoL since April, 2022 and August, 2023, respectively. In addition, a transitive dependency of mjml, minimatch, is not compatible with anything less than node 14 at this point, so email generation from templates was broken. With the move to newer node, I had to deal with a new node/docker restriction which won't allow you to npm install in the root directory. I wanted to maintain backwards compatiblity with current installs and their MJML_PATH parameter, so I worked around the problem by installing mjml in another directory and then moving node_modules to the root directory. It's not pretty, but it worked. I'm trying to run tests locally and I get one failure, but I'm not sure if it's my setup or something I need to correct. I've manually tested the setup, gone into every page, and not experienced problems.

Upgrading the docker build and nvm to node 20.18, which is the latest
supported, stable LTS version. Node 12 and 16 have been EoL since April,
2022 and August, 2023, respectively. In addition, a transitive
dependency of mjml, minimatch, is not compatible with anything less than
node 14 at this point, so email generation from templates was broken.
With the move to newer node, I had to deal with a new node/docker
restriction which won't allow you to `npm install` in the root
directory. I wanted to maintain backwards compatiblity with current
installs and their MJML_PATH parameter, so I worked around the problem
by installing mjml in another directory and then moving node_modules to
the root directory. It's not pretty, but it worked. I'm trying to run
tests locally and I get one failure, but I'm not sure if it's my setup
or something I need to correct. I've manually tested the setup, gone
into every page, and not experienced problems.
Copy link
Contributor

@mvilanova mvilanova left a comment

Choose a reason for hiding this comment

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

Thanks again for giving this file some love!

@bashbreakpoint
Copy link
Contributor Author

Happy to. I'm just working on getting it stood up for PoC and selling it to our business as a solution we need, and I'm just discovering minor things. It's pretty close to how I think it should be, though, so until I have time to work on a couple plugins, I might not have a lot of PRs left here.

@mvilanova mvilanova added enhancement New feature or request docker Pull requests that update Docker code labels Oct 31, 2024
@mvilanova mvilanova merged commit c87f545 into Netflix:master Oct 31, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docker Pull requests that update Docker code enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants