-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
WIP for version 0.9.0 #924
Conversation
This plan looks amazing! Thank you! Please, let me know if I can contribute with something, could work on some tasks for the JS APIs for example. |
8567643
to
ee678c6
Compare
Just to say on "Potentially roll back the file splitting optimization": If at all possible to retain this feature, please do. To me, it's ESBuild's killer feature over all other bundlers. Please don't take this as a demand! If it needs to be cut, so be it. But just wanted to let you know that, for some users at least, this is a highly valued aspect of ESBuild. |
I'm pulling it because I need to get TLA in since it's part of the language now and that feature is getting in the way. It's possible that it could be added back in a different way later on. But trying to solve for everything all at once is getting to be too nuts. I'm trying to reduce scope to make it more feasible. |
Fair enough. So the decision is made, and it will be removed in 0.9.0? |
That change is scheduled for 0.10.0, not 0.9.0. |
internal/resolver/resolver.go
Outdated
// want problems due to Windows paths, which are very unlike URL | ||
// paths. We also want to avoid any "%" characters in the absolute | ||
// directory path accidentally being interpreted as URL escapes. | ||
resolvedPath, status, token := esmPackageExportsResolveWithPostConditions("/", esmPackageSubpath, pkgJSON.exportsMap.root, nil) |
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.
Where do the conditions
come from? AFAICT this is the only initialization point for the exports
resolution phase
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.
They were added in the very next commit. The first commit only respects the default
condition and then the next commit adds browser
, node
, import
, and require
.
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.
Oh, haha 😅 thanks I'll look now
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.
Looks great! The only thing I'm not seeing – i think – is support for custom conditions?
For example, webpack adds development
-vs- production
conditions & a lot of people are making use of that already. I've also seen (and have authored) modules with a worker
condition. Technically anything can be a condition, and so long as resolver it willing to look at it & the pkg author has defined it in the correct order-hierarchy, then it should be matched.
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.
I'm already aware of the need for conditional exports. I'm still working on the 0.9.0 release and just haven't implemented them yet.
|
One of the problems with the require-ESM feature is that I believe the semantics are ill-defined the way it is currently implemented in esbuild. For example, consider these files: // entry.js
let fn = () => require('./a')
console.log('entry')
fn() // a.js
console.log('a')
import './b' // b.js
console.log('b') What console output do you expect? |
I know hybrid esm|cjs behavior is not well defined, but it's useful for write custom bundler and webpack also supports it , or do you have any idea how to escape the esm module hoist things, what I really want is inline require support in rollup/plugins#481 so as to insert some module to existing cjs|esm module. |
Inline export default require('./file.cjs'); As a demonstration that this is well-defined, you can even call import { createRequire } from 'module';
const require = createRequire(import.meta.url);
export default require('./file.cjs'); |
ac70415
to
4730387
Compare
Ok I will hold back on the "remove the ability to use |
I'm using this PR to put together the next breaking change release as I build it. Not sure if a PR is the best format for this but I'm giving it a shot. So this PR exists to publicly document what's going into the upcoming version 0.9.0 release. I will be updating it as I go.
For context: I have been trying to batch breaking changes together so that they are less disruptive. But they have been piling up because one of the big changes (the linker rewrite) is unfortunately taking longer than I had hoped. I'd like to do a breaking change release now to get some of these breaking changes out even though it won't contain the linker rewrite just to keep other things moving along. This might mean there are at least two breaking change releases coming up: this one and another one with the linker rewrite. Note that the next version after version 0.9.0 will be version 0.10.0, not version 1.0.0. It's definitely getting close to 1.0.0 but it's not quite that close.
The rewrite of the linker is being done to address many shortcomings of the current linker all at once. However, that has involved some significant R&D effort (e.g. 1, 2, 3, 4) and it is unfortunately still a work in progress. I did a brain dump of some information about the linker rewrite at the end of this post.
Definitely in version 0.9.0
Remove the now-unused
--avoid-tdz
flagThis flag no longer does anything because this transformation is now always applied during bundling. It hasn't been removed yet to avoid a breaking change.
Remove
SpinnerBusy
andSpinnerIdle
from the Go APIThese options were part of an experiment with the CLI that didn't work out. Watch mode no longer uses a spinner because it turns out people want to be able to interleave esbuild's stderr pipe with other tools and were getting tripped up by the spinner animation. These options are currently ignored but haven't been removed yet to avoid a breaking change.
Remove the
--summary
flag and instead just always print a summary (Summory option in.build()
#704)The summary can be disabled if you don't want it by passing
--log-level=warning
instead. And it can be enabled in the API by settinglogLevel: 'info'
. I'm going to try this because I believe it will improve the UX. People have this problem with esbuild when they first try it where it runs so quickly that they think it must be broken, only to later discover that it actually worked fine. While this is funny, it seems like a good indication that the UX could be improved. So I'm going to try automatically printing a summary to see how that goes.Remove the
startService()
API, leaving onlybuild()
andtransform()
The context is that the
startService()
API was added quickly in response to a user request and at the time I was unaware ofunref()
which makes it possible to use a child process without requiring people to callservice.stop()
. Many thanks to @SalvatorePreviti who contributed this change in Allows NodeJS to terminate also if there is an active service. #656. Callingservice.stop()
no longer does anything, so there is no longer a strong reason for keeping thestartService()
API around. The primary thing it currently does is just make the API more complicated and harder to use. I plan to add aninitialize({ wasmURL })
API to replace the configuration ofwasmURL
in the browser that used to be done bystartService()
. And thebuildSync()
andtransformSync()
APIs will still exist on node.Split the banner and footer features into separate values for JS and CSS (banner and footer flags are not honored in minified CSS #712)
I have been trying to make sure all of the features added are language-agnostic where it makes sense. However, the banner and footer features were an oversight and were added as a JavaScript-specific feature. They don't currently work with CSS. I plan to change the API to make it a mapping of language to value to make it more language-agnostic, which is a breaking change.
Probably in version 0.9.0
Remove the implicit
.mjs
and.cjs
extensionsI added these because Webpack has implicit
.mjs
extensions and it seemed like a reasonable idea to support these at the time. However, doing this can actually break packages that havesome-file.mjs
andsome-file.js
sitting side-by-side. This also isn't how node works so doing this breaks compatibility with node. So it feels like this should be an opt-in change instead of an opt-out change. I'm going to try removing them from the default configuration. You can still configure them yourself with the "resolve extensions" setting if you'd like, although it might break stuff. Otherwise you should use the.mjs
and.cjs
extensions explicitly instead of omitting them.Remove the ability to use
require(...)
on a file in ESM (ECMAScript module) formatI had originally enabled this because it seemed reasonable to support. You can just convert the ESM file to a CommonJS file when you hit this case and then load it as CommonJS instead. However, this has a few problems:
This isn't how node works. Doing this in node is impossible and will cause a syntax error. So it's kind of a weird thing to support and there isn't really a precedent that says how it should behave. I want esbuild to follow existing conventions instead of inventing new ones, and it turns out this was kind of inventing a new convention. It feels best for esbuild and for the ecosystem if people don't come to rely on this weird thing actually working, so I'm going to try disabling this.
Doing this makes import order really confusing to reason about. Part of what I am doing with the linker rewrite is to be much more strict about having esbuild follow the correct import order, but this edge case doesn't have a well-defined import ordering. The problem is that ESM import order is determined by a depth-first traversal of the import graph and is entirely statically determined at link time, but
require(...)
is dynamically-determined at run time. Where do ESM imports in the required file end up in the static module initialization order? Do they even end up in the static module initialization order at all? Should they also be converted from ESM to CommonJS recursively, potentially contaminating a large part of the bundle? There isn't really any existing ESM-native platform that behaves this way.It also causes extra issues now that TLA (top-level await) is a part of JavaScript. TLA is only available in ESM files and means that loading them can now be asynchronous. But
require(...)
is synchronous so loading an ESM file with a TLA file anywhere in its dependency chain must be forbidden. If usingrequire(...)
with ESM were to be supported then you may be seemingly arbitrarily prevented from adding anawait
to a file because some other file in a seemingly unrelated place happened to use arequire(...)
call. It seems better for code health to just avoid this possibility in the first place. This constraint also makes the bundler simpler to implement.Getting this change out should hopefully make the linker rewrite go more smoothly.
Maybe in version 0.9.0
Add support for node's
exports
field inpackage.json
(Supportexports
syntax in package.json #187)This feature was recently added to node. It allows you to rewrite what import paths inside your package map to as well as to prevent people from importing certain files in your package. Adding support for this to esbuild is a breaking change (i.e. code that was working fine before can easily stop working) so it must be done in a breaking change release.
Remove the
metafile
fromoutputFiles
(Feature Request: Build metadata as first-class, typed property of build result #633)Right now using
metafile
with the API is unnecessarily cumbersome because you have to extract the JSON metadata from the output file yourself instead of it just being provided to you as a return value. This is especially a bummer if you are usingwrite: false
because then you need to use a for loop over the output files and do string comparisons with the file paths to try to find the one corresponding to themetafile
. Returning the metadata directly is an important UX improvement for the API. This hasn't been done yet because doing this is a breaking change.Support content hashes in entry point names ([Feature] Toggle
contenthash
for all output filenames #518)Support for this is already planned. The plan is to add the
--entry-names=
flag and let you put the[hash]
placeholder somewhere inside it to cause esbuild to include the content hash in entry point names. You can then use themetafile
to figure out what the output path was for each entry point. However, this would introduce cycles into the content hash calculation as the content hash includes not just the current file but also all of the content hashes of all transitive dependencies and entry points can reference each other using dynamicimport(...)
expressions.Addressing this requires a different approach to chunk generation that is part of the linker rewrite. This technically isn't a breaking change but it has a strong possibility of accidentally introducing bugs (especially with source maps) since it's a lot of new code, so it may also be appropriate for a breaking change release. Although it could potentially make sense to hold this back for the next breaking change release to avoid delays.
Hopefully in version 0.10.0
Code splitting for all output formats ([Feature] Code splitting on async import() statements. #16)
Right now code splitting only works for the
esm
output format, and only works for JavaScript. It should also work for theiife
andcjs
output formats, and also for CSS. I have a plan to address this and it is a part of the linker redesign.Manual chunk labels ([Feature] Manual chunks #207)
This feature isn't implemented yet because it creates import cycles and the content hashing algorithm can't deal with cycles yet. But this is being kept in mind during the linker rewrite so this should be relatively easy to add when the new linker is ready.
Collapse duplicate external imports (Useless repeated imports when using external modules #475)
Right now each individual
import
statement for an external module ends up in the bundle even though they all reference the same external path and the external file is only ever imported once. I plan to collapse this into as few import statements as possible (usually one but may be up to three) as part of the rewrite of the linker.Bundling with top-level await (support for top level await #253)
It's important to have esbuild support top-level await now that it's officially a part of JavaScript and usable in both node and in the browser. Unfortunately the semantics are extremely complicated (and not even implemented correctly in V8 yet!) so esbuild's initial implementation may not be totally accurate. It will at least contain the important parts (
await
works and sibling modules go in parallel). Currently all implementations behave slightly differently so there isn't yet common consensus about exact behavior among implementations. My current plan is to bundle files into groups by what asynchronous files they transitively depend on to reduce the overall number of files, then evaluate files within those groups in the order they would have been evalauted in if there was no top-level await. Cross-group ordering will depend on the order of promise resolutions.Fix import ordering issues with internal code (Incorrect import order with code splitting and multiple entry points #399, The order of CSS imports is not respected #465)
Right now import order is not accurate when code splitting is active. The fundamental issue is that code in chunks is eagerly evaluated when that chunk is imported but different parts of the chunk need to be evaluated at different times for correctness. Either the chunk needs to be split into smaller chunks or code in chunks must be lazily evaluated to fix import order. I'm currently planning to lazily evaluate code to avoid additional chunk splits.
Potentially break the ordering of external imports
External imports cannot be lazily evaluated in
esm
and respecting the correct import order would hurt bundle optimization and make the bundler more complicated. Either chunks would have to be split into further chunks or code would potentially need to be awkwardly stuffed intoimport
statements containing data URLs, both of which would bloat code and hurt bundle download performance. I'm currently considering having external import order still be incorrect (i.e. hoisted to the top) since that's much simpler and seems to be what some other bundlers do anyway.Potentially roll back the file splitting optimization
Currently esbuild contains an optimization that no other bundler supports: individual unrelated statements within a file can end up in separate chunks when code splitting is active if they are used by a disjoint set of entry points. This means you can have a big library file of functions and potentially not have any shared chunks if no two entry points use any of the same functions. However, the blocking for top-level await works at the file boundary and this optimization made thinking through top-level await evaluation order too complicated. I'm seriously considering removing this optimization to restore my sanity and get top-level await out the door. I don't feel too bad about this because no other bundler does this yet anyway.