Only transform files that need to be transformed

[dylang]

This fixes a ton of weird bugs we are having, such as __name not defined in our webpack bundle, and made our code run much faster, while still working with ESM modules.

[dylang]

We could bypass this check (and always transformSync) when the extension is not .js.

[privatenumber]

I think this change avoids the bugs, rather than fixing them, by preventing tsx from transforming certain files.

__name not defined is likely related to privatenumber/tsx#113 but I don't see the relevance of Webpack. Can you elaborate on your issue there?

I would like to improve performance but I'm not sure if this is a good signal. What if the file has types or next generation syntax but no import/export?

Also, please test your code before marking a PR as ready.

[dylang]

relevance of Webpack

This is my guess:

1. We overwrite the node module loader.
2. Webpack loads modules to put in bundles.
3. Webpack uses Function.toString() and eval to reduce functions into strings and save as bundles.
4. ESBuild doesn't support Function.toString() or eval for multiple reasons, such as variable names can be changed or scope lost.
5. The __name bug specifically occurs the generated hot reloading code, not in our code that webpack is loading. So it could be how webpack loads the template used for the hotreloading code.

Before this change, webpack-dev takes a few minutes to load the bundles. After this change about 30 seconds.

What if the file has types

I agree, we should look at the file extension and always transform non-".js" files.

next generation syntax but no import/export?

I'm never going to find the reference, but I remember reading part of the compromise for ESM to be permitted in files that have the .js extension is that the file must have an import or export statement.

Also, please test your code before marking a PR as ready.

I agree, I'm a big fan of tests. I didn't realize the project had tests because they're not in the same directory as the code.

marking a PR as ready

I don't think I did this on purpose. The PR was intended to start a conversation with something slightly more useful than just a ticket with no code.

[dylang]

Here's the generated webpack hot reloading code that fails because `__name` is not defined. I formatted it with prettier so it's readable.

var currentModuleData = {};
var installedModules = __webpack_require__.c;
var currentChildModule;
var currentParents = [];
var registeredStatusHandlers = [];
var currentStatus = "idle";
var blockingPromises = 0;
var blockingPromisesWaiting = [];
var currentUpdateApplyHandlers;
var queuedInvalidatedModules;
__webpack_require__.hmrD = currentModuleData;
__webpack_require__.i.push(function (options) {
  var module2 = options.module;
  var require2 = createRequire(options.require, options.id);
  module2.hot = createModuleHotObject(options.id, module2);
  module2.parents = currentParents;
  module2.children = [];
  currentParents = [];
  options.require = require2;
});
__webpack_require__.hmrC = {};
__webpack_require__.hmrI = {};
function createRequire(require2, moduleId) {
  var me = installedModules[moduleId];
  if (!me) return require2;
  var fn = __name(function (request) {
    if (me.hot.active) {
      if (installedModules[request]) {
        var parents = installedModules[request].parents;
        if (parents.indexOf(moduleId) === -1) {
          parents.push(moduleId);
        }
      } else {
        currentParents = [moduleId];
        currentChildModule = request;
      }
      if (me.children.indexOf(request) === -1) {
        me.children.push(request);
      }
    } else {
      console.warn(
        "[HMR] unexpected require(" +
          request +
          ") from disposed module " +
          moduleId
      );
      currentParents = [];
    }
    return require2(request);
  }, "fn");
  var createPropertyDescriptor = __name(function (name2) {
    return {
      configurable: true,
      enumerable: true,
      get: function () {
        return require2[name2];
      },
      set: function (value) {
        require2[name2] = value;
      },
    };
  }, "createPropertyDescriptor");
  for (var name in require2) {
    if (Object.prototype.hasOwnProperty.call(require2, name) && name !== "e") {
      Object.defineProperty(fn, name, createPropertyDescriptor(name));
    }
  }
  fn.e = function (chunkId) {
    return trackBlockingPromise(require2.e(chunkId));
  };
  return fn;
}
__name(createRequire, "createRequire");
function createModuleHotObject(moduleId, me) {
  var _main = currentChildModule !== moduleId;
  var hot = {
    _acceptedDependencies: {},
    _acceptedErrorHandlers: {},
    _declinedDependencies: {},
    _selfAccepted: false,
    _selfDeclined: false,
    _selfInvalidated: false,
    _disposeHandlers: [],
    _main,
    _requireSelf: function () {
      currentParents = me.parents.slice();
      currentChildModule = _main ? void 0 : moduleId;
      __webpack_require__(moduleId);
    },
    active: true,
    accept: function (dep, callback, errorHandler) {
      if (dep === void 0) hot._selfAccepted = true;
      else if (typeof dep === "function") hot._selfAccepted = dep;
      else if (typeof dep === "object" && dep !== null) {
        for (var i = 0; i < dep.length; i++) {
          hot._acceptedDependencies[dep[i]] = callback || function () {};
          hot._acceptedErrorHandlers[dep[i]] = errorHandler;
        }
      } else {
        hot._acceptedDependencies[dep] = callback || function () {};
        hot._acceptedErrorHandlers[dep] = errorHandler;
      }
    },
    decline: function (dep) {
      if (dep === void 0) hot._selfDeclined = true;
      else if (typeof dep === "object" && dep !== null)
        for (var i = 0; i < dep.length; i++)
          hot._declinedDependencies[dep[i]] = true;
      else hot._declinedDependencies[dep] = true;
    },
    dispose: function (callback) {
      hot._disposeHandlers.push(callback);
    },
    addDisposeHandler: function (callback) {
      hot._disposeHandlers.push(callback);
    },
    removeDisposeHandler: function (callback) {
      var idx = hot._disposeHandlers.indexOf(callback);
      if (idx >= 0) hot._disposeHandlers.splice(idx, 1);
    },
    invalidate: function () {
      this._selfInvalidated = true;
      switch (currentStatus) {
        case "idle":
          currentUpdateApplyHandlers = [];
          Object.keys(__webpack_require__.hmrI).forEach(function (key) {
            __webpack_require__.hmrI[key](moduleId, currentUpdateApplyHandlers);
          });
          setStatus("ready");
          break;
        case "ready":
          Object.keys(__webpack_require__.hmrI).forEach(function (key) {
            __webpack_require__.hmrI[key](moduleId, currentUpdateApplyHandlers);
          });
          break;
        case "prepare":
        case "check":
        case "dispose":
        case "apply":
          (queuedInvalidatedModules = queuedInvalidatedModules || []).push(
            moduleId
          );
          break;
        default:
          break;
      }
    },
    check: hotCheck,
    apply: hotApply,
    status: function (l) {
      if (!l) return currentStatus;
      registeredStatusHandlers.push(l);
    },
    addStatusHandler: function (l) {
      registeredStatusHandlers.push(l);
    },
    removeStatusHandler: function (l) {
      var idx = registeredStatusHandlers.indexOf(l);
      if (idx >= 0) registeredStatusHandlers.splice(idx, 1);
    },
    data: currentModuleData[moduleId],
  };
  currentChildModule = void 0;
  return hot;
}
__name(createModuleHotObject, "createModuleHotObject");
function setStatus(newStatus) {
  currentStatus = newStatus;
  var results = [];
  for (var i = 0; i < registeredStatusHandlers.length; i++)
    results[i] = registeredStatusHandlers[i].call(null, newStatus);
  return Promise.all(results);
}
__name(setStatus, "setStatus");
function unblock() {
  if (--blockingPromises === 0) {
    setStatus("ready").then(function () {
      if (blockingPromises === 0) {
        var list = blockingPromisesWaiting;
        blockingPromisesWaiting = [];
        for (var i = 0; i < list.length; i++) {
          list[i]();
        }
      }
    });
  }
}
__name(unblock, "unblock");
function trackBlockingPromise(promise) {
  switch (currentStatus) {
    case "ready":
      setStatus("prepare");
    case "prepare":
      blockingPromises++;
      promise.then(unblock, unblock);
      return promise;
    default:
      return promise;
  }
}
__name(trackBlockingPromise, "trackBlockingPromise");
function waitForBlockingPromises(fn) {
  if (blockingPromises === 0) return fn();
  return new Promise(function (resolve) {
    blockingPromisesWaiting.push(function () {
      resolve(fn());
    });
  });
}
__name(waitForBlockingPromises, "waitForBlockingPromises");
function hotCheck(applyOnUpdate) {
  if (currentStatus !== "idle") {
    throw new Error("check() is only allowed in idle status");
  }
  return setStatus("check")
    .then(__webpack_require__.hmrM)
    .then(function (update) {
      if (!update) {
        return setStatus(applyInvalidatedModules() ? "ready" : "idle").then(
          function () {
            return null;
          }
        );
      }
      return setStatus("prepare").then(function () {
        var updatedModules = [];
        currentUpdateApplyHandlers = [];
        return Promise.all(
          Object.keys(__webpack_require__.hmrC).reduce(function (
            promises,
            key
          ) {
            __webpack_require__.hmrC[key](
              update.c,
              update.r,
              update.m,
              promises,
              currentUpdateApplyHandlers,
              updatedModules
            );
            return promises;
          },
          [])
        ).then(function () {
          return waitForBlockingPromises(function () {
            if (applyOnUpdate) {
              return internalApply(applyOnUpdate);
            } else {
              return setStatus("ready").then(function () {
                return updatedModules;
              });
            }
          });
        });
      });
    });
}
__name(hotCheck, "hotCheck");
function hotApply(options) {
  if (currentStatus !== "ready") {
    return Promise.resolve().then(function () {
      throw new Error(
        "apply() is only allowed in ready status (state: " + currentStatus + ")"
      );
    });
  }
  return internalApply(options);
}
__name(hotApply, "hotApply");
function internalApply(options) {
  options = options || {};
  applyInvalidatedModules();
  var results = currentUpdateApplyHandlers.map(function (handler) {
    return handler(options);
  });
  currentUpdateApplyHandlers = void 0;
  var errors = results
    .map(function (r) {
      return r.error;
    })
    .filter(Boolean);
  if (errors.length > 0) {
    return setStatus("abort").then(function () {
      throw errors[0];
    });
  }
  var disposePromise = setStatus("dispose");
  results.forEach(function (result) {
    if (result.dispose) result.dispose();
  });
  var applyPromise = setStatus("apply");
  var error;
  var reportError = __name(function (err) {
    if (!error) error = err;
  }, "reportError");
  var outdatedModules = [];
  results.forEach(function (result) {
    if (result.apply) {
      var modules = result.apply(reportError);
      if (modules) {
        for (var i = 0; i < modules.length; i++) {
          outdatedModules.push(modules[i]);
        }
      }
    }
  });
  return Promise.all([disposePromise, applyPromise]).then(function () {
    if (error) {
      return setStatus("fail").then(function () {
        throw error;
      });
    }
    if (queuedInvalidatedModules) {
      return internalApply(options).then(function (list) {
        outdatedModules.forEach(function (moduleId) {
          if (list.indexOf(moduleId) < 0) list.push(moduleId);
        });
        return list;
      });
    }
    return setStatus("idle").then(function () {
      return outdatedModules;
    });
  });
}
__name(internalApply, "internalApply");
function applyInvalidatedModules() {
  if (queuedInvalidatedModules) {
    if (!currentUpdateApplyHandlers) currentUpdateApplyHandlers = [];
    Object.keys(__webpack_require__.hmrI).forEach(function (key) {
      queuedInvalidatedModules.forEach(function (moduleId) {
        __webpack_require__.hmrI[key](moduleId, currentUpdateApplyHandlers);
      });
    });
    queuedInvalidatedModules = void 0;
    return true;
  }
}
__name(applyInvalidatedModules, "applyInvalidatedModules");

[dylang]

Here is a repro of the webpack issue:

https://stackblitz.com/edit/nextjs-3iyyq9?file=pages%2Findex.js

To see it, hit control-c, and re-run with npx tsx ./node_modules/.bin/next dev. Then look at the console:

[dylang]

I've updated the PR so all tests pass. Nice testing framework!

I don't know webpack well enough to know how to implement a test for this.

[privatenumber]

RE: Webpack issue

• Curious why you need to run next with tsx?
• I'm not familiar with using Next, but you can configure Webpack to not use eval via devtool

RE: Skipping files
The skipped files will not receive any of these transformations.

[dylang]

Curious why you need to run next with tsx?

We don't use next, it was a quick and easy way to get a reproduction case for you because we use the same hot module reloading library. I didn't want to have to set it all up.

configure Webpack to not use eval via devtool

That's just for the source map, not hot module reloading. We don't use the eval setting:

devtool: isProduction ? 'source-map' : isCI ? undefined : 'cheap-module-source-map',

The skipped files will not receive any of these transformations.

Correct, that is expected, as published modules that don't target ESM are already transpiled or built to target the current version of Node.

[privatenumber]

Why do you run Webpack with tsx?

Correct, that is expected, as published modules that don't target ESM are already transpiled or built to target the current version of Node.

1. Not true at all. Most Node packages aren't built or transpiled. This was one of the motivations behind tsx.
2. Your filter doesn't target published modules.

[dylang]

Most Node packages aren't built or transpiled.

Maybe we have a difference in terminology here? Can you share some public modules that are not ESM but need to be transpiled to use in node? I'm talking about code in our node_modules directory. I haven't seen public packages that needed to be transpiled to be used in Node, other than ESM, which is the whole reason we are using tsx. Thanks for helping me understand this better!

Your filter doesn't target published modules.

It does, that's why it's fixing the problems for us. "Published modules" in my mind are the modules we're installing into node_modules - is that what you mean?

Maybe the filter should only target node_modules, as our local code is highly likely to need to be transpiled. Ours is in TypeScript, so it will aways be transpiled, even with the changes in this PR.

I also found the code in Webpack that ESBuild is breaking:

• HMR uses require to load the hotreload template.
• Template.getFunctionContent does a .toString() to that function.

Even if evanw/esbuild#2605 is addressed, there's no guarantee that other tranformations or helper functions like __name couldn't be added in the future that will break otherwise working code because of the .toString().

[privatenumber]

Can you share some public modules that are not ESM but need to be transpiled to use in node?

I don't have a specific example at the moment, but most pre-ESM sindresorhus packages are uncompiled and use new ES features. The packages require a minimum Node version, but with tsx, the idea is you can run them in lower versions. One non-syntactical example (which luckily doesn't require a transform to support) is the node: prefix in require specifiers.

I'm talking about code in our node_modules directory.

"Published modules" would typically refer to packages installed from the npm registry, but in that case, uncompiled user-code is very common. Any use-case that symlinks (e.g. 'npm link' or monorepos). Check the tsx repo for Issues where devs import unbuilt TS projects (which can have allowJs enabled).

Maybe the filter should only target node_modules

I think this is what "target node_modules" implies, and is also what I was asking about. Currently, it also skips transformations on local modules which is likely uncompiled.

Even if evanw/esbuild#2605 is addressed, there's no guarantee that other tranformations or helper functions like __name couldn't be added in the future that will break otherwise working code because of the .toString().

Why do you need to use tsx to run Webpack?

---

To move forward with this, I'll need confirmation that esbuild doesn't have JS transformations for Node.js v12.20 and above. IIRC theres a file in esbuilds repo with transformations and the Node.js version ranges.

[dylang]

I don't have a specific example at the moment, but most pre-ESM sindresorhus packages are uncompiled and use new ES features.

His ESM packages are the reason we're using tsx. 😄 We had no trouble using his packages before the ESM migrations. The "new ES features" were always in line with the current LTS version of Node, and he always uses the engines prop in package.json so you get warnings if you try to use a module that's not compatible with your version of node.

Any use-case that symlinks (e.g. 'npm link' or monorepos).

Yes, I agree. We use a monorepo and never "build" anything except the webpack bundle or when publishing a node module for others to use.

I've updated the PR so we always transform code that is not in /node_modules/.

Currently, it also skips transformations on local modules which is likely uncompiled.

Sorry, I'm not sure what you mean by "currently". Before and after the changes from this PR local code is transformed. Otherwise we couldn't use it to run our projects which are in TypeScript.

Why do you need to use tsx to run Webpack?

When developers run yarn start it:

• Finds available ports for the backend and frontend.
• Spawns the backend graphql server with mock data.
• Starts up webpack-dev-server. This is where webpack is loaded and the problem occurs.
• Once ready, opens or refreshes a browser tab with the web content.
• If the backend is changed, it re-launches the backend, and then reloads the web page.

All of that is done in a project written in TypeScript, so we run it with tsx.

[dylang]

An engineer on my team just discovered tests failing because of esbuild transpiling jsdom.

I dug into this a little more, since workarounds without fully knowing the root cause always makes me a bit nervous. I haven't found the ultimate root cause, but it does look like this error might be related to esbuild mangling names. I've tracked it down to this code in JSDOM - or mainly, that's the code that should be getting called when constructing the event path for the top-level window object. The problem is that this call isn't holding when parent is window:

idlUtils.wrapperForImpl(parent).constructor.name === "Window"

Instead the constructor name is "windowConstructor", which causes this code to be executed instead, and that incorrectly sets the event target to the Window object instead of the actual DOM node we expect. But only when the event is dispatched for listeners directly on the window object.

This PR avoids this issue by not transpiling jsdom.

[privatenumber]

Please file a separate issue separately with reproduction. We should seek to address the problem than to avoid them without knowing why.

In case you missed it:

To move forward with this, I'll need confirmation that esbuild doesn't have JS transformations for Node.js v12.20 and above. IIRC theres a file in esbuilds repo with transformations and the Node.js version ranges.

[dylang]

Please file a separate issue separately with reproduction. We should seek to address the problem than to avoid them without knowing why.

Both of these are known caveats with esbuild mentioned on https://esbuild.github.io/api/

• The jsdom issues is because of the name being dropped (we're on a build before __name was always included because of the webpack issue).
• The webpack hmr issue is because Function.toString() it used to generate the code, there's no way ESBuild or Webpack can know the __name function needs to be included in that toString() output.

To move forward with this, I'll need confirmation that esbuild doesn't have JS transformations for Node.js v12.20 and above. IIRC theres a file in esbuilds repo with transformations and the Node.js version ranges.

This would be for those using Node < 12.20? Node 12 was EOL more than 6 months ago, do we still need this to support versions 12 and under?

[dylang]

Hi @privatenumber -

I decided that our goals aren't the same and it would be best to build our own ESM loader using babel-core so you don't need to make changes you don't want and we're not blocked by bugs introduced by nuances in ESBuild.

I truly appreciate your hard work on this project. You helped us finally be able to use third-party ESM modules. I hope we can use your work again in the future.

[RichieChoo]

__name not define when use @esbuild-kit/cjs-loader !!!