Skip to content

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.

Sign up for GitHub

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

breaking: require Node.js version >= 20.19 #1671

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d0dba5c
fix: By bumping node version to v20.19 we will fix the @handlebars/pa…
MichalBryxi Apr 15, 2025
75dbee4
fix: Do use lockfile!
MichalBryxi Apr 15, 2025
610a78d
fix: Node version requirement >= 20.19
MichalBryxi Apr 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .github/workflows/addon-docs.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ jobs:
# Fetch tags pointing to the current commit
TAGS=$(git tag --points-at $GITHUB_SHA)
echo "Tags found: $TAGS"

# Check if a tag exists and if the ref is 'refs/heads/main' or 'refs/heads/master'
if [ -n "$TAGS" ] && ([[ "${GITHUB_REF}" == "refs/heads/main" ]] || [[ "${GITHUB_REF}" == "refs/heads/master" ]]); then
echo "SHORT_CIRCUIT=true" >> $GITHUB_ENV
Expand All @@ -41,7 +41,7 @@ jobs:
cache: pnpm
- name: Install Dependencies
if: env.SHORT_CIRCUIT == 'false'
run: pnpm install --no-lockfile
run: pnpm install
- name: Deploy Docs
if: env.SHORT_CIRCUIT == 'false'
run: pnpm ember deploy production
run: pnpm ember deploy production
10 changes: 5 additions & 5 deletions .github/workflows/ci-cd.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ jobs:
- name: Install Node
uses: actions/setup-node@v4
with:
node-version: 18.x
node-version: 20.19.x
Copy link
Collaborator

Choose a reason for hiding this comment

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

why not

Suggested change
node-version: 20.19.x
node-version: 20.x

so it could be get latest v20 shuld there be any fixes/new versions available

- uses: pnpm/action-setup@v4
with:
version: 9
Expand All @@ -46,12 +46,12 @@ jobs:
- name: Install Node
uses: actions/setup-node@v4
with:
node-version: 18.x
node-version: 20.19.x
- uses: pnpm/action-setup@v4
with:
version: 9
- name: Install dependencies
run: pnpm i --no-lockfile
run: pnpm i
- name: Test
run: pnpm test:${{ matrix.script-name }}

Expand All @@ -71,7 +71,7 @@ jobs:
- name: Install Node
uses: actions/setup-node@v4
with:
node-version: 18.x
node-version: 20.19.x
- uses: pnpm/action-setup@v4
with:
version: 9
Expand Down Expand Up @@ -103,7 +103,7 @@ jobs:
- name: Install Node
uses: actions/setup-node@v4
with:
node-version: 18.x
node-version: 20.19.x
- uses: pnpm/action-setup@v4
with:
version: 9
Expand Down
4 changes: 2 additions & 2 deletions .tool-versions
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
nodejs 18.20.3
pnpm 9.9.0
nodejs 20.19
pnpm 9.9.0
4 changes: 2 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
"@glimmer/component": "^1.1.2",
"@glimmer/syntax": "^0.87.1",
"@glimmer/tracking": "^1.1.2",
"@handlebars/parser": "^2.1.0",
"@handlebars/parser": "^2.2.0",
"broccoli-bridge": "^1.0.0",
"broccoli-caching-writer": "^3.0.3",
"broccoli-filter": "^1.3.0",
Expand Down Expand Up @@ -190,7 +190,7 @@
}
},
"engines": {
"node": ">= 18"
"node": ">= 20.19"
Copy link
Collaborator

Choose a reason for hiding this comment

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

@MichalBryxi why not

Suggested change
"node": ">= 20.19"
"node": ">= 20.x"

Copy link
Author

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.

Copy link
Author

@MichalBryxi MichalBryxi Apr 23, 2025 •
edited
Loading

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)

Copy link
Author

Choose a reason for hiding this comment

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

cc: @SergeAstapov

},
"publishConfig": {
"registry": "https://registry.npmjs.org"
Expand Down
145 changes: 73 additions & 72 deletions pnpm-lock.yaml
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion test-apps/new-addon/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
"webpack": "^5.96.1"
},
"engines": {
"node": "16.* || >= 18"
"node": ">= 20.19"
},
"ember": {
"edition": "octane"
Expand Down
2 changes: 1 addition & 1 deletion tests/dummy/app/templates/docs/deploying.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ jobs:
cache: pnpm
- name: Install Dependencies
if: env.SHORT_CIRCUIT == 'false'
run: pnpm install --no-lockfile
run: pnpm install
- name: Deploy Docs
if: env.SHORT_CIRCUIT == 'false'
run: pnpm ember deploy production
Expand Down
10 changes: 10 additions & 0 deletions tests/dummy/app/templates/docs/usage.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,16 @@ To use AddonDocs, your addon must have a `devDependency` of **Ember v2.8** or hi
Note that your addon can still support older versions of Ember – it's just that you won't be able to run your AddonDocs documentation site against those older versions. (This means you also cannot write acceptance/application tests against your docs sites on older versions of Ember.)


### Node version

Due to [this issue](https://github.com/ember-learn/ember-cli-addon-docs/issues/1669) there is a requirement for consuming app to use [node>=20.19](https://nodejs.org/en/blog/release/v20.19.0/). Otherwise you might get following error during build:

```
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.
```


## Installation

```sh
Expand Down