esbuild minify mismanages some border cases

[vladmandic]

using esbuild to create a minified esm bundle of a project that imports @tensorflow/tfjs-backend-wasm creates a bundle that loads without issues, but fails to initialize (but really messy to pinpoint as error is a generic object undefined).

most likely because renamed variables don't match to ones that module is trying to reference when loading a js file in a worker thread.

creating same esm bundle with only minifyWhitespace works perfectly.

stack:

thread sent an error! blob:https://wyse:8000/3909a0ab-8a59-489d-bac5-27f060d00f0f:1: Uncaught ReferenceError: m is not defined
  C.onerror	@	tfjs-backend-wasm-threaded-simd.js:9
  error (async)		
  loadWasmModuleToWorker	@	tfjs-backend-wasm-threaded-simd.js:9
  (anonymous)	@	tfjs-backend-wasm-threaded-simd.js:9
  P	@	tfjs-backend-wasm-threaded-simd.js:9
  q	@	tfjs-backend-wasm-threaded-simd.js:9
  Promise.then (async)		
  (anonymous)	@	tfjs-backend-wasm-threaded-simd.js:9
  Promise.then (async)		
  Fe	@	tfjs-backend-wasm-threaded-simd.js:9
  Ac	@	tfjs-backend-wasm-threaded-simd.js:9
  (anonymous)	@	tfjs-backend-wasm-threaded-simd.js:9
  (anonymous)	@	backend_wasm.ts:279
  cX	@	backend_wasm.ts:244

[evanw]

I'm very interested in a repro case if you can provide one. I'm happy to pinpoint the error myself. Edit: I'm asking because I tried to re-create this myself and couldn't. The threaded SIMD backend seemed to work for me, although I'm very unfamiliar with TensorFlow.

[vladmandic]

quick test for minify issues - works without minify or minify-whitespace, but fails on full minify:

https://gist.github.com/vladmandic/3097052d32a902239e9b855af8fd8425

[evanw]

Ah ok. This code isn't safe for a minifier so it makes sense why minification breaks it. This isn't a problem with esbuild, it's a problem with the library. This library doesn't work with a Webpack production build either.

This library basically does this:

var WasmBackendModuleThreadedSimd = function () {
  var _scriptDir = typeof document !== "undefined" && document.currentScript ? document.currentScript.src : void 0;
  if (typeof __filename !== "undefined")
    _scriptDir = _scriptDir || __filename;
  return function (WasmBackendModuleThreadedSimd2) {
    var scriptDirectory = "";
    if (_scriptDir) {
      scriptDirectory = _scriptDir;
    }
  };
}();
...
wasm.mainScriptUrlOrBlob = new Blob([
  `var _scriptDir = undefined;
  var WasmBackendModuleThreadedSimd = ` + WasmBackendModuleThreadedSimd.toString()
], { type: "text/javascript" });

The minifier renames _scriptDir because it's a local variable, which then causes the code in mainScriptUrlOrBlob to break. Minifiers assume that you aren't going to call toString() on a function and then eval it somewhere else to reconstitute it.

One hack that the library author could do to "fix" this in both esbuild and Webpack would be to add a direct eval to tell the minifier that everything in that scope and all parent scopes must not be renamed:

 var WasmBackendModuleThreadedSimd = function () {
   var _scriptDir = typeof document !== "undefined" && document.currentScript ? document.currentScript.src : void 0;
   if (typeof __filename !== "undefined")
     _scriptDir = _scriptDir || __filename;
+
+  // Make sure JavaScript minifiers don't rename "_scriptDir", since code 
+  // elsewhere will convert the function below to a string and then back
+  // into a function with dynamic compilation and still expect it to be
+  // able to access a global variable called "_scriptDir".
+  eval('_scriptDir');
+
   return function (WasmBackendModuleThreadedSimd2) {
     var scriptDirectory = "";
     if (_scriptDir) {
       scriptDirectory = _scriptDir;
     }
   };
 }();

But that's a hack, and may not be completely robust. It will also trigger warnings with some bundlers because using direct eval is an anti-pattern in bundled code. A better fix would be to just remove this magic variable and make it an parameter of that function (or somehow pass it to that code normally) since it's clearly just a parameter for that code.

Anyway I'm going to close this issue because it's not a problem with esbuild. You should be able to continue to use this library with esbuild if you just avoid enabling --minify-identifiers. You can still enable --minify-whitespace and --minify-syntax and this particular problem shouldn't come up.

[vladmandic]

full agree, but wish (feature request) is to mark a module to skip optimizations on? this is a tiny part of the entire library, so it would make total sense to minify the rest and just skip this one.