[code-infra] Make engine version requirement strict

[brijeshb42]

We can safely start with this repo and eventually add this to other repos as well.

https://docs.renovatebot.com/configuration-options/#constraints

[mui-bot]

Bundle size report

Bundle	Parsed size	Gzip size
@base-ui-components/react	0B(0.00%)	0B(0.00%)

Details of bundle changes

Generated by 🚫 dangerJS against 70e97f3

[Janpot]

I'm not against, but I want to codeify somewhere our policy around minimum node.js versions. I feel like this keeps coming up over and over. Ideally we express it either in terms of LTS, or in terms of available features. That way we can stop discussing this over and over and point to a policy for guidance.

So I propose to document:

• Dev environment (workspace root package.json): "Active LTS" or lower. Why? maintainers/contributors should take the effort to stay up-to-date, we shouldn't waste code-infra time on backporting fixes and compatibility modes for outdated node.js versions.
• React libraries we publish (individual package.json): First node.js version that supports ESM or lower. Why? why not? it's a react library, it runs primarily in browsers. SSR is only a subset of users.
• CLI tools, node.js libraries, babel/eslint plugins we publish publicly (individual package.json): "Maintenance LTS" or lower. Why? seems best practice in the community.
• @mui/Internal-* node.js tooling (individual package.json): same as Dev environment. Why? unused except in environment that is already with this restriction.

Where to document:

Similar to https://www.notion.so/mui-org/Naming-convention-for-npm-packages-235cbfe7b66080058fd9f7aaf3c3177e
We either convert this to a broader documentation with guidelines for repo organization or create a document adjacent to it. (@oliviertassinari any preferences?)

Can we renovatebot this?

Probably, but good luck with that 🙂.

[brijeshb42]

I think the best way to codify this (repo-wise) would be to use engines field in package.json and not specify the version individually to things like Github Actions or Circle CI (for developers). I'll have to look into how to do this with Circle CI.

I mainly want to implement this because we updated all our CI config to use v22 without enforcing it at the user level. So some discrepancy is bound to creep in. CI should be using the same version as the developers.

We can have the same constraints as-is for our package users, which is node14 which we pass to Babel for transpilation. No changes there.

Also, I agree with your proposal about which node versions should be used.

[Janpot]

I mean "codify the rationale for which version is being put in the "engines" field".

[oliviertassinari]

to document

An attempt at it:

1. React libraries we publish (individual package.json): today we have https://mui.com/material-ui/getting-started/supported-platforms/#server. I propose the same strategy in [docs] Add "Supported platforms" page base-ui#1522 for what Node.js version we support. I don't see a need to change it. SSR is important; it needs to be broadly supported, so support versions down to maintenance mode: Maintenance.
2. Dev environment: It feels like the policy should be: aim to match 1. but allow pragmatic exceptions. For example, upgrading Node.js to unlock updating important dependencies, significant simplifications of internal ops by adopting new Node.js features, seems a win to swallow the overhead of fixing a few Node.js compatibility regressions. So Maintenance version by default, but LTS if there is enough gain for it, and only Active if there is a strong upside.
3. CLI tools: same as 2.
4. @mui/Internal-: Same as 1.

[Janpot]

But we want to keep it < 24, right?

[brijeshb42]

Not sure. The other constraint (^22) is quite restrictive. Ideally we'd want both package.json and renovate.json to be in sync. Having ^22 in package.json will require user to be always on v22 which is not as friendly.
Generally node.js releases are backwards compatible. So I don't see the requirement to restrict the max version.

[Janpot]

I believe we want to allow users to use node 24, but we should keep our CI at 22, right?

[brijeshb42]

Yes.

[Janpot]

And if we want different node versions for different engines fields

• >=14 for react packages
• >=20 for public cli
• >=22 for internal code

Wouldn't this constraint mess with that?

[brijeshb42]

IIUC, these I believe are for consumers of the package. We can specify engines.node in the package specific package.json file and that won't mess with the top-level engine in any way.

[Janpot]

Yes, absolutely, but wouldn't

  "constraints": {
    "node": ">=22"
  },

cause renovatebot to try to update those engines fields to >22?

[brijeshb42]

I'm not sure. I guess we'll have to deploy and see

[brijeshb42]

I have removed the constraints field since we anyways have disabled auto update of node versions in our base config.

[oliviertassinari]

Do we want this? Per https://pnpm.io/settings#enginestrict, it feels like the more we can get away with ignoring engine version, the better. Either because we don't want to upgrade Node.js to stay as low as possible, or because we want to upgrade Node.js but the dependency says it's not compatible with more recent versions.

Suggested change

engineStrict: true

[Janpot]

I believe we want this, yes. For our dev env we don't want to stay as low as possible. We want what allows us to write modern, secure, performant code and great contributor DX. We don't want to waste our and our contributor's time on problems caused by too low node.js version, nor do we want to waste our time on building polyfills and compatibility modes for outdated node.js. Best way to prevent us from having to chase these errors this is to just block altogether during the installation process.

I feel like we keep circling back to this. Are you sure you're not conflating end-user node.js version with contributor node.js version? Again, 100% agree that for our React libs we can keep it as low as possible. Personally, I wouldn't even explicitly set it. But these node versions are untouched here.

it feels like the more we can get away with ignoring engine version, the better.

But you can't just ignore the engines field, that would mean we would need to write our tooling with maximum backwards compatibility. On top of the fact that none of our dependencies are following this philosophy, it would be a total waste of time, there is simply no demand for this. If a contributor wants to be able to ignore engine version, they should just use a node version manager.

[oliviertassinari]

We want what allows us to write modern, secure, performant code and great contributor DX

I got the impression that engineStrict goes against this objective for the infra packages. It doesn't make it possible to upgrade Node.js to v22 if a package says it only support Node.js up to v20.

[Janpot]

It doesn't make it possible to upgrade Node.js to v22 if a package says it only support Node.js up to v20.

isn't that a good thing? Why would we upgrade to node 22 if not all of our dependencies can run under it? If a dependency blocks us from upgrading to node.js maintenance LTS, it's a clear sign this package is going to be a liability for us.

[oliviertassinari]

If a dependency blocks us from upgrading to node.js maintenance LTS, it's a clear sign this package is going to be a liability for us.

Yeah maybe, I thought that if the test in the CI pass, then we can ignore what the engine field says. I have grown to not trust the compatibility version library say they have. But either way seems ok.

[Janpot]

Yep, in practice many libraries just work outside of their officially supported node.js version range. For me, when we're blocked from upgrading it tends to mean one of two things:

1. our dependency is hopelessly outdated
2. the package is unmaintained

which ideally is solved by upgrading or replacing it. Ofcourse we should always be pragmatic, it's not always immediately possible, I get that, but we can relax the engineStrict again any time. As long as we're not unreasonably blocked by it, why not just keep it enabled?

[oliviertassinari]

What do we gain with upgrading Node from the Maintenance to LTS version?

[Janpot]

Node.js 22 things we've used already or want to be able to use:

• fs.glob
• better ESM interoperability (but need 22.12 for this)
• Array.fromAsync
• Set methods (e.g. new Set().union())
• Iterator helpers

also

V8's Maglev Compiler is now enabled by default on supported architectures (https://v8.dev/blog/maglev). Maglev improves performance for short-lived CLI programs.

sounds like it would positively affect us, but it's not a feature that would break our tooling on <22

24 would bring more useful features I'd love to be able to start using such as explicit resource management

[oliviertassinari]

So valuable for scripts mostly, but not so relevant for Netlify?

For public frontend packages, this seems right https://github.com/mui/base-ui/blob/caa205021e9cdc58d6e8e509a3adf381469fa40a/netlify.toml#L9.

For internal tools like here, I guess, we don't care, we can be on a LTS or even more recent version.

[Janpot]

So valuable for scripts mostly, but not so relevant for Netlify?

Scripts, but also things like next.config.js and any code it calls, which does run in Netlify. The llms generation script runs in Netlify as well, the caching plugin as well.

For internal tools like here, I guess, we don't care, we can be on a LTS or even more recent version.

Netlify uses internal tooling, if we bring it down to v20 then we bring all tooling down to v20. Which is just as fine for me, but I don't really see a lot of benefits. We're not using the actual compiled output in the docs, so the argument that this somehow helps us test that our libs work under node v20 is largely moot. preset-env already takes care of this very well, and I think we can mostly trust on that, we have been up until now.

[oliviertassinari]

We're not using the actual compiled output in the docs

In this repository, I imagine that we using the compiled version of Base UI, Material UI to SSR render them, but with so much lag that it's not different from a community member reporting a regression.

preset-env already takes care of this very well, and I think we can mostly trust on that, we have been up until now.

Hmm, I have seen bunch of cases where Babel wouldn't catch issues during the ES5 to ES6 high pace of language evolution era.

Scripts, but also things like next.config.js and any code it calls, which does run in Netlify. The llms generation script runs in Netlify as well, the caching plugin as well.

Ah, so those also runs in the product repository's Netlify builds.

Overall, I think that if we stick to #552 (comment), we should be fine. Managing going slower fixing regressions vs. going slower because doing work we wouldn't need to do if we were more up to date.

[Janpot]

Ah, in this repository, yes, it could serve as a form of integration testing. But unfortunately packages have already been published to production by that time, so it's already way too late. Perhaps instead we should set up a specific integration test to run after pkg.pr.new has run, using the published preview packages. It could run on node maintenance LTS, and be just a next.js and/or vite application that we run the build for. Or even the docs though they may be a bit heavyweight for this purpose. (edit: opening #562)

Imagining having this set up, do you have any objections to use latest node (24) on all of our dev environments?

[oliviertassinari]

Imagining having this set up, do you have any objections to use latest node (24) on all of our dev environments?

@Janpot If we have one comprehensive enough smoke test that runs with Node.js v14, I guess this could catch most of the potential regressions. It's not as good as running every test, but it probably still passes the bar of good enough.

I can't help thinking that Next.js must have the same problem. How do they solve it? It looks like they run on the LTS, Node 20: https://github.com/vercel/next.js/blob/1207ae6c0ca8705f84ce901208004f0c3f17c797/package.json#L138 and run all tests with 18, the lowest version they support, and 20: https://github.com/vercel/next.js/blob/canary/.github/workflows/build_and_test.yml#L514, vercel/next.js#67274. So what I think I was describing as a strategy #552 (comment) is more or less Next.js. Now, to consider that we are not building a tool like they do, so we don't have as strong constraints on Node.js as they do, so likely Base UI, Material UI, can relax this. Pigment CSS would be a bit more of a challenge.

---

I see a similar version of this problem with Vitest, but a bigger version of this problem; as with browsers, we saw more compatibility issues than with Node.js. We used to test on BrowserStack the oldest version of the range that we support: https://mui.com/material-ui/getting-started/supported-platforms/#browser. Proof: https://github.com/mui/material-ui/blob/v4.x/test/karma.conf.js#L124. And now it's Chrome only, and the more recent version​ only. Proof: https://app.circleci.com/pipelines/github/mui/base-ui?branch=pull%2F2543. This feels like we went backward.

There, we might want to run all the tests, so maybe it's more about setting Docker environments to run those tests.

[oliviertassinari]

better ESM interoperability (but need 22.12 for this)

It looks like this was backported nodejs/node#56927, https://nodejs.org/en/blog/release/v20.19.0.

Edit: Well, no, I don't know how they did that, but it doesn't work, when I try to load http://localhost:3001/x/introduction/ with v20.19.2 it crashes

but v22.18.0 works.

---

So how about:

1. We upgrade Node.js to the more recent version, the LTS v22? Why: we are struggling with the ESM migration, we want to kill CJS.
2. We add a simple smoke test for older versions of Node.js. Why: to mitigate the short-term cost of 1. [code-infra] Integration test with published previews #562
3. We use [code-infra] Make engine version requirement strict #552 (comment) as the high-level strategy to have a long-term robustness. Why: we shouldn't see another CJS -> ESM for a long time. It's unlikely that our dependencies would drop support for Node.js maintenance mode as they did for ESM. Now, they will at one point once the maintenance version become legacy. So when they do, if we can't yet drop the old Node.js version because we are in between two majors, we still follow them, we upgrade Node.js and rely on 2 to get test coverage.

[oliviertassinari]

Ok, policy added to the engineering handbook: https://www.notion.so/mui-org/Node-js-support-strategy-25acbfe7b66080df9d8bf6634f8c9367.

[brijeshb42]

@Janpot I noticed that we are using 24.3 for CircleCI but 22 for other environments. Was this somehow missed or is it intentional ?
Also, I don't see any way for Circle to pick up the version from package.json without complicating the config (and the build times).

[Janpot]

Was this somehow missed or is it intentional?

Not intentional I think.

Also, I don't see any way for Circle to pick up the version from package.json

I don't think we can make package.json the absolute single source of truth. Especially not in docker environment where checkout happens after a specific node image has to be started first.