Skip to content

fix(compile): only include used npm packages in compiled vfs #29338

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

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

nathanwhit
Copy link
Member

Fixes #19764.

Interaction w/ non-statically-analyzable paths might be problematic, maybe we flag this? Though I think we'll have that problem with most solutions involving bundling/slimming down node_modules

@nathanwhit nathanwhit requested a review from dsherret May 16, 2025 18:10
@dsherret
Copy link
Member

Interaction w/ non-statically-analyzable paths might be problematic, maybe we flag this?

Yeah, I think it should be an opt-in flag so that it works by default and maybe doesn't work with an optimization.

@@ -767,6 +791,7 @@ impl<'a> DenoCompileBinaryWriter<'a> {
// we'll flatten to remove any custom registries later
let mut packages = npm_resolver
.resolution()
.subset(reqs)
Copy link
Member

Choose a reason for hiding this comment

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

We'll need to think of something for when there's a node_modules directory.

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.

deno compile binaries should not include @types/node in the npm virtual filesystem
2 participants