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

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

Open
privatenumber opened this issue May 1, 2021 · 8 comments

Comments

@privatenumber
Copy link
Contributor

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:

async function foo() {}

The following command outputs:

esbuild --target=es2015 foo.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
Copy link

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
Copy link
Contributor Author

privatenumber commented May 1, 2021

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
Copy link
Owner

evanw commented May 6, 2021

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
Copy link

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

@mariuslundgard
Copy link

mariuslundgard commented Sep 9, 2022

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
Copy link

JWo1F commented Dec 19, 2022

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
Copy link

+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
Copy link

ScottAwesome commented Aug 29, 2023

@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

suchipi added a commit to suchipi/kame that referenced this issue Sep 1, 2023
esbuild doesn't have a good way to dedupe helpers when using the transform API, so I'm not gonna do this rn.

See evanw/esbuild#1230
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

No branches or pull requests

8 participants