evanw / esbuild

An extremely fast bundler for the web
https://esbuild.github.io/
MIT License
37.95k stars 1.14k forks source link

Fill out metafile export names in iife mode, or provide way to get exported names in non-ESM mode #3607

Open jakebailey opened 7 months ago

jakebailey commented 7 months ago

Right now, TypeScript's bundle uses --format=iife --global-name=ts with a footer, resulting in:

var ts = (() => {
  // ...
  return ts_exports2;
})();
if (typeof module !== undefined && module.exports) {
  module.exports = ts;
}

This helped us remain compatible with our old bundles pre-5.0, which worked similarly to the above but with the old namespace object.

In https://github.com/microsoft/TypeScript/pull/57133, I'm trying to use the same 0 && (module.exports = { foo, bar, baz }); trick that esbuild does to "annotate" the exports for Node, as our bundle is silently broken for anyone trying an ESM namespace import. If were were using --format=cjs --platform-node (along with export * instead of export =), then esbuild would handle this for us, but in order to keep our format, we can't do that.

--metafile does not fill in the export info when using --iife (or even cjs; #3110, #3281). This means that in order to do the same annotation trick, I need some other way to get the names. At the moment, I'm making our build import the bundle and read the list of names, which I'd prefer not to do.

Is there any way that --metafile could provide a list of exported names in --format=iife mode, as it does in --format=esm? I understand that it could be argued that the only "export" is the global name (if provided), but I'm looking for any way for me to statically reproduce the export annotation trick. It doesn't need to be the exports prop on metafile at all; I know this is a bit of an "XY problem" scenario.

The closest thing I've gotten is to run esbuild a second time but emit ESM, then use only the metafile and throw away the output, but that turns out to require code changes in TypeScript to eliminate export = ts and seems to be a bit slower than just importing the (almost) final bundle.

It could be the case that this is just a pure dupe of #3281 / #3110, of course (in which case feel free to close). Or, you'll come up with some other idea 😅

evanw commented 7 months ago

First of all, I think this makes sense to do (which is why #3281 is open).

But I had a quick thought after reading your post: Have you already considered using --platform=node along with a wrapper that looks something like this?

var ts = {}; ((module) => {
  // ... esbuild's output goes here ...
})(typeof module !== "undefined" && module.exports ? module : { exports: ts });

You can try this out with the following esbuild options:

banner: {
  js: 'var ts = {}; ((module) => {',
},
footer: {
  js: '})(typeof module !== "undefined" && module.exports ? module : { exports: ts });',
},

It seems like that would be roughly equivalent. I'm mentioning this because I'm going to be traveling really soon and might not have a ton of time for this in the next week or so.

jakebailey commented 7 months ago

I just tested it out, and it does work, so long as I switch us from:

import * as ts from "./_namespaces/ts"
export = ts

To export * from "./_namespaces/ts".

But I'm afraid that bit in particular breaks the bundle because we get __esModule when we didn't before, which changes the import helpers / tslib's behavior to no longer match the export shape we used to have. This was something I didn't notice early on when I switched us to modules and esbuild, but someone noticed while testing the nightlies pre-5.0.

In https://github.com/microsoft/TypeScript/pull/57163, I actually tried out switching to export *, but had to do more wonky hacking on the bundle to make __toCommonJS a noop. (Probably, I could post-process the export object to drop it, and the new snippet you gave is a little more condusive to that.)

Of course, I realized after filing this issue that I was looking for exports but also trying to avoid export *, which meant that I was effectively asking for something very odd; after all, the exported thing is just an object at that point, not named members. Certainly a little bit of the XY problem...

jakebailey commented 7 months ago

In shorter words, maybe I don't even need what I originally described at all, and should just use your suggestion and strip __esModule (or, see if there's a way to not emit it in the first place and avoid the object copy).

jakebailey commented 7 months ago

I updated https://github.com/microsoft/TypeScript/pull/57133 with your suggestion, leaving the only hack as the __esModule thing. So, I guess I could probably rescind my request and be okay.

It'd be helpful if the __esModule thing were configurable in some way, but I can totally understand not wanting to do that.