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.

Sign up for GitHub

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

Jump to bottom

[BUG] package-lock issues with workspaces #2505

Closed
targos opened this issue Jan 18, 2021 · 8 comments · Fixed by npm/arborist#229
Closed

[BUG] package-lock issues with workspaces #2505

targos opened this issue Jan 18, 2021 · 8 comments · Fixed by npm/arborist#229
Assignees
@ruyadorno
Labels
Bug thing that needs fixing Priority 1 high priority issue Release 7.x work is associated with a specific npm 7 release
Milestone
OSS - Sprint 24

Comments

@targos
Copy link
Contributor

targos commented Jan 18, 2021 •
edited
Loading

Current Behavior:

After installing the dependencies in an npm workspace for the first time, running npm install a second time changes the package-lock.json:

  1. Some dependencies are removed from it
  2. The "name" field of the sub-packages is set (it wasn't present on the first install)

Expected Behavior:

Running npm install right after creation of the package-lock.json shouldn't change it.

Steps To Reproduce:

You can reproduce with the following repo: the last commit contains the package-lock.json created by the first npm install --legacy-peer-deps call.

git clone https://github.com/targos/npm-workspace-lock.git
cd npm-workspace-lock
npm install
git diff

image

image

Environment:

  • OS: Windows 10
  • Node: 15.7.0
  • npm: 7.5.1
@targos targos added Bug thing that needs fixing Needs Triage needs review for next steps Release 7.x work is associated with a specific npm 7 release labels Jan 18, 2021
@AviVahl
Copy link

AviVahl commented Jan 22, 2021

This issue has been driving me crazy, heh.

On my repo (ts-tools) it doesn't remove any dependencies on the second run, but does add the "name" field of all local packages.

I end up running git clean -fdx && rm package-lock.json && npm i && git clean -fdx && npm i every time I want to regenerate my lock file from scratch. The second git clean -fdx && npm i is to ensure lock file is "final".

@darcyclarke darcyclarke added Priority 1 high priority issue and removed Needs Triage needs review for next steps labels Feb 2, 2021
@darcyclarke darcyclarke added this to the OSS - Sprint 23 milestone Feb 2, 2021
@darcyclarke
Copy link
Contributor

@targos can you try the latest v7 & see if this is still reproducible? (ie. npm i -g npm@7)

@targos
Copy link
Contributor Author

targos commented Feb 2, 2021 •
edited
Loading

@darcyclarke It is still reproducible with npm v7.5.1.

I pushed a new commit to the repro that corresponds to the state after rm -rf node_modules && rm package-lock.json && npm install.
Running npm install after that results in:

> npm i

removed 36 packages, and audited 1810 packages in 2s

129 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities

And there's a similar diff as before (removed packages + added "name" fields)

@targos
Copy link
Contributor Author

targos commented Feb 2, 2021

I also just realized that the second npm install seems to remove the packages that have "extraneous": true, in the lock file.

@ruyadorno ruyadorno self-assigned this Feb 4, 2021
ruyadorno added a commit to ruyadorno/arborist that referenced this issue Feb 12, 2021
@ruyadorno
fix: deps of linked deps in package-lock
54a2663 
When having optional dependencies of a dependency of
a top-level link dependency, e.g:

    root -> LINK(a)
    a -> (b)
    b -> OPTIONAL(c)

    * c marked as extraneous

Any optional dependency (and all its nested nodes) were marked
as extraneous in package-lock due to `calcDepFlags` missing
properly following the target of Link nodes.

This changeset fixes it by properly following the result of the
`treeverse.visit` method that will have already followed any
link targets at that point.

Fixes: npm/cli#2505
@ruyadorno ruyadorno mentioned this issue Feb 12, 2021
Merged
@ruyadorno
Copy link
Contributor

should be out in v7.5.4

@ruyadorno ruyadorno mentioned this issue Feb 12, 2021
Closed
@targos
Copy link
Contributor Author

targos commented Feb 12, 2021

The first problem is fixed, but the second one remains. Should I create a new issue for it?

@ruyadorno
Copy link
Contributor

The first problem is fixed, but the second one remains. Should I create a new issue for it?

yes please! that seems like a much lower priority but it would be very nice to make sure we track and fix it in due time!

Thank you so much for the report and help @targos 🏆 really appreciated!

@targos
Copy link
Contributor Author

targos commented Feb 14, 2021

Done: #2700

@ruyadorno ruyadorno mentioned this issue Feb 24, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ruyadorno ruyadorno

Labels
Bug thing that needs fixing Priority 1 high priority issue Release 7.x work is associated with a specific npm 7 release
Projects
None yet
Milestone
OSS - Sprint 24
Development

Successfully merging a pull request may close this issue.

fix: deps of linked deps in package-lock ruyadorno/arborist
4 participants
@ruyadorno @darcyclarke @targos @AviVahl