-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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): support using a local copy of npm packages #28512
Conversation
@@ -236,6 +237,54 @@ impl CliLockfile { | |||
)) | |||
}) | |||
.collect(), | |||
patches: if workspace.has_unstable("npm-patch") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This requires putting this in the deno.json. I think that's fine because the patch feature needs to be in a deno.json and I think it's not worth it to support --unstable-npm-patch
because this is only going to exist for a few weeks.
.filter(|f| !from_config_file.contains(f)), | ||
) | ||
.map(|f| f.to_owned()) | ||
.collect::<Vec<_>>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unrelated cleanup because I had trouble reading this code ("from_config_file")...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This adds support for using a local copy of an npm package. ```js // deno.json { "patch": [ "../path/to/local_npm_package" ], // required until Deno 2.3, but it will still be considered unstable "unstable": ["npm-patch"] } ``` 1. Requires using a node_modules folder. 2. When using `"nodeModulesDir": "auto"`, it recreates the folder in the node_modules directory on each run which will slightly increase startup time. 3. When using the default with a package.json (`"nodeModulesDir": "manual"`), updating the package requires running `deno install`. This is to get the package into the node_modules directory of the current workspace. This is necessary instead of linking because packages can have multiple "copy packages" due to peer dep resolution. Caveat: Specifying a local copy of an npm package or making changes to its dependencies will purge npm packages from the lockfile. This might cause npm resolution to resolve differently and it may end up not using the local copy of the npm package. It's very difficult to only invalidate resolution midway through the graph and then only rebuild that part of the resolution, so this is just a first pass that can be improved in the future. In practice, this probably won't be an issue for most people. Another limitation is this also requires the npm package name to exist in the registry at the moment.
This adds support for using a local copy of an npm package.
"nodeModulesDir": "auto"
, it recreates the folder in the node_modules directory on each run which will slightly increase startup time."nodeModulesDir": "manual"
), updating the package requires runningdeno install
. This is to get the package into the node_modules directory of the current workspace. This is necessary instead of linking because packages can have multiple "copy packages" due to peer dep resolution.Caveat: Specifying a local copy of an npm package or making changes to its dependencies will purge npm packages from the lockfile. This might cause npm resolution to resolve differently and it may end up not using the local copy of the npm package. It's very difficult to only invalidate resolution midway through the graph and then only rebuild that part of the resolution, so this is just a first pass that can be improved in the future. In practice, this probably won't be an issue for most people.
Another limitation is this also requires the npm package name to exist in the registry at the moment.
Closes #27854
Closes #19621
Closes #15624