Fix up "Use symbolic GitHub Actions Node.js versions (#49403)"

[jablko]

I see the nightly and update package-lock.json workflows failed last night thanks to my contribution:

I misunderstood the npm docs: A registry <-> token environment variable association is required. Set one up the same way as in the npm publish starter workflow (which is also the way it was before).

[jablko]

I've added the --ignore-scripts option to the update package-lock.json workflow.

The issue with that workflow was that between npm v6 and v7, npm install --package-lock-only started running lifecycle scripts, and bumping the Node.js version from v14 to v16 bumped npm from v6 to v8: npm/cli#2787 (comment)

The conclusion seems to have been that --package-lock-only shouldn't run lifecycle scripts, but the npm cli issue was closed as fixed, without actually being fixed. Whatever --- the --ignore-scripts option will suppress lifecycle scripts either way.

[typescript-bot]

This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise.

[jakebailey]

Given we are using a v1 lockfile, we probably shouldn't have upgraded any of the workflows that could change the lockfile and push the code back; I'd like to see these brought back down to 14.

Based on #49403, this means pretty much every workflow that isn't a test workflow should be restored back to Node 14 so NPM 6 is used; anything that messes with versions will change the lockfile (including the update-package-lock, set-version, sync-branch, release-branch-artifact, etc). It may also include any workflow that does npm ci; IIRC npm will fail on this because the lockfile version doesn't match.

Eventually, I think we'll be able to ignore this problem via corepack and specify which npm we want the project to use, but until then, or Node 14 goes away, lockfile v1 is going to be the least-bad option.

[jablko]

@jakebailey The v2 lockfile is backwards compatible to v1 lockfiles. I think this means we can use it with npm v6 as well as Node.js >= v16. I'll double check ...

[jakebailey]

That is technically true, but causes pain for anyone who actually uses npm6 (like volta users who automatically get node14 when working in the TS repo), because the lockfile is converted back to v1, and then you have to deal with the fact that a committed file changed.

Again, eventually we're going to change to v2, because 14 will be gone, but I don't think that a cleanup/tweak to our GitHub actions is the place to start requiring a totally different lockfile format.

[jablko]

@jakebailey The reverse logic applies, no? Users on npm > v6 have to deal with the fact that the committed package-lock.json got converted to v2 vs. converted back to v1?

I double checked and the npm v6 ci command is fine with lockfile v2: Doesn't fail, doesn't convert it to v1.

The only workflow that both runs npm install without --no-package-lock and commits package-lock.json is the update package-lock.json workflow. I've restored node-version: 14 to that one, but I think it's six of one, half dozen of the other, whether npm v6 users or npm > v6 users get the pain that a committed file changed?

[jakebailey]

I think it's six of one, half dozen of the other, whether npm v6 users or npm > v6 users get the pain that a committed file changed?

That is true, but isn't feedback I've personally seen on the repo. I've seen at least one time a v2 lockfile was committed and had to be reversed, so it does happen. We aren't exactly good about documenting the versions we expect people to use outside the volta configuration, so that's something to improve. We could simply set a volta configuration that points to node 16 but npm 6, to make that expectation clear (right now we set node 14, which implies npm 6), but that won't fix the GHA issues.

Unrelated, but another nit about the GHA changes are the way the unpinned versions look in the checks, which makes it a little unclear what actually failed if something does fail:

So, I'm sort of iffy on whether or not the trick is beneficial if it's only saving us from modifying a list of versions once or twice a year; the other versions can be static as they aren't actually effects seen by end users.

[jablko]

Does the update package-lock.json workflow handle rolling back any accidentally committed changes, nightly? Whether it be rolling back v2 -> v1 as now, or v1 -> v2 after we upgrade to npm > v6.

Unrelated, but another nit about the GHA changes are the way the unpinned versions look in the checks, which makes it a little unclear what actually failed if something does fail:

That's a thing. I'll see if there's a way to improve it.

[jakebailey]

Does the update package-lock.json workflow handle rolling back any accidentally committed changes, nightly?

If you mean whether or not this workflow can undo an accidental upgrade or downgrade, probably given it reruns npm and then commits it back, but truthfully it's not clear to me how the workflow even does any updates at all (I would have never expected --package-lock-only to update anything). It's easier to catch problems with committing a v2 lockfile because you'll end up with a really huge diff from the added lines (or, vice versa).

[jakebailey]

FWIW I don't know that the job names can be fixed; that is generated statically where you don't actually have info about what version of node an action is going to pick (if you're running an action, it's already too late).

[RyanCavanaugh]

@weswigham @jakebailey gtg?

[jakebailey]

Now that this is run with node 14, I think this can be dropped, though if it exists it probably won't be harmful. Shame about that npm bug.

[jakebailey]

Note that this probably wouldn't work in npm7+ at all, because npm install doesn't just overwrite package-lock with new versions anymore. When we switch to lockfile v2, I think we'll have to come up with a new updating system.