evanw / esbuild

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

`inject` option should only "inject once" for each entrypoint #1557

Open osddeitf opened 3 years ago

osddeitf commented 3 years ago

Hello, and thanks for this awesome project.

I'm trying to create my own build tool for React. I want the compile time as small as possible, so i use React CDN in my index.html, with my entrypoint index.tsx compiled to index.js.

When i use jsx with the option minifyIdentifiers: true, React.createElement is leave as is. That's correct since "react" package is excluded from the bundle by external: ["react"], and no React identifiers are imported.

Bundle size matters, so that's why i need to use inject option with the following shim.ts, with jsxFactory option changed to _jsx, and the minifyIdentifiers option works as intended:

const _jsx = React.createElement
export { _jsx }

But when I tried to turn off minifyIdentifiers to inspect what's generated to figure it out, it looked like this:

// src/shim.ts
var _jsx;
var init_shim = __esm({
  "src/shim.ts"() {
    _jsx = React.createElement;
  }
});

// template/index.tsx
init_shim();

// template/app.tsx
init_shim();

// dist/navigation/Navigation1.js
init_shim();

I ran with only one entrypoint, but there're multiple init_shim(). Now that I actually understand how inject option works. But isn't it a huge performance penalty if my shim.ts do some little heavy computation, and I need to compile a lot of files into a single entrypoint? And what if there's some side effect inside shim.ts (bad practice anyway)? That's the problem.

Proposal

We should only need to have one injection like init_shim() above for each entrypoint. That'll makes sense, because the purpose of inject is to "inject global variables" only, and it'll be called first in any entrypoint. It'll only be a breaking change for those who are relying on side effects of each "injection".

Environment & configuration

osddeitf commented 3 years ago

Actually, my codes have a pollyfill for import { useState } from 'react'.

tsconfig.json

{
  "compilerOptions": {
    "baseUrl": ".",
    "paths": {
      "react": ["react-pollyfill.ts"]
    }
  }
}

react-pollyfill.ts

declare var React: any
module.exports = React

When i tried to modify react-pollyfill.ts like this:

declare var React: {
  useState: (x: any) => any,
  useEffect: (x: any) => any,
}

export var useState = React.useState
export var useEffect = React.useEffect

The result is different:

(function() {
  // src/shim.production.ts
  var _jsx = React.createElement;

  // template/react-pollyfill.ts
  var useState = React.useState, useEffect = React.useEffect;

not having a lot of codes:

(function() {
  var __create = Object.create;
  var __defProp = Object.defineProperty;
  var __getOwnPropDesc = Object.getOwnPropertyDescriptor;
  var __getOwnPropNames = Object.getOwnPropertyNames;
  var __getProtoOf = Object.getPrototypeOf, __hasOwnProp = Object.prototype.hasOwnProperty;
  var __markAsModule = function(target) {
    return __defProp(target, "__esModule", { value: !0 });
  };
  var __esm = function(fn, res) {
    return function() {
      return fn && (res = (0, fn[Object.keys(fn)[0]])(fn = 0)), res;
    };
  };
  var __commonJS = function(cb, mod) {
    return function() {
      return mod || (0, cb[Object.keys(cb)[0]])((mod = { exports: {} }).exports, mod), mod.exports;
    };
  };
  var __reExport = function(target, module, desc) {
    if (module && typeof module == "object" || typeof module == "function")
      for (var keys = __getOwnPropNames(module), i = 0, n = keys.length, key; i < n; i++)
        key = keys[i], !__hasOwnProp.call(target, key) && key !== "default" && __defProp(target, key, { get: function(k) {
          return module[k];
        }.bind(null, key), enumerable: !(desc = __getOwnPropDesc(module, key)) || desc.enumerable });
    return target;
  }, __toModule = function(module) {
    return __reExport(__markAsModule(__defProp(module != null ? __create(__getProtoOf(module)) : {}, "default", module && module.__esModule && "default" in module ? { get: function() {
      return module.default;
    }, enumerable: !0 } : { value: module, enumerable: !0 })), module);
  };

  // src/shim.production.ts
  var _jsx, init_shim_production = __esm({
    "src/shim.production.ts": function() {
      _jsx = React.createElement;
    }
  });

  // template/react-pollyfill.ts
  var require_react_pollyfill = __commonJS({
    "template/react-pollyfill.ts": function(exports, module) {
      init_shim_production();
      module.exports = React;
    }
  });

  // template/index.tsx
  init_shim_production();

  // template/app.tsx
  init_shim_production();

  // dist/navigation/Navigation1.js
  init_shim_production();
...

How's that happened? is it because i mixing between esm and commonjs modules?

hyrious commented 3 years ago

is it because i mixing between esm and commonjs modules?

Yes. A simple rule: all commonjs codes have side effects so they should be executed everytime when they are imported.

As in your problem, you don't have to use inject as it is used to polyfill global names instead of import "react". The tsconfig/path is enough. Just make sure all your code is esm.

// tsconfig.json
{ "compilerOptions": { "paths": { "react": ["./react-shim.js"] } } }

// react-shim.js
export default React
export var useState = React.useState

// a.js
import React, { useState } from "react"
import "./b"
console.log(React, useState('a'))

// b.js
import { useState } from "react"
console.log(useState('b'))

// esbuild a.js --bundle
(() => {
  // react-shim.js
  var react_shim_default = React;
  var useState = React.useState;

  // b.js
  console.log(useState("b"));

  // a.js
  console.log(react_shim_default, useState("a"));
})();
osddeitf commented 3 years ago

so, if multiple injection is commonjs expected behavior, i'll close this issue then

osddeitf commented 3 years ago

but, as i know, when multiple files require() the same module, the side effect only runs once. So that's still a problem.

evanw commented 3 years ago

This is similar to an existing issue: #475. The linker would ideally merge duplicate imports. However, this gets complicated with top-level await so doing this is not as straightforward as it would initially seem.

antongolub commented 4 months ago

@evanw,

How about wrapping __esm and __commonJS factories with once helper as a temporary workaround?

// This makes module initializers once-time callable.
var once = (fn) => {
    var result, done = false
    return function() {
        return done ? result : (done = true, result = fn.apply(this, arguments))
    }
}

// This is for lazily-initialized ESM code. This has two implementations, a
// compact one for minified code and a verbose one that generates friendly
// names in V8's profiler and in stack traces.
export var __esm = (fn, res) => once(function __init() {
    return fn && (res = (0, fn[__getOwnPropNames(fn)[0]])(fn = 0)), res
})