Add engines

[maureenlholland]

If this changeset needs to go into the FXC codebase, please add the WMO and FXC label.

One-line summary

Pin npm and node versions to ensure consistent updates with dependabot

Significant changes and points to review

Major node version update from 20-24 (our last major update was here: https://github.com/mozilla/bedrock/pull/13939/files)

Issue / Bugzilla link

Peer fields were added in Dependabot package-lock update: #16897
These fields were being removed in local npm install steps. This is likely due to a mismatch in npm versions (although it is hard to diagnose as the peer field is not documented in npm, yet it is in npm GitHub PRs)

Testing

Run npm install locally and confirm the lock file does not change

Node upgrade tests: confirm the following run without error or noise

• make build
• make run

Integration test run 🟢 : https://github.com/mozilla/bedrock/actions/runs/19857946405

Re-running integration test for updated NPM version 🟢 : https://github.com/mozilla/bedrock/actions/runs/20096322968

Environment details
  node: v24.11.1
  npm: 11.6.2
  yarn: 1.22.22

[janbrasna]

I'm all for moving the stack to Node 24 officially, but it may need harmonizing across more surfaces:

[janbrasna]

Bedrock currently runs on Node 20:

bedrock/Dockerfile

Line 25 in 4b47277

FROM node:20.14.0-slim AS assets

[stevejalim]

@maureenlholland Shall we move the entire farm to Node 24? I'm up for that if you and @stephaniehobson are OK with it. 24 is LTS so makes sense

[maureenlholland]

@stevejalim (ah, sorry, mixed up my pings!) yeah, I'm testing it out for now. Our last update was summer 2024, so it feels like a good maintenance upgrade

[janbrasna]

In general this doesn't need any Node upgrade, and actually a Node 20 + npm 10 env would be just more stable. It's npm 11.5–11.6 bugs, combined with Dependabot adding support for it few weeks back.

"Peer fields were added in Dependabot package-lock update […] These fields were being removed in local npm install steps."

I believe that's a bug. Not sure if the keys should be there or not, but I'd err on the side of the past npm versions, and see the recent additions as a bug, and hopefully these would get removed over time with subsequent updates of Dependabot's npm like in a local setup.

They had a bug around 11.5 that was approached later not by reverting, but moving further around 11.6 looking like the resulting lockfiles are then wrapped differently between these…? These were added in 11.6.1 to fix a recent bug, but then 11.6.2 did a different fix and 11.6.3 reverted a lot of things back taking a different angle, with 11.6.4 still fixing some bugs from that exercise. So hopefully, the lockfiles for <11.5 and ≥11.6.4 should keep stable. It's just the spotty range that now Dependabot uses as a default, that yields these extra arborist values.

This was fine before npm 11.5 and before dependabot-core moved from npm 10.9.x to 11.6.x (incl. introducing a handful of bugs re. parsing semver constraints) — every patch since has a different behavior but unfortunately dependabot chose to preinstall a version right in the middle of this mess, at least a version before the peer key bug was fixed, so the dirty lockfile is a bug on their side — the local result should still be the right one.

More importantly, the results could differ between npm ci and npm install — or between a first and a second run of npm install in a row of the same project.

Normally engines won't be able to select any runtimes but Dependabot seems to take the values as a strict constraint.

So we could use the engines just temporarily, to avoid those buggy ranges for now, with possible removal at some point later?

[janbrasna]

I've written up some past decisions in #16910 for consideration.

(My personal take would be… try major only for now, and constrain more explicitly if any woes appear later — being expected more on developers' ARM machines, not the Linux build pipelines.)

[maureenlholland]

Probably won't have time to get back to this today, will have to return to it next week.

Skimming the comments, my gut reactions are:

• we can drop node out of engines field, since only npm version seems related to dependabot, can update to suggested ^11.6.4 npm version
• we're due for a node upgrade but it doesn't have to be in this PR

[janbrasna]

I see it as a temporary measure to try to force Dependabot out of the bugged version range, so it doesn't matter too much. Ideally this could be removed in a few weeks (or months, based on how quickly they update the version they use by default) — at least let's see if the higher version than what they normally provide a) works and installs in the container; b) removes the peer mess back to what it used to look like.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.30%. Comparing base (32680db) to head (ec39fd9).
⚠️ Report is 11 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #16906   +/-   ##
=======================================
  Coverage   80.30%   80.30%           
=======================================
  Files         163      163           
  Lines        9093     9093           
=======================================
  Hits         7302     7302           
  Misses       1791     1791           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[maureenlholland]

integration tests failed, but it looks related to the timeouts we were having issues with before, going to rebase and re-run the tests

[maureenlholland]

There's a twinned Springfield PR. If this PR is approved, I'll open that one to review as well.

[janbrasna]

This has one wrinkle in a slightly sensitive place — the deployment image build /actions/runs/20097110957 would log:

#36 [release assets  8/13] RUN npm ci --verbose
#36 0.727 npm warn EBADENGINE Unsupported engine {
#36 0.727 npm warn EBADENGINE   package: '[email protected]',
#36 0.727 npm warn EBADENGINE   required: { node: '^24.0.0', npm: '^11.6.4' },
#36 0.727 npm warn EBADENGINE   current: { node: 'v24.11.1', npm: '11.6.2' }
#36 0.727 npm warn EBADENGINE }

(for reasons mentioned in the threads above — it's not meant as a constraint to actually update the runtime, but conversely to throw around warnings on installation that there's not a match in the current stack being used. — it should not cause any failure in the current shape, but just thought it's worth mentioning…)

This should a) go away once node:24-slim tags an image with newer binaries preinstalled, b) is probably not worth adding the hassle of upgrading npm in the builder pipeline, c) will be hopefully over with dependabot also updating soon with the reason to add the engines with such high version constraint becoming moot here.

[janbrasna]

(It seems their 11.6.4 bump is mainly fast-tracked to v25.x for CVE reports, don't see any immediate ports to v24.x atm tho…)

[maureenlholland]

Gonna close for now and revisit in new year, hopefully Dependabot's NPM issue will be resolved and this will be a more straightforward node upgrade