Skip to content

Name the binding and the module when a re-export does not resolve (WebKit bump) #41840

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.

Sign up for GitHub

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

Draft
robobun wants to merge 5 commits into
base: main
Choose a base branch
from
+112 −3
Draft

Name the binding and the module when a re-export does not resolve (WebKit bump) #41840

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a95ecee
Name the binding and the module when a re-export does not resolve
robobun Sep 7, 2026
673abbc
Merge branch 'main' into robobun/3b9d0a79/reexport-link-diagnostics
Jarred-Sumner Sep 7, 2026
3f698e1
Merge remote-tracking branch 'origin/main' into robobun/3b9d0a79/reex…
robobun Sep 14, 2026
7d8c91b
Cover the re-export link errors in compiled executables
robobun Sep 14, 2026
c4aba83
test: use describe.each for the re-export link error cases
robobun Sep 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion scripts/build/deps/webkit.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* prebuilt tarball's release tag). Override via `--webkit-version=<hash>` to test a
* branch. From https://github.com/oven-sh/WebKit releases.
*/
export const WEBKIT_VERSION = "2e2aa2290fac856d6f451ceacb58f7f5b44dd057";
export const WEBKIT_VERSION = "da1219be982e62fbc8a1a7f0a77372b107c98dd2";

@claude claude Bot Sep 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 WEBKIT_VERSION is pinned to da1219be982e…, the head of the unmerged oven-sh/WebKit#581 branch — an ephemeral ref that a force-push or branch delete will make unfetchable, breaking every clean build's sparse WebKit fetch. Fix: before merge, swap this to the resulting oven-sh/WebKit main SHA once #581 lands (REVIEW.md / landing-prs.md "Dependencies & vendoring": never merge a pin to an unmerged-PR build). The PR description already flags this as a pre-merge task; this comment tracks it so it cannot be approved past.

Extended reasoning...

scripts/build/deps/README.md and .claude/docs/landing-prs.md require WEBKIT_VERSION to be a commit that exists on oven-sh/WebKit main, because the build does a sparse git fetch of exactly that SHA on every platform (deps/webkit.ts → source.kind "github" with sparse). da1219be982e is the branch head of oven-sh/WebKit#581, which the PR author states is not yet merged. If this PR merges before #581, or #581 is rebased/force-pushed before merging, that commit disappears from the remote and every fresh bun run build fails at the WebKit fetch step. Base branch pins 2e2aa2290fac…, a merged main commit, so this is a regression relative to base. The author already noted this must change before landing; the finding confirms and tracks that requirement as a review gate.

Verification: normal — acknowledged in diff: the PR description itself states "This PR points WEBKIT_VERSION at that branch head (da1219be982e). Before this lands, oven-sh/WebKit#581 has to merge and the pin has to move to the resulting main sha." That claim checks out against the code and the repo rules. /home/claude/bun/scripts/build/deps/webkit.ts:6 pins `WEBKIT_VERSION =… | normal — acknowledged in…

@robobun robobun Sep 14, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, this pin must not merge as it is. da1219be982e was a branch head, and since the revert of #41330 it did not resolve to a prebuilt tarball either, which is why every build-bun lane of build 112197 failed.

The pin is now autobuild-preview-pr-581-e075a38d, the preview build of oven-sh/WebKit#581 rebased on WebKit main. CI can fetch it on every lane. It is still not a main sha, so I converted the PR to a draft. When oven-sh/WebKit#581 merges, the pin moves to the merge commit and the PR leaves draft. I leave this thread open until then.

@claude claude Bot Sep 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pin moved from the raw branch SHA to autobuild-preview-pr-581-e075a38d, but that is still an ephemeral preview build of the unmerged oven-sh/WebKit#581 — the tag name says so. The landing-prs.md "Dependencies & vendoring" rule this comment tracks is about not merging with WEBKIT_VERSION pointed at any unmerged-PR artifact, not just an unfetchable SHA. Before this PR lands, #581 needs to merge and this constant needs to become the resulting oven-sh/WebKit main commit hash.


/**
* WebKit (JavaScriptCore) — the JS engine, with WTF and bmalloc.
Expand Down
79 changes: 77 additions & 2 deletions test/js/bun/typescript/type-export.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,9 @@ test.concurrent("js file with through export", async () => {

const result = await run([bunExe(), "b.js"], dir);

expect(result.stderr.trim()).toInclude("SyntaxError: export 'type_only' not found in './ts.ts'");
expect(result.stderr.trim()).toMatch(
/^SyntaxError: Export named 'type_only' not found in module '[^']*[\\/]ts\.ts'\.$/m,
);
expect(result.exitCode).toBe(1);
});

Expand All @@ -240,10 +242,83 @@ test.concurrent("js file with through export 2", async () => {

const result = await run([bunExe(), "b.js"], dir);

expect(result.stderr.trim()).toInclude("SyntaxError: export 'type_only' not found in './ts.ts'");
expect(result.stderr.trim()).toMatch(
/^SyntaxError: Export named 'type_only' not found in module '[^']*[\\/]ts\.ts'\.$/m,
);
expect(result.exitCode).toBe(1);
});

// The link-time check for `export { a as b } from "./m"` used to name the alias
// (`b`) and, for a missing `default`, print "export default cannot be used with
// export *". It now uses the same messages as `import { a } from "./m"`: the
// requested binding and the resolved path of the requested module.
describe("re-export of a binding the requested module does not provide", () => {
const cases: [name: string, files: Record<string, string>, message: RegExp][] = [
[
"export { default } from",
{
"p.mjs": `export const a = 1;`,
"mid.mjs": `export { default } from "./p.mjs";`,
"main.mjs": `import d from "./mid.mjs"; console.log(d);`,
},
/^SyntaxError: Missing 'default' export in module '[^']*[\\/]p\.mjs'\.$/m,
],
[
"export { default as x } from",
{
"p.mjs": `export const a = 1;`,
"mid.mjs": `export { default as x } from "./p.mjs";`,
"main.mjs": `import { x } from "./mid.mjs"; console.log(x);`,
},
/^SyntaxError: Missing 'default' export in module '[^']*[\\/]p\.mjs'\.$/m,
],
[
"import d from; export { d as x }",
{
"p.mjs": `export const a = 1;`,
"mid.mjs": `import d from "./p.mjs"; export { d as x };`,
"main.mjs": `import { x } from "./mid.mjs"; console.log(x);`,
},
/^SyntaxError: Missing 'default' export in module '[^']*[\\/]p\.mjs'\.$/m,
],
[
"export { nope as y } from",
{
"p.mjs": `export const a = 1;`,
"mid.mjs": `export { nope as y } from "./p.mjs";`,
"main.mjs": `import { y } from "./mid.mjs"; console.log(y);`,
},
/^SyntaxError: Export named 'nope' not found in module '[^']*[\\/]p\.mjs'\.$/m,
],
[
"export { dup as z } from a module with conflicting export *",
{
"a.mjs": `export const dup = "a";`,
"b.mjs": `export const dup = "b";`,
"root.mjs": `export * from "./a.mjs"; export * from "./b.mjs";`,
"mid.mjs": `export { dup as z } from "./root.mjs";`,
"main.mjs": `import { z } from "./mid.mjs"; console.log(z);`,
},
/^SyntaxError: Export named 'dup' cannot be resolved due to ambiguous multiple bindings in module '[^']*[\\/]root\.mjs'\.$/m,
],
];

for (const [name, files, message] of cases) {
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
// The check runs when mid.mjs links, so it fires the same way whether
// mid.mjs is a dependency or the entry point.
for (const entry of ["main.mjs", "mid.mjs"]) {
test.concurrent(`${name} (entry ${entry})`, async () => {
await using dir = tempDir("reexport-missing", files);

const result = await run([bunExe(), entry], String(dir));

expect(result.stderr.trim()).toMatch(message);
expect({ stdout: result.stdout, exitCode: result.exitCode }).toEqual({ stdout: "", exitCode: 1 });
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
});
}
}
});

describe("through export merge", () => {
// this isn't allowed, even in typescript (tsc emits "Duplicate identifier 'value'.")
for (const fmt of ["js", "ts"]) {
Expand Down
Loading