oven-sh / bun

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

`bun build --compile`: `Bun.resolve(..., import.meta.url)` resolves relative to `cwd` #13405

Closed ctjlewis closed 2 months ago

ctjlewis commented 2 months ago

What version of Bun is running?

1.1.25-canary.36+d55b5cc16

What platform is your computer?

Darwin 23.6.0 arm64 arm

What steps can reproduce the bug?

Create child directory with compile.ts in it. Put test.txt next to it.

// dir/compile.ts
import { createRequire } from "module"

console.log(Bun.resolveSync("./test.txt", import.meta.url))
console.log(createRequire(import.meta.url).resolve("./test.txt"))

From outside of dir/, you can see that bun dir/compile.ts correctly resolves to ./text relative to dir/compile.ts, i.e. dir/test.txt.

When you compile it, even without --outfile:

bun build --compile compile.ts

And then exit the directory and run the binary, it will error:

error: Cannot find module "./test.txt" from "file:///$bunfs/root/compile"

If you're inside the directory, i.e. cwd is . relative to import.meta.url, the binary will work. If test.txt is in cwd, it will work. But it is not correctly resolved relative to the binary itself.

What is the expected behavior?

The ./file case should work for binaries, including for createRequire(import.meta.url).resolve.

Running the binary from outside the directory should be like bun compile.ts, and resolve relative to the file.

What do you see instead?

The binary tries to resolve relative to the current working directory. If you move test.txt to cwd and run the binary from another folder, it will work.

Additional information

Related:

ctjlewis commented 2 months ago

cc @Jarred-Sumner, don't mean to ping you for every issue I open but if we can always guarantee these statements map correctly, then we can always correct for situations like this by just copying dependencies from node_modules to the build directory.

If dependency A requires ./internal/thing.wasm, we can just cp -r node_modules/thing_module/dist/internal ./dist if it comes down to it, and any import ... from "./internal/thing.wasm" statements will resolve correctly. Here it's even more trivial because it's just ./yoga.wasm.

It makes sense that there was a hole here because --compile intends to put everything into the binary and not have external dependencies.

Jarred-Sumner commented 2 months ago

I guess we could make createRequire(import.meta.url) become require at bundle time and then inline import.meta.url and then require.resolve would cause the .wasm file to be bundled correctly

ctjlewis commented 2 months ago

Oh you're right, it does work for vanilla require(). Though the forms differ (Module vs string):

console.log(require("./test.txt"))

TS:

bun dir/compile.ts

Module {
  __esModule: true,
  default: "Metamorphosis\n\nIn the garden of life, we plant our seeds,\nNurtured by time, watered by deeds.\nEach day a petal, each year a leaf,\nGrowing through joy, and sometimes grief.\n\nFrom tender sprout to mighty tree,\nWe stretch our branches, wild and free.\nRoots deep in earth, crown touching sky,\nA living testament of you and I.\n\nThrough seasons changing, we adapt and thrive,\nIn nature's dance, we come alive.\nFrom caterpillar to butterfly's wing,\nWe emerge renewed, ready to sing.\n\nSo embrace the change, let yourself grow,\nFor in transformation, your true self you'll know.",
}

Binary, outside dir:

./dir/compile 

Metamorphosis

In the garden of life, we plant our seeds,
Nurtured by time, watered by deeds.
Each day a petal, each year a leaf,
Growing through joy, and sometimes grief.

From tender sprout to mighty tree,
We stretch our branches, wild and free.
Roots deep in earth, crown touching sky,
ctjlewis commented 2 months ago

Can't inline import.meta.url because we might install the application anywhere, right? We just have to make sure the ESM equivalent of require("./...") works with createRequire(import.meta.url).resolve("./..."), that it doesn't try to read it as if its relative to cwd and not import.meta.url.

Or maybe there is a way to inline it in the actual binary itself that I don't know much about. Maybe a $0 in Bash type thing?


I guess it really could have been brought into the binary the same way an import or require statement would have brought in a PNG or something, and we are just surprised to see it act as a side effect here, like normal code. It's unexpected that createRequire(import.meta.url).resolve(path) and require.resolve(path) would turn up different things (former: error, latter: correct output) from outside the directory.

Ultimately, createRequire(import.meta.url) should be perfectly synonymous with require in a compiled program, and for --compile or --bundle --target bun, createRequire(import.meta.url) can be inlined to global require directly. Or, that statement should always return the global require, or whatever divergence is happening there should be corrected.

I wish I could be more help actually fixing these!

ctjlewis commented 2 months ago

Also, in Bun, require is defined in global scope but is not on global/globalThis? The change I made in Yoga's repo would still require one more shim on the user's end: globalThis.require = require. But if you do that before the ink imports, then it would work, at least.

Workaround for Yoga

For now, it is possible to build this program by:

  1. Building a single bundle: bun build program.ts --bundle
  2. Manually or programmatically replacing createRequire(import.meta.url) with raw require. In Yoga's upstream bundle, it's minified, so it's actually _(import.meta.url).
  3. Compiling that edited bundle to a binary with bun build --compile built.js.
Jarred-Sumner commented 2 months ago

Related https://github.com/oven-sh/bun/issues/13418

ctjlewis commented 2 months ago

Shimming currently:

// shim.ts
const appBundle = Bun.file("dist/app.js")
let content = await new Response(appBundle).text()

// Replace createRequire(import.meta.url) with require
content = content.replace(/createRequire\(import\.meta\.url\)/g, "require")

// Replace $(import.meta.url) where $ is "createRequire as something"
const createRequireAsRegex = /(?<=createRequire as )(.+?)(\w*)(?=\})/g
const matches = content.match(createRequireAsRegex)

if (!matches?.length) {
  throw new Error("No matches found")
}

console.log(matches)
for (const match of matches) {
  content = content.replaceAll(new RegExp(`${match}(import.meta.url)`, "g"), "require")
}

await Bun.write("dist/app.js", content)
console.log("Successfully shimmed dist/app.js")
"scripts": {
    "bundle": "bun build --minify --bundle src/ink/app.tsx --outfile dist/app.js --target node",
    "bundle-replace": "bun shim.ts",
    "compile": "bun build --minify --compile dist/app.js --outfile dist/app",
    "postcompile": "cp node_modules/yoga-wasm-web/dist/yoga.wasm dist",
}