oven-sh / bun

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

`Bun.build` makes reference to undefined variable `tagSymbol` #11032

Open ashtonsix opened 4 months ago

ashtonsix commented 4 months ago

What version of Bun is running?

1.1.7+b0b7db5c0

What platform is your computer?

Darwin 23.1.0 arm64 arm

What steps can reproduce the bug?

import { tmpdir } from "os";

const fpathentry = `${tmpdir()}/${Math.random().toFixed(36).slice(2)}.js`;
const fpathmod = `${tmpdir()}/${Math.random().toFixed(36).slice(2)}.js`;
const outdir = `${tmpdir()}/${Math.random().toFixed(36).slice(2)}`;

await Bun.write(
  fpathmod,
  `if (condition) exports.x = "x"; else exports.x = "y";`,
);

await Bun.write(fpathentry, `import {x} from "${fpathmod}"; export {x}`);

const output = await Bun.build({
  entrypoints: [fpathentry],
  target: "browser",
  outdir,
});

const outpath = output.outputs.find((o) => o.kind === "entry-point")!.path;

console.log(await Bun.file(outpath).text());

What is the expected behavior?

Valid build output

What do you see instead?

Invalid build output, that makes reference to undefined variable tagSymbol:

if (condition) {
  var $x = "x";

  export { $x as x };
} else
  $x = "y";
export {
  tagSymbol as x
};

Additional information

I encountered this error while trying to create a minimal reproduction for another error (not yet reproduced), where the following build output was produced during a build of react-dom (invalid because it makes reference to exports without definining it):

var require_client = __commonJS(() => {
  if (false) {
  } else {
    exports.createRoot = /* ... */
  }
});
kzc commented 4 months ago

There are two unrelated bugs at play here. One is importing a non-existent symbol from a commonjs module, and the other has to do with not detecting exports correctly in a commonjs source file for branches.

Here's a reduced test case for the first bug:

$ cat 11032a.cjs 
exports.unused = 123;
$ cat 11032a.mjs 
import {noSuchVariable1, noSuchVariable2} from "./11032a.cjs";
export {noSuchVariable1, noSuchVariable2};
console.log(noSuchVariable1, noSuchVariable2);
$ bun build 11032a.mjs
// 11032a.mjs
console.log(undefined, undefined);
export {
  tagSymbol as noSuchVariable2,
  tagSymbol as noSuchVariable1
};

Having tagSymbol show up in bun build output is rather odd because it doesn't appear as a string literal in the bun code base. It only appears as a variable name in bun/src/runtime.js.

Let's do a strings on the bun binary to see if it is found elsewhere:

$ strings bun | grep tagSymbol
var tagSymbol;
  tagSymbol ??= Symbol.for("CommonJSTransformed");
    if ((kind === "object" || kind === "function") && !mod_exports[tagSymbol]) {
      Object.defineProperty(mod_exports, tagSymbol, {
var tagSymbol;
  tagSymbol ??= Symbol.for("CommonJSTransformed");
    if ((kind === "object" || kind === "function") && !mod_exports[tagSymbol]) {
      Object.defineProperty(mod_exports, tagSymbol, {
var tagSymbol;
  tagSymbol ??= Symbol.for("CommonJSTransformed");
    if ((kind === "object" || kind === "function") && !mod_exports[tagSymbol]) {
      Object.defineProperty(mod_exports, tagSymbol, {
var tagSymbol;
  tagSymbol ??= Symbol.for("CommonJSTransformed");
    if ((kind === "object" || kind === "function") && !mod_exports[tagSymbol]) {
      Object.defineProperty(mod_exports, tagSymbol, {

Nope. But it's curious that the runtime.js contents appears 4 times in the bun binary - not sure why that's the case.

Let's edit the bun binary as follows:

$ strings bun-edited | grep t.gSymbol
var t1gSymbol;
  t1gSymbol ??= Symbol.for("CommonJSTransformed");
    if ((kind === "object" || kind === "function") && !mod_exports[t1gSymbol]) {
      Object.defineProperty(mod_exports, t1gSymbol, {
var t2gSymbol;
  t2gSymbol ??= Symbol.for("CommonJSTransformed");
    if ((kind === "object" || kind === "function") && !mod_exports[t2gSymbol]) {
      Object.defineProperty(mod_exports, t2gSymbol, {
var t3gSymbol;
  t3gSymbol ??= Symbol.for("CommonJSTransformed");
    if ((kind === "object" || kind === "function") && !mod_exports[t3gSymbol]) {
      Object.defineProperty(mod_exports, t3gSymbol, {
var t4gSymbol;
  t4gSymbol ??= Symbol.for("CommonJSTransformed");
    if ((kind === "object" || kind === "function") && !mod_exports[t4gSymbol]) {
      Object.defineProperty(mod_exports, t4gSymbol, {

Now let's rerun the test case with the newly edited bun binary:

$ ./bun-edited build 11032a.mjs
// 11032a.mjs
console.log(undefined, undefined);
export {
  t1gSymbol as noSuchVariable2,
  t1gSymbol as noSuchVariable1
};

If the commonjs symbol import is not found then bun build ought to be erroring out instead of using the first variable name declared in the first instance of bun/src/runtime.js embedded in the bun binary.

Now let's look at reduced test cases for the second bug.

$ cat 11032b.cjs 
if (Math.random() < .5) exports.foo = "foo"; // does not work
console.log(exports.foo);
$ bun build 11032b.cjs 
// 11032b.cjs
if (Math.random() < 0.5) {
  var $foo = "foo";

  export { $foo as foo };
}
console.log($foo);

The export block emitted is invalid.

Let's change the test case slightly to introduce braces in the if block:

$ cat 11032c.cjs 
if (Math.random() < .5) {exports.bar = "bar";} // works if braces present
console.log(exports.bar);
$ bun build 11032c.cjs 
var __commonJS = (cb, mod) => () => (mod || cb((mod = { exports: {} }).exports, mod), mod.exports);

// 11032c.cjs
var require_11032c = __commonJS((exports) => {
  if (Math.random() < 0.5) {
    exports.bar = "bar";
  }
  console.log(exports.bar);
});
export default require_11032c();
$ bun build 11032c.cjs | bun -
bar
$ bun build 11032c.cjs | bun -
undefined

That works fine.

Now let's remove the if braces again, but "predeclare" the commonjs export:

$ cat 11032d.cjs 
exports.baz = undefined; // works if cjs export is "predeclared" at top level
if (Math.random() < .5) exports.baz = "baz"; // note no braces
console.log(exports.baz);
$ bun build 11032d.cjs
// 11032d.cjs
var $baz = undefined;
if (Math.random() < 0.5)
  $baz = "baz";
console.log($baz);
export {
  $baz as baz
};
$ bun build 11032d.cjs | bun -
baz
$ bun build 11032d.cjs | bun -
undefined

That also works.

A bug fix might investigate one of these workarounds.