-
Notifications
You must be signed in to change notification settings - Fork 810
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
Upgrade esbuild
to 0.19.0
+
#5222
Comments
Hopefully it's appropriate to start noting real-world problems this is starting to cause.
|
Resurfacing from #2980 wrangler is using I hit this again today when trying to do something like this: import package from '../package.json' with { type: 'json' } wrangler throws:
from the old esbuild version. But manually bundling with even just version 0.1912: works fine. |
The workaround (for now) is to use overrides in your package.json. For example, if you're using npm: // ...
"overrides": {
"esbuild": "0.21.4"
},
// ... |
This is also blocking for the |
This was addressed in #6884, and is available to test in |
Following on from #5213, this issue tracks upgrading the
esbuild
version of all tools inworkers-sdk
includingwrangler
to0.19.0
and above.0.19.0
adds support for bundling import paths containing wildcards (e.g.await import("./data/${lang}.json")
will bundle all files matching./data/**/*.json
, then select the correct module at runtime). Wrangler currently allows modulerules
to support these kinds of variable-based dynamic imports. Wildcard imports cannot be disabled, and do not triggeronResolve()
plugin hooks (if they did, we could mark them asexternal
effectively disabling this feature).Potential paths forward
Option 1: integrate wildcard imports with custom module rules
In Wrangler's module collector, we would continue to mark modules matching module rules as
external
. Whilst modules imported via wildcard imports don't triggeronResolve()
plugin hooks, they do triggeronLoad()
hooks for each matched module. If anonLoad()
hook was triggered, we'd know the module must've come from a wildcard import as theonResolve()
hook marks collected modules asexternal
, andexternal
s don't triggeronLoad()
. We could add anonLoad()
hook matching module rules that re-exported the module from an externalimport()
. We'd have to be very careful here not to convert dynamic imports into static imports (i.e. we don't wantawait import(...)
to become multipleimport ... from "..."
s). Users rely on dynamic imports to lazy-load parts of their code and we don't want to prevent this.Even without additional module rules, the wildcard import feature allows files
esbuild
natively understands to be bundled. For example,await import("./data/${lang}.js")
would be bundled without any additional configuration from the user. With our currentesbuild
bundle, this would result in a runtime error as the module wouldn't be included in the Worker upload. With this in mind, the wildcard import feature is providing functionality that wasn't previously supported. However, there is a danger it bundles too many files (e.g.node_modules
or files that would cause a build time error because of unsupportednode:*
modules). Workers have a strict size limit, and wildcard imports could easily create bundles exceeding this.If we enabled this feature, we'd likely want to inform affected users. Currently,
esbuild
generates output that looks like this for wildcard imports:We could regexp on
// import("...") in ... \n __glob(
in the output to warn users when a wildcard import was bundled.Option 2: mark modules matched by custom module rules as top-level external
Instead of implementing the module collector as an
esbuild
plugin, we could run it beforeesbuild
, then pass its collected modules to the top-levelexternal
option. Whilst wildcard imports don't triggeronResolve()
hooks, they do respect this option. For example, settingexternal: ["*.wasm"]
, then includingawait import("./data/*.wasm")
in source code results in the following output:If we did this, we'd need to restart
esbuild
whenever new modules matchingrules
were added. This option still has a danger of bundling too many files likeOption 1
.Option 3: submit PR to
esbuild
to allow this feature to controlledIdeally, a mechanism would be added to
esbuild
to disable wildcard imports. This could either be via a top-level build option or via a plugin hook. A build option would be relatively simple to implement, but having the ability to control this feature via a plugin would be even better. There are a few ways this could be supported:onResolve()
with the glob pattern generated from the template literal/string concatenation asargs.path
, and allowonResolve()
to return multiple module paths in this case. If theonResolve()
hook returned{ external: true }
, theimport()
statement would be bundled verbatim. This would also allow wildcard imports to be supported byesbuild
running in the browser, which relies on plugins to provide a virtual "file system".onGlobResolve()
hook with similar behaviour to the above.onResolve()
with each module matching the glob pattern. Bundling wildcard imports can be thought of as a three step process: first the import is converted to a glob pattern, second the pattern is evaluated and becomes__glob({ ...: () => import(...), ... })
, third the new__glob()
code is bundled as usual. This intermediate second stage can be seen when using the top-levelexternal
option as inOption 2
. Arguably,onResolve()
should be called on each of these intermediateimport(...)
statements, which would allow us to mark them asexternal
.Option 4: consider switching to another bundler
If none of these options work, we could consider another more-flexible bundler like Rollup or the newer Rolldown when that matures. We could also consider switching to a "bundle-less" dev approach like Vite, potentially using the module-fallback service in a similar way to the new Vitest integration. This would move away from
wrangler dev
running pretty much the same code that gets deployed, which is a nice guarantee to have (though arguably already broken by the middleware system).The text was updated successfully, but these errors were encountered: