-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
esbuild minify mismanages some border cases #538
Comments
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. |
quick test for minify issues - works without minify or minify-whitespace, but fails on full minify: https://gist.github.com/vladmandic/3097052d32a902239e9b855af8fd8425 |
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 One hack that the library author could do to "fix" this in both esbuild and Webpack would be to add a direct 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 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 |
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. |
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:
The text was updated successfully, but these errors were encountered: