microbundle icon indicating copy to clipboard operation
microbundle copied to clipboard

Imports within try blocks are hoisted incorrectly

Open dcastil opened this issue 5 years ago • 3 comments

Problem

Microbundle hoists a require() call out of a try {} block in a dependency. When that require() call throws, it is not caught by the try {} block anymore.

I'm not sure whether this is an issue with microbundle or rollup. A solution to this would be not to hoist require() calls if they are in a nested block or wrap them in a function to evaluate them lazily.

How to reproduce

Make sure you have yarn installed globally, then run following commands:

mkdir bug-test
cd bug-test
yarn add microbundle esm
echo "import 'esm'" > test.js
yarn microbundle test.js --format cjs --target node --external none
node dist/bug-test.js

Expected behaviour

Commands above should execute and exit without error code.

Actual behaviour

The node dist/bug-test.js command throws this error:

internal/modules/cjs/loader.js:969
  throw err;
  ^

Error: Cannot find module 'internal/bootstrap/loaders'
Require stack:
- /Users/dany/code/bug-test/dist/bug-test.js
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:966:15)
    at Function.Module._load (internal/modules/cjs/loader.js:842:27)
    at Module.require (internal/modules/cjs/loader.js:1026:19)
    at require (internal/modules/cjs/helpers.js:72:18)
    at Object.<anonymous> (/Users/dany/code/bug-test/dist/bug-test.js:7:31)
    at Module._compile (internal/modules/cjs/loader.js:1138:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1158:10)
    at Module.load (internal/modules/cjs/loader.js:986:32)
    at Function.Module._load (internal/modules/cjs/loader.js:879:14)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:71:12) {
  code: 'MODULE_NOT_FOUND',
  requireStack: [ '/Users/dany/code/bug-test/dist/bug-test.js' ]

The reason for this error is that there is this code in the esm module:

[…] function q(e){let t;try{const{internalBinding:n}=require("internal/bootstrap/loaders"),r=n("natives");x(r,e)&&(t=r[e])}catch(e){}return"string"==typeof t?t:""} […]

// Simplified:
try { require("internal/bootstrap/loaders") } catch (e) {}

which is modified to this by microbundle in dist/bug-test.js:

[…] var loaders = _interopDefault(require('internal/bootstrap/loaders')); […] function q(e){let t;try{const{internalBinding:n}=loaders,r=n("natives");x(r,e)&&(t=r[e]);}catch(e){}return "string"==typeof t?t:""} […]

// Simplified:
var loaders = require("internal/bootstrap/loaders")
try { loaders } catch (e) {}

Additional info

microbundle v0.12.0 Node v12.17.0 Mac OS 10.15.5 Related: https://github.com/nodejs/node/issues/33656

dcastil avatar May 30 '20 11:05 dcastil

Thanks for the detailed bug report 🎉

That's a tough one as there is no correct way to transform that code to esm without leading to unexpected and incorrect behaviour. microbundle is built on the assumption that the source is authored in esm. We ship with @rollup/plugin-commonjs by default to make a best effort to be able to bundle npm packages that don't have an esm entry.

In your particular example there is no valid equivalent in esm, because esm has no synchronous inline requires. The package you're trying to bundle (esm) is very special in that regard as it needs to patch in some error handling around each require call. I'm not sure if even other bundlers are able to bundle it correctly.

One might assume that only specifying cjs as a format has some special code paths for that, but I'm not aware of any rollup plugin that does that. All in all microbundle is an opiniated rollup preset in a nutshell and we rely on existing rollup plugins.

marvinhagemeister avatar May 30 '20 16:05 marvinhagemeister

Thanks for your explanation! Makes sense that the code ends up like this after being transformed to ESM and back.

Hmmm yeah that seems like a tough edge case. I'm trying to bundle a VS Code extension (which is essentially a NodeJS app) that needs to evaluate specific ES6 modules in the editor workspace at runtime. Probably that's too far of what microbundle was built for, the transform of commonjs dependencies to esm isn't necessary in my case.

dcastil avatar May 30 '20 18:05 dcastil

It's a bug in one of our dependencies. I'm sure the rollup folks can easily fixed it when they're made aware of it :+1:

marvinhagemeister avatar Jun 13 '20 11:06 marvinhagemeister