Skip to content
Open
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
"private": true,
"description": "A sample Node.js app using Express",
"engines": {
"node": "20.x || 22.x || 24.x"
"node": "20.x || 22.x || 24.x",
"npm": "11.*"
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

❯ /pr-review-toolkit:review-pr https://github.com/heroku/nodejs-getting-started/pull/404

  Critical Issues (0 found)

  None.

  Important Issues (0 found)

  None.

  Suggestions (1)

  - Version range specificity: "npm": "11.*" pins to npm 11.x. When a future Node major (e.g., 26.x) ships with npm 12+, this constraint will force the
  buildpack to downgrade npm. This is fine for a getting-started template (it'll get updated), but worth noting in a comment or the PR description that this
  will need a bump alongside any future Node major addition. (Already implicitly addressed by the PR description's context.)

  Strengths

  - Well-motivated: The PR body clearly explains the problem (5 noisy upgrade notices per tutorial build) and the mechanism (buildpack resolves + installs
  latest 11.x).
  - Minimal change: Only touches what's needed — no scope creep.
  - Correct JSON syntax: Trailing comma properly added to the preceding line.
  - Compatible range: npm 11 supports Node >= 20, which aligns with all listed engine versions (20.x, 22.x, 24.x).

  Recommended Action

  Ship it. This is a clean, low-risk fix that improves the developer experience for anyone following the getting-started tutorial.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

To be more consistent with how we tend to document and use a major version range specifier:

Suggested change
"npm": "11.*"
"npm": "11.x"

},
"main": "index.js",
"scripts": {
Expand Down