chore(infra): test Node.js v16 and expand "engines" range to include it #713
Conversation
This also updates the docs repository. The `engines-strict` setting in the `.npmrc` will enforce that the operators of this repository are using the right version. Someone who wishes to circumvent this can pass `--ignore-engines` when running `npm install`, much like someone circumventing a library's own `engines` requirements.
The newer images are smaller and suggested to be widely cached. In practice, the older images are widely cached because they're still more prevalent but happy to use the newer images, particularly since the older images aren't getting as much in the way of updates anymore (e.g., there are not any Node.js 16 images for it.)
Using this instead of `.nvmrc` since this allows more granular control over the `npm` version that gets installed. Ref: https://docs.netlify.com/configure-builds/manage-dependencies/#npm
abernix
force-pushed
the
abernix/node-16
branch
from
790f3be
to
e663ea0
Compare
The constraint on this can be enforced exclusively at the monorepo level since this package is not published and that constraint need not be conveyed by any user at runtime.
The "circleci" manager is intentionally disabled as of this commit. While we do benefit from its updating of "Orb" versions, the CircleCI manager also attempts to update Docker images using its "Docker" datasource. This is really handy, in theory, but we have intentionally varying major Node.js docker image identifiers in our CircleCI configuration to test on each major Node.js platform! I suspect that enabling the "circleci" manager would cause all of these to update to the latest version (e.g., 16) when we want them to be intentionally different! I'm going to try to craft an fix for this, but I might do it upstream on Renovate itself using its Regex manager. It's also worth noting that there is other configuration that could be used to _only_ renovate "orbs" on this file, but I'm taking the short-cut route and not juggling with that right now. I can test the repository locally with my own copy of Renovate and come up with another solution given some free time.
| // there is other configuration that could be used to _only_ renovate "orbs" | ||
| // on this file, but I'm taking the short-cut route and not juggling with | ||
| // that right now. I can test the repository locally with my own copy of | ||
| // Renovate and come up with another solution given some free time. -Jesse |
There was a problem hiding this comment.
Note to self: Another option might be https://docs.renovatebot.com/configuration-options/#matchcurrentversion.
abernix
force-pushed
the
abernix/node-16
branch
from
edf76d6
to
1249c8d
Compare
abernix
requested review from
glasser,
martijnwalraven,
pcmanus and
trevor-scheer
and removed request for
a team
abernix
force-pushed
the
abernix/node-16
branch
2 times, most recently
from
5931d1d
to
99f1a3d
Compare
abernix
force-pushed
the
abernix/node-16
branch
5 times, most recently
from
82be098
to
a2696a0
Compare
To overcome an error which was occurring during the "Rustup install" step that was producing a "memory allocation of 16777216 bytes failed" error that was ALSO NOT causing the _setup_ step to fail, this commit: - Use the 64-bit installer rather than the 32-bit! I would have thought that this on its own could have been the major root of the problem, but it seems like it's also paired best with the next bullet. - Limits the amount of RAM for the _installation of Rust_ (and nothing else), overriding the auto-detection which seems un-reliable in this virtual env. rust-lang/rustup#2229 (comment) - Uses a larger resource size, which actually makes the tests run faster anyhow. This will use more credits but use less minutes. I'll look at the numbers in a month to see what I think, but I'm not worried right now. (note to self: 76203c/1900m, right now, jesse) - Prevent missed failure catch in installation via .exe file, instead opting for using LASTEXITCODE which seems to be a recommended way to do this when using EXE files. Having the "echo" there certainly seems like it could have been hurting things too (since echo returns 0, but that didn't seem to fix it on its own).
abernix
force-pushed
the
abernix/node-16
branch
2 times, most recently
from
136fe89
to
e25349b
Compare
| // its "Docker" datasource. This is really handy, in theory, but we have | ||
| // intentionally varying major Node.js docker image identifiers in our | ||
| // CircleCI configuration to test on each major Node.js platform. Enabling | ||
| // the "circleci" manager would cause all of these to update to the latest |
There was a problem hiding this comment.
I'm not trying to verify whether this is true or not (I kinda hope it's actually not hard to tell it not to take major version bumps), happy to move forward with circleci disabled for now even if there's a chance you're being overly conservative.
There was a problem hiding this comment.
I suspect it's easy to tell it to not take major version bumps for this file, yes.
abernix
changed the title
abernix
changed the title
Inspired by apollographql/federation#708 as well as the Netlify change in apollographql/federation#713 This also fixes engines declarations in the individual packages which should have been done in #5137.
Inspired by apollographql/federation#708 as well as the Netlify change in apollographql/federation#713 This also fixes engines declarations in the individual packages which should have been done in #5137.
Inspired by apollographql/federation#708 as well as the Netlify change in apollographql/federation#713 This also fixes engines declarations in the individual packages which should have been done in #5137. Also fix a weird problem related to `@apollo/protobufjs`. For some reason, Node 12 and 14 with npm 7 (but not Node 16!) was running into an issue where the fact that `@apollo/protobufjs` doesn't actually depend on a bunch of packages like `espree` that its CLI needs caused the `apollo-pbjs` calls to fail. I think when we forked, protobuf.js was midway through a refactor to make the CLI into a separate package, but we haven't set that up to work in our fork. Adding all those big packages as dependencies of `@apollo/protobufjs` didn't seem great because that would make them non-dev dependencies of apollo-server themselves. Splitting the packages (and making the CLI package a devDep of apollo-reporting-protobuf) is the best long-term solution but for now adding the CLI deps as devDependencies of apollo-reporting-protobuf appears to work?
|
I'm trying to update to Node 17 in my project, but I am blocked by yarn, as these packages restrict the use of this engine. It seems very pessimistic to have an upper bound on the node version, as for the vast majority of times, a new Node version will not break these packages. This can also be seen by there being no actually code change in this PR. |
|
For reference, for the gateway package, the upper limit was added in this commit: 4ef79ae#diff-e441a2bec2f27de2e89f69427e4837410164f9ad4dd7e9428e4286b024ee5d78R15 But I can't really find anything in the commit message or PR that indicates why this was done. |
|
@holm It’s not unreasonable for a library to be pessimistic about supporting various major versions without first validating that things aren’t broken. No upper bound means we're claiming support for ALL future major versions of Node in those releases. We'd rather ship with a clear expectation of what we support than allow our users to break unexpectedly. Yarn supports working around this (buyer beware) via the |
|
It may not be unreasonable, but I don't think it's very helpful. NodeJS goes out of their way to be as backwards compatible as possible and from my experience, it is incredibly rare for packages to break in new node versions, unless they rely on internal APIs og uses C code.
It's the choice of the maintainers of course, I think it's just an annoyance with a tiny benefit. |
Inspired by apollographql/federation#708 as well as the Netlify change in apollographql/federation#713 This also fixes engines declarations in the individual packages which should have been done in #5137. Also fix a weird problem related to `@apollo/protobufjs`. For some reason, Node 12 and 14 with npm 7 (but not Node 16!) was running into an issue where the fact that `@apollo/protobufjs` doesn't actually depend on a bunch of packages like `espree` that its CLI needs caused the `apollo-pbjs` calls to fail. I think when we forked, protobuf.js was midway through a refactor to make the CLI into a separate package, but we haven't set that up to work in our fork. Adding all those big packages as dependencies of `@apollo/protobufjs` didn't seem great because that would make them non-dev dependencies of apollo-server themselves. Splitting the packages (and making the CLI package a devDep of apollo-reporting-protobuf) is the best long-term solution but for now adding the CLI deps as devDependencies of apollo-reporting-protobuf appears to work?
This PR introduces testing for and expands package's
enginesrange for thenodeengine to support the v16 line, which was recently released and is scheduled to go into it's Long-Term Support contract with the Node.js Foundation in October. This PR targets thenpm@7upgrade PR (#708).I've made a couple other related changes within this PR, but of notable importance, I've intentionally disabled Renovation of Docker images within CircleCI because I'm pretty sure that right now, by having moved the
docker.imageproperty into this file, rather than in the Orb (which I think is the right direction to go), it would just be updating everything to the true numerical latest version (e.g., 16, or even 17), which is not what we want. (e.g., all our tests which intentionally test different versions of Node.js would test the same recent version). I think I know how to fix this, but I don't want to couple it to this PR and don't have time to look into it further at the moment (and would benefit from some local development time with Renovate and this PR actually having landed already).