-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Implement onDynamicImport
plugin hook
#1273
Conversation
Update from upstream
I've updated my branch with the changes introduced by #1291, so the implementation now respects the shim. I suspect there's a better way of finding the dynamic expression imports in the printer that doesn't involve using |
33fc625 and 539af45 introduce the following changes:
|
@evanw We've been running a forked version of esbuild in production for a while now, which includes this plugin hook. It works pretty well and it allows us to resolve at runtime the vast majority of Would you be willing to give this another look? Again, I'm happy to make any changes you think are necessary for this to land. |
@evanw Is there any update on accepting/reviewing this PR? This is basically the last thing needed for our tech org to adopt esbuild over webpack. Graphql tools uses some dynamic imports, making it essentially impossible to use with esbuild. By the way: I love esbuild. It's insanely impressive what you're able to accomplish on performance and simplicity |
@evanw I'd also like to request that this PR be reviewed/accepted. My org is considering a move away from webpack, and esbuild looks very promising. However, we have a lot of code that depends on dynamic imports, so this is a non-starter for us without this PR, and ideally some sort of plugin being merged into esbuild first. |
@evanw this would be a life saver for me. |
This is the only feature missing for us… 😢 Any chance we could get this PR merged soon ? |
Jumping in, I also needed this for my rails + vue app. 🙏 |
FYI I'm planning on releasing a form of this sometime soon, but built into esbuild instead. You can see my work in progress version here: #2508. |
Closing in favour of #2508. |
This PR implements an
onDynamicImport
plugin hook as per #700 (comment).This allows consumers to, first of all, detect when an import contains a dynamic expression, which can potentially lead to a broken build. This in itself is already useful, since detecting this became impossible after v0.11.11.
More importantly, it allows plugin authors to receive the dynamic expression and make decisions based on its contents, with the option to provide a replacement module that will be responsible for providing the runtime implementation for the expression.
Example:
Imagine an entry point with the following code:
One could write a plugin for processing this expression like so:
In a nutshell, the implementation works as follows:
When the parser finds a
require
with a dynamic expression, it extracts its string value using a newRangeOfCallArgs
method, and flags the import record as dynamic.In the scan phase, we look for any import records with dynamic expressions and run any
onDynamicImport
plugin hooks. The plugin hook execution for different imports is fully parallelised. Within each import, we run the plugins in serial until one of them "claims" the plugin (i.e. returns acontents
property).At the end of scanning each file, we collect the results generated from any
onDynamicImport
plugins and generate a path for the replacement module using theDynamicImportPathTemplate
path template, which by default contains the name of the entry point and the hash of the replacement module for determinism.With that path, we add an entry to
AdditionalFiles
so that the replacement module is written to disk.We also add the path to the import record's entry in the AST, so that the parser knows how to rewrite the
require
callWhen the printer finds a
require
call for a dynamic expression (E
), it checks whether there is a path for a replacement module (P
) defined in the AST:P
exists, it printsrequire(P)(E)
P
does not exist, it prints the originalrequire(E)
I've tried my best to respect the existing primitives, structures and conventions, but I'm sure there's lots of room for improvement in this initial draft PR. Also, it currently only works for
require
calls. However, I'd love to get some feedback on this before implementing it forimport
statements.I have added two tests to the plugin test suite that demonstrate the behaviour, but more tests will be needed (both in the TypeScript and Go sides) before any of this is merged.
I'm happy to answer any questions. Thanks in advance!
Implements #700.