evanw / esbuild

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

feat: option to import helper methods instead of inlining them in transform API #1230

Open privatenumber opened 3 years ago

privatenumber commented 3 years ago

Problem

Currently, when applying syntax transformations via transform API, helper methods are inlined to the top of the file. Because they are inlined, they can be duplicated in a system where the transform API is applied per module.

Example
Input: ```js async function foo() {} ``` The following command outputs: > esbuild --target=es2015 foo.js ```js // `__async` helper function added to the top of the file var __async = (__this, __arguments, generator) => { return new Promise((resolve, reject) => { var fulfilled = (value) => { try { step(generator.next(value)); } catch (e) { reject(e); } }; var rejected = (value) => { try { step(generator.throw(value)); } catch (e) { reject(e); } }; var step = (x) => x.done ? resolve(x.value) : Promise.resolve(x.value).then(fulfilled, rejected); step((generator = generator.apply(__this, __arguments)).next()); }); }; function foo() { return __async(this, null, function* () { }); } ```

This happens in esbuild-loader so I have been recommending using the minifier to apply syntax transformations as a tip.

This was fine until I realized applying syntax transformations at the optimization stage could be dangerous as the input is the distribution asset rather than a module. As a result, the helpers can be inadvertently be inserted at the top of a JS chunk file and pollute the global scope and collide with other helpers in different chunks.

(As suggested in the thread, a possible solution here is to pass in format: 'iife', but I think it's preferable to do transformations at the loader stage so it doesn't interfere with the format Webpack chunks are expected to be in.)

I believe the helper duplication is happening in other tools that are powered by esbuild as well, such as rollup-plugin-esbuild and esbuild-register (Node.js).

Feature request

An import-helpers option/flag that imports the helpers instead of inlining them. With this option enabled, the example above would output something like this:

import __async from 'esbuild/helpers/async';

function foo() {
  return __async(this, null, function* () {
  });
}

This way, helper methods will only be declared once in a system.

joshwilsonvu commented 3 years ago

I think this is a good solution for esbuild-loader, as evidenced by similar options for Babel and Typescript, but is it even possible for Webpack to resolve a new import in the minifier? From my understanding, Webpack’s asset optimization stage has no support or need for import resolution, since the file format at that stage is essentially a native script to be optimized. Because of that, adding a new import for the helpers in that stage would probably cause a syntax error in the browser. So I think we have to rely on ESBuild to control the placement of the helpers.

privatenumber commented 3 years ago

The transformations would be done at the loader stage instead of the optimization stage. You may have missed this part:

I think it's preferable to do transformations at the loader stage so it doesn't interfere with the format Webpack chunks are expected to be in.

evanw commented 3 years ago

The run-time code is currently in a single JavaScript file, which exists as a string (or more accurately a function that returns a string based on compiler options) within the compiler itself: runtime.go. This feature isn't super straightforward because the run-time library currently adapts itself to different esbuild settings, so there isn't necessarily a single file to point to. The current branch points are ES6/no-ES6, browser/node, and minified/non-minified, and there may be more decision points in the future. Creating a set of files for of all possible combinations doesn't feel like a good direction to go in. Maybe the way to go could be to have a single less-optimized file containing all options and switch between them at run-time instead. Or something like that at least.

This is a duplicate of #284. However this issue has more detail, so I will close that one in favor of this one instead.

bennypowers commented 2 years ago

What's to stop us from just using tslib for this (at least, for done cases like decorators)?

mariuslundgard commented 2 years ago

Not deduplicating helpers is causing our icon library (@sanity/icons) build to bloat when using esbuild. Each icon component is in a separate file, so the helpers are duplicated for each icon – of which there are hundreds.

So can't wait for this! ✨

JWo1F commented 1 year ago

For those who are also waiting for this feature, I sketched a small plugin for Rollup, which cuts out the helper for asynchronous functions and replaces it with import from a virtual module. I don't want to publish it to the npm because the template is stored as a string and can be changed at any time. However, if it will be useful to someone - take it and use it.

I'm really looking forward to moving the helper into separate imports.

import MagicString from 'magic-string';
import escape from 'escape-string-regexp';

const tmpl = `(__this, __arguments, generator) => {
  return new Promise((resolve, reject) => {
    var fulfilled = (value) => {
      try {
        step(generator.next(value));
      } catch (e) {
        reject(e);
      }
    };
    var rejected = (value) => {
      try {
        step(generator.throw(value));
      } catch (e) {
        reject(e);
      }
    };
    var step = (x) => x.done ? resolve(x.value) : Promise.resolve(x.value).then(fulfilled, rejected);
    step((generator = generator.apply(__this, __arguments)).next());
  });
}`;

const regex = new RegExp(`var (.+?) = ${escape(tmpl)};`);

export default function StripAsyncPlugin() {
  return {
    name: 'strip-async-plugin',
    resolveId(source) {
      if (source === 'virtual:async') {
        return `\0${source}`;
      }
      return null;
    },
    load(id) {
      if (id === '\0virtual:async') {
        return `export default ${tmpl};`;
      }
      return null;
    },
    transform(code) {
      if (!code.includes(tmpl)) return;

      const builder = new MagicString(code);
      builder.replace(regex, (code, name) => `import ${name} from "virtual:async";`);

      return {
        code: builder.toString(),
        map: builder.generateMap(),
      };
    },
  };
}
rjharmon commented 1 year ago

+1 for a simple option to use tslib's async helper.

It would be great if esbuild can cue into the tsconfig importHelpers setting.

ScottAwesome commented 1 year ago

@evanw any thought given to this currently?

Its the only downside to using esbuild for libraries in particular. Its less of a problem for apps, but I wouldn't expect people to compile down their node_modules to avoid the inefficiencies here, not to mention as tools like tsup & vite are gaining popularity this is unfortunately just bloating everyone's bundles over and over again.

This ends up leeading to workarounds I don't feel great about (currently, I don't let esbuild transpile my code, instead I use an SWC plugin to transpile code, but that feels icky, though its still reasonably fast, it does lead to a lack of congruence)

Another potential alternative would be for esbuild to recognize the helpers and de-dupe, but that seems like more work and to be a service to the community, I imagine you'd need some mode you could run over node_modules in the bundling process to do this effectively as well for other tools to take advantage of