-
Notifications
You must be signed in to change notification settings - Fork 81
fix: update deno run commands to set node-modules-dir to manual #6771
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
Conversation
aaea002 to
962297f
Compare
| (Netlify Build completed in 1ms)␊ | ||
| Build step duration: Netlify Build completed in 1ms` | ||
|
|
||
| ## In integration dev mode, install local plugins and install the integration when forcing build |
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.
How come this is getting removed?
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 test does not exist - which is why the snapshot was removed when I regenerated snapshots
Reorders the code to add vendor import maps before bundle generation starts. This ensures the import map is properly initialized with vendor dependencies before any bundling logic executes, preventing potential issues with missing dependencies during the bundling process.
3893be8 to
654f9fc
Compare
654f9fc to
f7ad345
Compare
| if (vendor) { | ||
| importMap.add(vendor.importMap) | ||
| } | ||
|
|
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 fixes a bug where we were not including npm/vendor dependencies for our tarball bundles
| '--quiet', | ||
| '--code-splitting', | ||
| '--allow-import', | ||
| '--node-modules-dir=manual', |
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 fixes the bug where Deno was adding a .deno folder into the node_modules folder, which caused issues for our hosted build system as it would cache the .deno folder - which is not a good thing to do because the package versions that Deno was selecting were not adhering to the Netlify Edge Functions' lockfiles (if those lockfiles were from bun/pnpm/yarn/npm)
Adds the
--node-modules-dir=manualflag to deno bundle commands in tarball bundling.Docs: https://docs.deno.com/runtime/fundamentals/node/#choosing-a-node_modules-mode
This will stop our bundling step from accidentally adding a
.denofolder into thenode_modulesfolder, which was causing issues for builds which use a cachednode_modulesfolderThis was a bit difficult to test out locally, I instead did a manual test using buildbot via https://github.com/netlify/buildbot/pull/3874 and then making a site's build use this custom buildbot image