Open arcanis opened 3 years ago
This is a good suggestion. It matches with some of my thinking for some other improvements I'm planning for CSS as well. AST manipulation plugins are problematic because they require synchronous access to the AST while macro-style plugins like this involve less communication traffic and are easier to parallelize.
I don't think I understand what data is represented by the path in your example. It might be sufficient to just provide the AST as a string containing the source code for the import()
expression itself and let the plugin take it from there. That avoids having to specify an AST format and is also more efficient to communicate.
Perhaps something like this:
build.onDynamicImport({ ...some filtering options TBD... }, args => {
args.expression; // The raw source code of the import() expression
args.importer; // The path of the importing file
args.namespace; // The namespace of the importing file
args.resolveDir; // The resolve directory of the importing file
return {
contents, // A module whose default export is the function to call to implement the dynamic import
};
});
So you might do something like this (using the acorn parser for the sake of example):
build.onDynamicImport({}, args => {
const ast = acorn.parseExpressionAt(args.expression, 0, {ecmaVersion: 2020})
assert.strictEqual(ast.type, 'ImportExpression');
const contents = `
export default async (source) => {
${/* do something with ast.source */}
}
`
return { contents }
});
This should be future-proof for import()
potentially taking multiple arguments if import assertions add additional syntax. And having the raw source code lets you parse out any special magic Webpack comments that your plugin may want to interpret. It's not fully general-purpose because you won't be able to handle dynamic imports that rely on the context of the surrounding code the way a normal AST-style plugin might work in other bundlers. But if you want to do that, you can always just write a custom on-load plugin instead and preprocess the whole source file.
I'm imagining that esbuild would transform something like this:
import(something).then(
result => console.log(result),
error => console.error(error),
)
into something like this:
import importShim from '...auto-generated module representing the future result of calling the plugin...'
importShim(something).then(
result => console.log(result),
error => console.error(error),
)
and then the plugin would be called asynchronously. The parser can finish parsing the file without waiting for the plugin. The benefit of replacing the import()
expression with a direct call to the shim function instead of with another import()
expression to load the shim is that it avoids a potential double round trip over the network to load a module dynamically.
I don't think I understand what data is represented by the path in your example. It might be sufficient to just provide the AST as a string containing the source code for the
import()
expression itself and let the plugin take it from there.
I like that - I'm not sure the Webpack comment would really be implementable (they tend to have an effect on the generated files themselves; for example loaders would have no way to name the chunks esbuild would generate), but getting the AST is good enough for most other use cases.
The benefit of replacing the
import()
expression with a direct call to the shim function instead of with anotherimport()
expression to load the shim is that it avoids a potential double round trip over the network to load a module dynamically.
That makes sense - if one really needs the dynamic list to be loaded dynamically (for instance if there's a very large list of possibilities that you really don't want in the main bundle), I guess it can be done in a way by generating an intermediary module that would resolve to a specially crafted dynamic import, which itself would be caught by the regular resolution/loader pipeline for further transformation. But it's probably kind of a fringe case.
I wonder if esbuild could take whatever knowledge it has about constant bindings in that scope and also thread those through so that the expression evaluator could be aware of, for example, string constants from parent scopes used as the prefix.
@evanw I understand this feature might not be a priority at the moment (which is fine - the other improvements I've seen land are just as useful!) - since it's currently blocking us in our adoption, I was wondering whether it's something you think could be implemented by external contributors? I have very little Go knowledge, but perhaps with a few pointers I could give it a try, since it seems to be mostly an extension of what loaders already do.
Same here. I'm thinking of switching over to esbuild, but missing dynamic imports is the primary issue preventing this right now.
@evanw Thanks for pointing me to this issue — I should've searched a bit better before creating #1178.
Your spec looks great and I think it solves everything I pointed out in my issue.
I'd love to contribute to this. Would you be open to a PR?
@evanw I hope you don't mind me following up on this, but I would really love to see this landing in esbuild. Not being able to handle dynamic imports is the biggest hurdle we're currently dealing with when it comes to adopting esbuild. From the comments in this issue, it looks like a lot of folks feel the same.
I started implementing the API you described in https://github.com/evanw/esbuild/issues/700#issuecomment-765094384, but before getting too deep into it I'd like to confirm that you're still okay with that approach and also check whether you'd welcome a PR.
This is a significant piece of work, and I suspect you already have a mental model of how you'd implement it, so I could start a draft PR and start adding bits of functionality iteratively, under your guidance. Would that work for you?
Thanks again!
We'd also like to see this feature for Ionic and Stencil, since they use dynamic imports to only load what's being used at runtime, but currently prevents Ionic from using esbuild: https://github.com/ionic-team/ionic-framework/issues/22924
macro-style plugins like this involve less communication traffic and are easier to parallelize.
I think this is an interesting idea. Obviously hooking into import()
expressions is a big use case, but any thoughts on supporting a generalized hook for arbitrary call expressions to allow for babel-plugin-macros
-esque plugins?
For example, instead of the existing fibonacci plugin:
import x from 'fib(10)'
could instead be expressed in a more natural manner:
import fib from "fib.macro"
let x = fib(10)
Fibonacci is obviously contrived, but there are many useful macros for graphql, css-in-js, etc. that take advantage of these kind of compile-time abstraction. Might be out of scope for esbuild, but possibly worth considering.
any thoughts on supporting a generalized hook for arbitrary call expressions to allow for
babel-plugin-macros
-esque plugins?
Sort of. I'm planning on experimenting with this using CSS first since CSS is a much simpler language and is more suitable for this (the AST is basically a nested token tree, and there are perhaps no macro hygiene concerns?). CSS is also kind of unusable at scale without an abstraction mechanism, which is not the case for JS, so macros are more important. But these would perhaps be two different mechanisms since you can't import something in CSS to defer its evaluation the way the JS proposal above works (since CSS has no abstractions).
One question is how to make this efficient, especially if the work cannot be deferred like this JS proposal. One idea could be to allow some special cases for certain common operations such as unconditionally substituting some tokens, potentially with placeholders for some of the arguments. Then maybe those special cases could be done inline in the core without having to call out to a plugin. And there would be a fallback that always calls the plugin to support more general behavior. But I don't want to create any API around something like this without doing the R&D first.
TL;DR: I've thought about this some and it could be cool. I think a general macro system may have merit for CSS and I think a special-cased macro system like the one proposed here makes sense for JS, but doing a general macro system for JS seems pretty complicated (e.g. with hygiene) and I'm not sure I want to expand esbuild's scope that much, at least not right now.
I started implementing the API you described in https://github.com/evanw/esbuild/issues/700#issuecomment-765094384, but before getting too deep into it I'd like to confirm that you're still okay with that approach and also check whether you'd welcome a PR.
I appreciate you putting effort toward this. I haven't had time yet to look at the PR and it's getting late so I'm not going to get to it tonight. I hope to look at it soon. Some quick thoughts for now:
require()
and dynamic import()
.onDynamicImport
might not be the best name given some of the points above.I appreciate you putting effort toward this. I haven't had time yet to look at the PR and it's getting late so I'm not going to get to it tonight. I hope to look at it soon. Some quick thoughts for now:
- We should finalize the API before we get into this.
- It should work for both dynamic
require()
and dynamicimport()
.- I'm guessing that for performance reasons people will want ways of roughly including/excluding ones that clearly don't apply, probably via regex like other esbuild plugin callbacks. This doesn't need to be implemented in the first iteration but the design should be able to extend to those.
- I wonder if some people will want plugins that run even for static imports. For example to do crazy things like #1240. Not sure how to address that or if that should just be out of scope.
- The name
onDynamicImport
might not be the best name given some of the points above.
Currently, #1273 only handles dynamic require()
. I'd love your feedback on the approach before implementing it for import()
.
In terms of filtering capabilities, I kept build.onDynamicImport
with an object parameter, so we can add one or more properties for people to include/exclude certain expressions further down the line.
I agree that onDynamicImport
is probably not the best name. Perhaps something like onDynamicResolve
or onDynamicExpression
?
I just realised that resolveDir
is missing from the list of arguments sent to the plugin handlers. I'll add it to the PR.
@evanw Sorry for pestering. I appreciate that you're busy pushing other features, but it's getting a bit difficult for me to keep #1273 up-to-date while waiting to get some feedback. After merging the latest main branch into my fork, I realised that tests are failing after #1291, because it directly affects the logic that handles dynamic requires.
If there's anything I can do to help you review the PR and the general approach, please let me know.
I’ve been mulling an idea like this the last few days, with no idea other people were thinking/acting on it. I’m glad to see an active, open issue with some prior thought on the topic!
My (naive, wrong) thinking was that plain functions could be used to propogate dynamic import calls with build-runtime data, which could be code split moving that runtime elsewhere (and potentially enabling further processing eg CSS-in-JS virtual CSS modules). E.g.
const foo = (data) => import(`data:${serialize(data)}`)
Obviously this doesn’t work for a variety of reasons. This issue and discussion gets to most of the heart of those reasons, but to highlight core issues:
I think in a general sense what would be useful here from a plugin perspective is to relax aspects of the AST non-goal:
I think other than composability (which plugin piping addresses) this resolves everything a dynamic virtual plugin should need without an open ended AST interface. And I think/hope it could still be parallelized with namespacing and still be within the ESBuild design goals.
I’ll add that part of my motivation for addressing this is the performance-focused goal of ESBuild itself. But another part is around forming a less tooling-coupled solution to code transforms/codegen. One of my biggest pain points doing frontend dev is having multiple builds with slightly different behavior/semantics, and where some of them expect a runtime. If the runtime could start from a data:
URL, and build tools optimize with a subset of the AST as needed, this could make other tooling a lot more portable between environments. It could empower the next 3x (or whatever) perf leap for the next new bundler hotness, without holding it back waiting for a mature plugin ecosystem.
Just jumping on here to say I've also been playing round with esbuild on a current project that has some relatively old package dependencies and I'm falling over when it comes to dynamic import / require. Keen to follow the discussion on this thread and see if we can get esbuild to a point where it can take the place of Webpack in our dev and build workflows. Ultimately, it would be nice if package managers could move to ES6 (with types) but I think it would be a powerful addition if esbuild could deal with the hybrid world we are going to be saddled with for a while with ESM and CJS modules...
In case anyone is interested, Netlify is running a fork of esbuild in production, which includes the onDynamicImport
hook proposed in https://github.com/evanw/esbuild/pull/1273. We'd love to get this (or any variation of it that @evanw is comfortable with) merged into the main project and decommission the fork.
Until then, everyone is more than welcome to use the fork. We also welcome feedback on the implementation.
This looks interesting @eduardoboucas, thanks for sharing. I'll have a look and see if I can get something working with this in the short term.
I have gone through the discussion still can't find a way to bundle dynamic imports
, however importing the module at the beginning of the file bundles the correct file but it will increase the size of bundled app.js
, do you have a solution for this ?
For me the issue is some NPM packages we depend on use dynamic imports and I'm dearly hoping we can get this into esbuild so we can speed up our dev and devops pipelines.
I'd also love to see some form of dynamic imports in esbuild!
Can you have some type of config to define potential dynamic import paths kind of like how webpack does it? Even if we have to manually define the first part of the path so it's easier for esbuild to make guesses.
Without dynamic imports esbuild is currently unusable for me, which sucks because its better than others in every other regard
I fully appreciate the challenges of building open source software. As users, we love what works and get frustrated with what doesn't, we all have day jobs... esbuild is quality software. We use Vite but we also depend on older NPM packages which we don't want to be shackled to webpack for.
This may be a niche issue (I don't know if it is or not), but it would be great to have some comment on if or how we might move this forward. I don't personally have the skills to help but would dearly love to if I did.
Is there a way that i can help to get this working in esbuild? Or can others jump in that already have prototypes?
This case started over 3 years ago now and still we don't have this, now that angular has builders based on esbuild (like the new application builder), which is really way faster, i wanted to move over The end result was not so nice, and i have to really maintain this now to look if more files are added by the 3rd party component, for example Numbro that we include:
for now i left the angular to be dynamic: https://github.com/Servoy/servoy-eclipse/blob/master/com.servoy.eclipse.ngclient.ui/node/src/ngclient/locale.service.ts#L161
and copy the files ourself, happily that works for those files because they are already .mjs files so in the right EMS module all others like Numbro are commonjs so they are nicely rewritten by the builder so the import works, if i would just copy them as assest my self and try to import then it wouldn't work that way (numbro would add itself to window.number[locale])
the same holds true for: https://github.com/Servoy/bootstrapcomponents/blob/master/projects/bootstrapcomponents/src/calendar/basecalendar.ts#L181 where we use a 3rd party calendar field that also have locale files (that are i think in commonjs notation)
and uppy:
which i think is already in ESM format.
But why is this so difficult todo or implement, seems to me everything already is their, the right files are generated if it is a static import.. so from a high level point of view i would say:
import('static')
that would result in given the static url to something that generates the file and adds to the build/system if you then have
import(${dynamic}
);
i would say notice that and replace the variables with * and do a listing of everything you get and call one on one the same code as above. (so it looks like you have import statements of all of them in code)
Is there any progress on this issue? Has anyone found a workaround to solve this?
there is an example:
const puppetFileList = [
'../../out/wechaty/deprecated/file-box_pb.js',
'../../out/wechaty/puppet/base_pb.js',
'../../out/wechaty/puppet/contact_pb.js',
'../../out/wechaty/puppet/download-upload_pb.js',
'../../out/wechaty/puppet/event_pb.js',
'../../out/wechaty/puppet/friendship_pb.js',
'../../out/wechaty/puppet/location_pb.js',
'../../out/wechaty/puppet/message_pb.js',
'../../out/wechaty/puppet/mini-program_pb.js',
'../../out/wechaty/puppet/referrer_pb.js',
'../../out/wechaty/puppet/room_pb.js',
'../../out/wechaty/puppet/room-invitation_pb.js',
'../../out/wechaty/puppet/room-member_pb.js',
'../../out/wechaty/puppet/tag_pb.js',
'../../out/wechaty/puppet/url-link_pb.js',
'../../out/wechaty/puppet_grpc_pb.js',
'../../out/wechaty/puppet_pb.js',
]
/**
* Huan(202108):
* if there's a "package.json" file in the `out/` directory,
* then all the files in the `out/` directory will be treated as one module,
* which means tht `require` each file under that directory will add methods to the same module.
*/
// for (const pkg of pkgs) {
// console.info('## pkg:', pkg)
// const module = require(pkg)
// console.info(Object.keys(module).length)
// // OOPS! The output number above will be keep increasing
// }
module.exports = puppetFileList.reduce((acc, pkg) => ({
...acc,
...require(pkg),
}), {}) // Huan(202108): must provide a `{}` as the initial value, or it will be `[]`
I know there have been multiple threads on the subject (#661, #453, #322, #56), but since plugins are now a thing I figured it could be worth opening one more discussion. Perhaps there could be a solution that doesn't require statically pregenerating files? At least I haven't seen my suggestion elsewhere 😄
ESBuild currently warns when it detects dynamic imports, and leaves them in-place. It's a good start, but it's not enough: whatever is tasked from consuming them will still lose critical information, in particular the location the dynamic import is relative to (for instance, when doing an
import('./widgets/' + name)
into an application compiled via--bundle
, I really need to know where was the file that made this import).My suggestion to support dynamic imports with only a little extra API surface would be to instead yield the generation of an intermediary dynamic import module to a special type of loader. For example, the following loader would allow dynamic imports where each module would end up in their own chunk:
On the other hand, if I wanted to make a dynamic import where all modules are in the same chunk, then I'd just need my loader to generate regular
import from
statements, without the consumer code having to know about this. I feel like this might match the kind of flexibility you had in mind given how loaders currently work.