Skip to content
This repository has been archived by the owner on Mar 14, 2024. It is now read-only.

package-lock.json mismatches are not caught by presubmit #7619

Closed
jeffposnick opened this issue Mar 31, 2022 · 2 comments · Fixed by #7632
Closed

package-lock.json mismatches are not caught by presubmit #7619

jeffposnick opened this issue Mar 31, 2022 · 2 comments · Fixed by #7632
Labels
eng For items that require engineering work. P1 A high priority task. This task *must* be completed soon.

Comments

@jeffposnick
Copy link
Contributor

#7614 introduced a mismatch between our package.json and package-lock.json files, which prevented production deployments from succeeding. Ostensibly, this should have been caught by our presubmit GitHub Action, but looking at the logs, I see that npm ci succeeded when run during presubmit.

The difference might be that our production deployment uses npm ci --production, and presubmit uses npm ci, but I do not see anything in the docs about --production changing the way package-lock.json mismatches are treated by npm ci.

We should investigate the discrepancy and ensure that this sort of mismatch is properly flagged in presubmit.

@jeffposnick jeffposnick added P1 A high priority task. This task *must* be completed soon. eng For items that require engineering work. labels Mar 31, 2022
@jeffposnick
Copy link
Contributor Author

As per the npm documentation, running npm ci when there's a mismatch between what's requested in package.json and package-lock.json should result in the command failing. But, apparently, there's a longstanding bug that prevented that documented behavior from actually being implemented.

This bug was resolved in this PR, which made it into the v8.4.1 release of npm.

The Cloud Build image that we use to deploy web.dev uses node v16.14.2 with npm v8.5.0, meaning that its npm ci will fail when there's a mismatch.

Our staging server build on Netlify is configured to use whatever version of node is requested in .nvmrc, which is currently node v16.14.0 with npm v8.3.1. So that explains why the build succeeded on the staging server.

Our presubmit build on GitHub actions also ended up using npm v8.3.1, but I am less clear on why that happened. It may have also been from .nvmrc, but it also might have been due to

runs:
using: 'node16'
main: 'index.js'

(which just says node16, without a specific minor/patch version number). There's also a setup-node GitHub Action, but we're not using that in our presubmit workflow.

I'm going to follow-up with a PR to try to get everything bumped up to use at least npm v8.5.0, to match the Cloud Build environment.

@Ljkingoftires
Copy link

As per the npm documentation, running npm ci when there's a mismatch between what's requested in package.json and package-lock.json should result in the command failing. But, apparently, there's a longstanding bug that prevented that documented behavior from actually being implemented.

This bug was resolved in this PR, which made it into the v8.4.1 release of npm.

The Cloud Build image that we use to deploy web.dev uses node v16.14.2 with npm v8.5.0, meaning that its npm ci will fail when there's a mismatch.

Our staging server build on Netlify is configured to use whatever version of node is requested in .nvmrc, which is currently node v16.14.0 with npm v8.3.1. So that explains why the build succeeded on the staging server.

Our presubmit build on GitHub actions also ended up using npm v8.3.1, but I am less clear on why that happened. It may have also been from .nvmrc, but it also might have been due to

runs:
using: 'node16'
main: 'index.js'

(which just says node16, without a specific minor/patch version number). There's also a setup-node GitHub Action, but we're not using that in our presubmit workflow.

I'm going to follow-up with a PR to try to get everything bumped up to use at least npm v8.5.0, to match the Cloud Build environment.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
eng For items that require engineering work. P1 A high priority task. This task *must* be completed soon.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants