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

refactor AND bugfix #5350

Merged
merged 2 commits into from
Aug 24, 2022
Merged

refactor AND bugfix #5350

merged 2 commits into from
Aug 24, 2022

Conversation

wraithgar
Copy link
Member

@wraithgar wraithgar commented Aug 23, 2022

Two commits here, one is a refactor so that we could more easily find the bug, the second is the bugfix.

When installing a relative path into a workspace, arborist was trying to install it relative to the project root. This was ok before workspaces as that was the only place you could add new deps. With workspaces we need to now add new deps relative to the tree it's being added to. The second commit does this.

~/D/n/s/ws $ npm pkg delete dependencies.a -w b
{}
~/D/n/s/ws $ node ../../cli i ./workspaces/a -w b

up to date, audited 5 packages in 871ms

found 0 vulnerabilities
~/D/n/s/ws $ npm pkg get dependencies -w b
{
  "b": {
    "a": "file:../a"
  }
}
~/D/n/s/ws $ npm ls
ws@1.0.0 /Users/wraithgar/Development/npm/scratch/ws
├── a@1.0.0 -> ./workspaces/a
└─┬ b@1.0.0 -> ./workspaces/b
  └── a@1.0.0 deduped -> ./workspaces/a

@wraithgar wraithgar requested a review from a team as a code owner August 23, 2022 22:40
@wraithgar
Copy link
Member Author

This was refactoring and cleanup trying to debug an issue, the bug still stands but this code allowed us to find where the bug starts, to be fixed later.

@wraithgar wraithgar changed the title fix: inline single-use functions refactor AND bugfix Aug 24, 2022
Added link deps need to be relative to the package they're being added
to, not the project root.  In the past the project root was the only
place you could add things but workspaces changed this.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants