Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] npm install removes resolved and integrity fields #6301

Open
2 tasks done
targos opened this issue Mar 29, 2023 · 16 comments
Open
2 tasks done

[BUG] npm install removes resolved and integrity fields #6301

targos opened this issue Mar 29, 2023 · 16 comments
Labels
Bug thing that needs fixing Priority 1 high priority issue Release 9.x work is associated with a specific npm 9 release

Comments

@targos
Copy link
Contributor

targos commented Mar 29, 2023

Is there an existing issue for this?

  • I have searched the existing issues

This issue exists in the latest npm version

  • I am using the latest npm

Current Behavior

Sometimes, when npm updates the package-lock.json after a npm install, it replaces some of the "resolved" and "integrity" fields with "license".

Example:
CleanShot 2023-03-29 at 12 19 41

Expected Behavior

npm install should not randomly change the schema of the lockfile.

Steps To Reproduce

I'm still trying to figure out reproduction steps. I think the bug is related to the state of node_modules when npm install is executed.

Environment

  • npm: 9.5.0
  • Node.js: v18.15.0
  • OS Name: macOS 13.3
  • System Model Name: Macbook Pro M2
  • npm config:
; "user" config from /Users/mzasso/.npmrc

//registry.npmjs.org/:_authToken = (protected)

; "project" config from /project/.npmrc

lockfile-version = "3"
@targos targos added Bug thing that needs fixing Needs Triage needs review for next steps Release 9.x work is associated with a specific npm 9 release labels Mar 29, 2023
@targos targos changed the title [BUG] <title> [BUG] npm install removes resolved and integrity fields Mar 29, 2023
@wraithgar
Copy link
Member

We've seen this before but never had a reproduction case for it. That would go a long way to fixing this.

@KDPRoss
Copy link

KDPRoss commented May 9, 2023

I can't git this to repro in a package-lock.json that I'm able to share publicly, but I've observed that having a trailing comma in the JSON, e.g., a thing like this:

...
    "node_modules/graceful-fs": {
      "version": "4.2.11",
      "resolved": "https://artifactory.corp.tanium.com/api/npm/tanium-npm/graceful-fs/-/graceful-fs-4.2.11.tgz",
      "integrity": "sha512-RbJ5/jmFcNNCcDV5o9eTnBLJ/HszWV0P73bc+Ff4nS/rJj+YaS6IGyiOL0VoBYX+l1Wrl3k63h/KrH+nhJ0XvQ==", <=== inappropriate comma
    },
...

will reliably induce this in a large package-lock.json that I'm working with. (I'm on npm 9.6.6 with Node 16.19.1 on x64 macOS with a v3-format lockfile.) Not quite a full repro, but might help move things forward?

Given the invalid-JSON package-lock, npm ci also reports a slightly-cryptic error:

npm ERR! code EUSAGE
npm ERR! 
npm ERR! The `npm ci` command can only install with an existing package-lock.json or
npm ERR! npm-shrinkwrap.json with lockfileVersion >= 1. Run an install with npm@5 or
npm ERR! later to generate a package-lock.json file, then try again.

@targos
Copy link
Contributor Author

targos commented May 22, 2023

I finally found a very simple way to reproduce (npm 9.6.7).
It's not the operations I was doing when I opened the issue, but maybe the root cause is the same:

mkdir repro
cd repro
npm init -y
npm install lodash underscore
rm package-lock.json
rm node_modules/.package-lock.json
rm -rf node_modules/lodash
npm i

After npm install lodash underscore, lockfile contains:

    "node_modules/lodash": {
      "version": "4.17.21",
      "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.21.tgz",
      "integrity": "sha512-v2kDEe57lecTulaDIuNTPy3Ry4gLGJ6Z1O3vE1krgXZNrsQ+LFTGHVxVjcXPs17LhbZVGedAJv8XZ1tvj5FvSg=="
    },
    "node_modules/underscore": {
      "version": "1.13.6",
      "resolved": "https://registry.npmjs.org/underscore/-/underscore-1.13.6.tgz",
      "integrity": "sha512-+A5Sja4HP1M08MaXya7p5LvjuM7K6q/2EaC0+iovj/wOcMsTzMvDFbasi/oSapiwOlt252IqsKqPjCl7huKS0A=="
    }

After the last command, it contains:

    "node_modules/lodash": {
      "version": "4.17.21",
      "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.21.tgz",
      "integrity": "sha512-v2kDEe57lecTulaDIuNTPy3Ry4gLGJ6Z1O3vE1krgXZNrsQ+LFTGHVxVjcXPs17LhbZVGedAJv8XZ1tvj5FvSg=="
    },
    "node_modules/underscore": {
      "version": "1.13.6",
      "license": "MIT"
    }

@wraithgar wraithgar added Priority 1 high priority issue and removed Needs Triage needs review for next steps labels May 22, 2023
@wraithgar wraithgar self-assigned this May 22, 2023
@theredspoon
Copy link

theredspoon commented Jun 9, 2023

Ran into this issue when I deleted package-lock.json but not node_modules and then ran npm install. I'm running npm version 9.5.1.

Deleting node_modules as well and then running npm install seemed to resolve the issue for me.

@wraithgar
Copy link
Member

Yes the root cause here is if there is a valid tree but NO package-lock, there is no integrity for npm to pull from because that is not something that is in the node_modules folder. Integrity is something that is attached to the .tgz file that is unpacked to GET node_modules.

We're still determining if this is a bug. If you didn't get the content from a registry there will be no integrity.

smaye81 added a commit to bufbuild/protobuf-es that referenced this issue Jul 14, 2023
Fixes #526.

This follows the advice specified in
npm/cli#6301 for regenerating a
package-lock.json file by removing node_modules and package-lock.json.

It appears there is an open issue where resolved and integrity fields
are removed at times when the above steps are not followed.

This also pins `@types/node` to all patch versions as upgrading the
major/minor version causes issues in our TypeScript compatibility test
since `@types/node` dropped support for v4.1 and Protobuf-ES still
supports it.

Note that while this fixes the file with this PR, we cannot guarantee
that this won't occur again due to the open issue with npm.
@targos
Copy link
Contributor Author

targos commented Aug 25, 2023

The steps I gave in #6301 (comment) are the best I could find to reliably reproduce the problem, however we got it multiple times in cases where a package-lock is actually present. I think it happens when someone from the team pulls commits from GitHub, has an outdated node_modules tree (because dependencies were updated by someone else), and manually changes a dependency version in the package.json (to update it) before running npm install

@boneskull
Copy link
Contributor

Has npm decided if this is a bug, yet?

package-lock.json churns wildly, and this is part of the problem. Even if this is intended behavior, it's not nice.

@wraithgar
Copy link
Member

@boneskull can you reliably reproduce this in an isolated case? We're still fuzzy on what the root cause is here.

@boneskull
Copy link
Contributor

@boneskull
Copy link
Contributor

As shown in the example repo and from my experience, it only seems to affect dev deps unless someone has a counterexample. I don't know if automated conflict resolution causes it or if workspaces are necessary.

@wraithgar
Copy link
Member

Resolve the conflict in packages/something/package.json by combining the changes.

What exactly is being ran here?

@boneskull
Copy link
Contributor

To cause the conflict? Switch to the conflict branch then git rebase main

@wraithgar
Copy link
Member

No, to resolve the conflict.

@boneskull
Copy link
Contributor

  1. In package.json, the resolution should take both changes; i.e. add prettier and eslint
  2. In package-lock.json, no manual resolution; run npm install after step 1.

@boneskull
Copy link
Contributor

@wraithgar Can you reproduce?

sarcasticadmin added a commit to sarcasticadmin/wave that referenced this issue Mar 27, 2024
Originally these fields were missing due to outstanding issue with npm:

  - npm/cli#4460
  - npm/cli#6301
sarcasticadmin added a commit to sarcasticadmin/wave that referenced this issue Mar 27, 2024
Originally these fields were missing due to outstanding issue with npm:

  - npm/cli#4460
  - npm/cli#6301
sarcasticadmin added a commit to sarcasticadmin/wave that referenced this issue Mar 27, 2024
Originally these fields were missing due to outstanding issue with npm:

  - npm/cli#4460
  - npm/cli#6301
@wraithgar
Copy link
Member

This also happens w/ os and libc fields: #7493

Same root cause: when npm reads from node_modules only to populate lockfile info.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug thing that needs fixing Priority 1 high priority issue Release 9.x work is associated with a specific npm 9 release
Projects
None yet
Development

No branches or pull requests

5 participants