-
Notifications
You must be signed in to change notification settings - Fork 2.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
fix(dev): support polyfill package imports in deps #7198
fix(dev): support polyfill package imports in deps #7198
Conversation
🦋 Changeset detectedLatest commit: 05364f8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 16 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
)} before continuing.` | ||
); | ||
} | ||
return nodeBuiltins.filter((mod) => !dependencies.includes(mod)); |
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 filter was redundant because an error would have already been thrown if this was the case. This is because this filter is the inverse of fakeBuiltins
.
@@ -36,7 +36,7 @@ | |||
"chokidar": "^3.5.1", | |||
"dotenv": "^16.0.0", | |||
"esbuild": "0.17.6", | |||
"esbuild-plugins-node-modules-polyfill": "^1.3.0", | |||
"esbuild-plugins-node-modules-polyfill": "^1.4.0", |
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 introduces the new fallback: "empty"
option: imranbarbhuiya/esbuild-plugins-node-modules-polyfill#146
🤖 Hello there, We just published version Thanks! |
🤖 Hello there, We just published version Thanks! |
Fixes #5521
This error was happening because all Node built-ins were marked as external. This meant that imports for
"buffer/"
(which is supposed to force the import of the package rather than the Node built-in) would be left as-is in the client bundle.I added a feature upstream to esbuild-plugins-node-modules-polyfill (imranbarbhuiya/esbuild-plugins-node-modules-polyfill#146) which allows us to inject empty fallbacks instead. This fixes the issue because it only matches on exact import specifiers (i.e.
"buffer"
matches as a Node built-in, but"buffer/"
doesn't).