Skip to content
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

Merged
merged 27 commits into from
Mar 9, 2021
Merged

WIP for version 0.9.0 #924

merged 27 commits into from
Mar 9, 2021

Conversation

evanw
Copy link
Owner

@evanw evanw commented Mar 6, 2021

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 flag

    This 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 and SpinnerIdle from the Go API

    These 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 setting logLevel: '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 only build() and transform()

    The context is that the startService() API was added quickly in response to a user request and at the time I was unaware of unref() which makes it possible to use a child process without requiring people to call service.stop(). Many thanks to @SalvatorePreviti who contributed this change in Allows NodeJS to terminate also if there is an active service. #656. Calling service.stop() no longer does anything, so there is no longer a strong reason for keeping the startService() API around. The primary thing it currently does is just make the API more complicated and harder to use. I plan to add an initialize({ wasmURL }) API to replace the configuration of wasmURL in the browser that used to be done by startService(). And the buildSync() and transformSync() 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 extensions

    I 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 have some-file.mjs and some-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) format

    I 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 using require(...) with ESM were to be supported then you may be seemingly arbitrarily prevented from adding an await to a file because some other file in a seemingly unrelated place happened to use a require(...) 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 in package.json (Support exports 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 from outputFiles (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 using write: 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 the metafile. 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 the metafile 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 dynamic import(...) 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 the iife and cjs 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 into import 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.

@SalvatorePreviti
Copy link
Contributor

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.

@evanw evanw force-pushed the 0.9.0-wip branch 2 times, most recently from 8567643 to ee678c6 Compare March 7, 2021 00:49
@overlookmotel
Copy link

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.

@evanw
Copy link
Owner Author

evanw commented Mar 7, 2021

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.

@overlookmotel
Copy link

Fair enough. So the decision is made, and it will be removed in 0.9.0?

@evanw
Copy link
Owner Author

evanw commented Mar 7, 2021

That change is scheduled for 0.10.0, not 0.9.0.

// 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)
Copy link
Contributor

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

Copy link
Owner Author

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.

Copy link
Contributor

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

Copy link
Contributor

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.

Copy link
Owner Author

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.

@hardfist
Copy link
Contributor

hardfist commented Mar 8, 2021

Remove the ability to use require(...) on a file in ESM (ECMAScript module) format do you mean remove hybrid esm/cjs support ? it's one of the key features we rely on (one of the reason that we migrate from rollup to esbuild ), could you make it an options to disable/enable ?
it can't be replaced with import, because import will hoist module to top-level, which will effect some global variable assignment order.

@evanw
Copy link
Owner Author

evanw commented Mar 8, 2021

Remove the ability to use require(...) on a file in ESM (ECMAScript module) format do you mean remove hybrid esm/cjs support ? it's one of the key features we rely on (one of the reason that we migrate from rollup to esbuild ), could you make it an options to disable/enable ?
it can't be replaced with import, because import will hoist module to top-level, which will effect some global variable assignment order.

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?

@hardfist
Copy link
Contributor

hardfist commented Mar 8, 2021

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.

@evanw
Copy link
Owner Author

evanw commented Mar 8, 2021

what I really want is inline require support in rollup/plugins#481 so as to insert some module to existing cjs|esm module.

Inline require in an ESM module would continue to be supported with the proposed behavior, provided that the target of the require is a CommonJS module. This is well-defined. So doing something like this would continue to work:

export default require('./file.cjs');

As a demonstration that this is well-defined, you can even call require from ESM on a CommonJS module in node natively without needing a bundler, although you do need to pull in the require function manually while esbuild would continue to provide it automatically:

import { createRequire } from 'module';
const require = createRequire(import.meta.url);
export default require('./file.cjs');

@evanw evanw force-pushed the 0.9.0-wip branch 2 times, most recently from ac70415 to 4730387 Compare March 9, 2021 03:46
@evanw
Copy link
Owner Author

evanw commented Mar 9, 2021

Ok I will hold back on the "remove the ability to use require on a file in ESM (ECMAScript module) format" change for now. I just realized that esbuild's inject feature is actually another way to insert an ESM import statement into a CommonJS module so solving this problem will require more thought after all. The point of the 0.9.0 release is to get the pending breaking changes out without waiting for the more difficult ones to be finished, so this release shouldn't wait for that change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants