-
-
Notifications
You must be signed in to change notification settings - Fork 144
breaking: require Node.js version >= 20.19 #1671
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
base: main
Are you sure you want to change the base?
breaking: require Node.js version >= 20.19 #1671
Conversation
…rser issue - Reported in ember-learn#1669
- Without this the CI fails, because it installs incorrect dependencies
- Fixes ember-learn#1669 Caused by: ``` require() of ES Module .../.pnpm/@handlebars+parser@file+..+handlebars-parser/node_modules/@handlebars/parser/dist/cjs/index.js not supported. Instead change the require of index.js in null to a dynamic import() which is available in all CommonJS modules. ``` When using older node in consuming app.
2aa71d3
to
610a78d
Compare
@@ -190,7 +190,7 @@ | |||
} | |||
}, | |||
"engines": { | |||
"node": ">= 18" | |||
"node": ">= 20.19" |
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.
@MichalBryxi why not
"node": ">= 20.19" | |
"node": ">= 20.x" |
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.
Anything below 20.19
results in following error in consuming app for me:
> [email protected] start /Users/michal/ember/ember-intl/docs/ember-intl
> ember serve
require() of ES Module /Users/michal/ember/ember-intl/node_modules/.pnpm/@handlebars+parser@file+..+handlebars-parser/node_modules/@handlebars/parser/dist/cjs/index.js not supported.
Instead change the require of index.js in null to a dynamic import() which is available in all CommonJS modules.
Stack Trace and Error Report: /var/folders/8q/yrjx_8s115q43r8w5qd90_m00000gn/T/error.dump.09dfcadbf87770bc232cc47d14bf82ae.log
ELIFECYCLE Command failed with exit code 1.
node --version
v20.18.0
IDK how to signify that the consuming app should have certain node version, but sounds reasonable to impose minimal node version also on this repo to not bump into the same set of issues in, say, the testing app (which I think is actually happening).
Related Discord convo.
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.
Ah yeah, this addon's CI itself is having issue with node < 20.19
: https://github.com/ember-learn/ember-cli-addon-docs/actions/runs/14612606117/job/40993520690?pr=1672 (different PR that does not have these patches)
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.
cc: @SergeAstapov
@@ -21,7 +21,7 @@ jobs: | |||
- name: Install Node | |||
uses: actions/setup-node@v4 | |||
with: | |||
node-version: 18.x | |||
node-version: 20.19.x |
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.
why not
node-version: 20.19.x | |
node-version: 20.x |
so it could be get latest v20 shuld there be any fixes/new versions available
Caused by:
When using older node in consuming app.
This PR is based on the actual fix PR for this very repo << it should be reviewed & accepted first. This is just to make sure the build will be green. I can retarget separately if it makes it easier to read.