oven-sh / bun

Incredibly fast JavaScript runtime, bundler, test runner, and package manager – all in one
https://bun.sh
Other
74.1k stars 2.77k forks source link

Issue with `@protobufjs/inquire` when bundled with Bun #14891

Open Nevexo opened 1 week ago

Nevexo commented 1 week ago

What version of Bun is running?

1.1.33+247456b67

What platform is your computer?

Linux 5.15.153.1-microsoft-standard-WSL2 x86_64 x86_64

What steps can reproduce the bug?

Clone demo repo: https://github.com/Nevexo/bun-protobufjs-issue

Install deps and run as usual, should work as expected listening on port 50051 on loopback interface.

Testing the gRPC endpoint isn't required, but use the reflector to get the protobuf and send a request to /SayHello with "name" set, expect a message back including the name value.

Build the repo with Bun, bun build --target bun --sourcemap --outdir ./dist/ .

Run the bundled file bun ./dist/index.js, expect error ReferenceError: Can't find variable: XMLHttpRequest

What is the expected behavior?

Should run and behave as the non-bundled verison.

What do you see instead?

75 |  * @variation 3
76 |  */
77 |
78 | /**/
79 | fetch.xhr = function fetch_xhr(filename, options, callback) {
80 |     var xhr = new XMLHttpRequest();
                       ^
ReferenceError: Can't find variable: XMLHttpRequest
      at fetch_xhr 

Additional information

Discord thread with initial investigation: https://canary.discord.com/channels/876711213126520882/1300770124260970527

protobufjs is a dependency of @grpc/grpc-js.

I know XMLHttpRequest() isn't supported, but protobufjs shouldn't even be attempting to use XMR as the fs library is present, and all required methods are supported (readFile and readFileSync), however the protobufjs inquire() method, which will call require against a module only if it exists, always fails to import fs after bundling.

The library is supposed to read the protobuf file from disk, but will fallback to XMLHttpRequest if the fs library isn't available.

`inquire()` implementation ```js function inquire(moduleName) { try { var mod = eval("quire".replace(/^/,"re"))(moduleName); // eslint-disable-line no-eval if (mod && (mod.length || Object.keys(mod).length)) return mod; } catch (e) {} // eslint-disable-line no-empty return null; } ```

It's possible to patch the issue out by removing calls to inquire() and replacing them with cjs require(), and removing the check in lib/fetch/index.js's fetch(), which checks for the presence of the fs module.

That's about as far as I've managed to trace it back so far, as I don't have much knowledge of the bundler, but happy to help with any further investigation into the issue.

paperdave commented 1 week ago

Image

Seen this in another package that uses eval. I think we should make any direct eval in any file ensure that unminified require, __dirname, and __filename all exist.