[go: nahoru, domu]

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

feat(unstable/npm): support peer dependencies #16561

Merged
merged 47 commits into from
Nov 8, 2022

Conversation

dsherret
Copy link
Member
@dsherret dsherret commented Nov 8, 2022

This adds support for peer dependencies in npm packages.

  1. If not found higher in the tree (ancestor and ancestor siblings), peer dependencies are resolved like a dependency similar to npm 7.
  2. Optional peer dependencies are only resolved if found higher in the tree.
  3. This creates "copy packages" or duplicates of a package when a package has different resolution due to peer dependency resolution—see https://pnpm.io/how-peers-are-resolved. Unlike pnpm though, duplicates of packages will have _1, _2, etc. added to the end of the package version in the directory in order to minimize the chance of hitting the max file path limit on Windows. This is done for both the local "node_modules" directory and also the global npm cache. The files are hard linked in this case to reduce hard drive space.

This is a first pass and the code is definitely more inefficient than it could be.

Closes #15823

Copy link
Member
@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initial review

cli/lockfile.rs Show resolved Hide resolved
cli/tests/integration/npm_tests.rs Outdated Show resolved Hide resolved
cli/npm/resolution/snapshot.rs Outdated Show resolved Hide resolved
@dsherret dsherret marked this pull request as ready for review November 8, 2022 18:03
Copy link
Member
@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Massive effort David. I'm sure we'll find more quirks once this goes out to users to try, but this first pass is working very well already!

@dsherret dsherret merged commit cbb3f85 into denoland:main Nov 8, 2022
@dsherret dsherret deleted the npm_peer_deps branch November 8, 2022 19:17
@dsherret dsherret mentioned this pull request Nov 8, 2022
25 tasks
bartlomieju pushed a commit that referenced this pull request Nov 8, 2022
This adds support for peer dependencies in npm packages.

1. If not found higher in the tree (ancestor and ancestor siblings),
peer dependencies are resolved like a dependency similar to npm 7.
2. Optional peer dependencies are only resolved if found higher in the
tree.
3. This creates "copy packages" or duplicates of a package when a
package has different resolution due to peer dependency resolution—see
https://pnpm.io/how-peers-are-resolved. Unlike pnpm though, duplicates
of packages will have `_1`, `_2`, etc. added to the end of the package
version in the directory in order to minimize the chance of hitting the
max file path limit on Windows. This is done for both the local
"node_modules" directory and also the global npm cache. The files are
hard linked in this case to reduce hard drive space.

This is a first pass and the code is definitely more inefficient than it
could be.

Closes #15823
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.

Unable to discover some of npm dependencies
2 participants