Fix panic when parsing declare global with type annotations #24491

robobun · 2025-11-08T01:30:13Z

Summary

Fixes a panic that occurs when parsing TypeScript code with declare global { ... } blocks containing type annotations.

The Problem

When parsing code like this:

declare global {
  TIMER: NodeJS.Timeout;
}

globalThis.TIMER = setInterval(() => console.log("Started"), 1000);

The parser incorrectly treated TIMER: NodeJS.Timeout; as a labeled statement instead of a type annotation. This caused the parser to push a .label scope during the parse pass, but when visiting the arrow function later, it expected a .function_args scope, resulting in:

panic(main thread): Scope mismatch while visiting
Expected this scope (.function_args)
Found this scope (.label)

The Fix

Added a check in src/ast/parseStmt.zig to detect when we're in a TypeScript declare context (opts.is_typescript_declare). When an identifier followed by a colon is encountered in this context, we now skip the type annotation and return a TypeScript node instead of creating a label scope.

Testing

Added comprehensive tests in test/js/bun/transpiler/declare-global.test.ts
Tests fail on stable Bun (reproduces the crash) ✅
Tests pass with this fix ✅
All existing transpiler tests still pass ✅

🤖 Generated with Claude Code

Co-Authored-By: Claude [email protected]

When parsing TypeScript code with `declare global { ... }` blocks, identifiers followed by colons (like `TIMER: NodeJS.Timeout;`) were incorrectly treated as labeled statements instead of type annotations. This caused the parser to push a `.label` scope during the parse pass, but when visiting the AST later, arrow functions expected `.function_args` scopes, resulting in a "Scope mismatch while visiting" panic. The fix checks if we're in a TypeScript declare context before treating an identifier followed by a colon as a labeled statement. If we are in a declare context, we skip the type annotation and return a TypeScript node instead of creating a label scope. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>

robobun · 2025-11-08T01:30:22Z

^{Updated 7:33 PM PT - Nov 7th, 2025}

❌ Your commit c3c02d39 has 3 failures in Build #31274 (All Failures):

test/v8/v8.test.ts - 1 failing on 🐧 12 x64-asan
test/js/node/test/parallel/test-http-url.parse-https.request.js - core dumped on 🐧 12 x64-asan
test/js/node/path/browserify.test.js - 1 failing on 🪟 2019 x64
test/js/node/path/browserify.test.js - 1 failing on 🪟 2019 x64-baseline

🧪 To try this PR locally:

bunx bun-pr 24491

That installs a local version of the PR into your bun-24491 executable, so you can run:

bun-24491 --bun

coderabbitai · 2025-11-08T01:33:59Z

Walkthrough

Adds special-case parsing to treat identifier: Type as a TypeScript declaration in declare contexts (skipping the type and requiring a semicolon) and adds tests that run global TypeScript declare statements to ensure no panics or scope mismatches.

Changes

Cohort / File(s)	Summary
TypeScript declare context parsing `src/ast/parseStmt.zig`	Detects `identifier: Type` when TypeScript parsing is enabled and the current context is a TypeScript declare; advances past the colon, skips the type annotation, expects/inserts a semicolon, and returns a TypeScript marker node early. Applied in both the main and fallthrough identifier parsing paths.
Global type declaration tests `test/js/bun/transpiler/declare-global.test.ts`	Adds two Bun-based tests that run scripts with global `declare` TypeScript statements and assert no panics or scope mismatches on stderr, that stdout contains "SUCCESS", and that exit code is 0.

Possibly related PRs

fix(parser): typescript module parsing bug #23284: Modifies src/ast/parseStmt.zig to adjust TypeScript declare/identifier parsing and disambiguation logic, closely related at the code level.

Suggested reviewers

pfgithub
dylan-conway

Pre-merge checks

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: fixing a panic when parsing TypeScript declare global blocks with type annotations.
Description check	✅ Passed	The description comprehensively covers both required template sections: what the PR does (includes problem, solution, and testing) and how the code was verified.

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between d2cf560 and 749552a.

📒 Files selected for processing (1)

test/js/bun/transpiler/declare-global.test.ts (1 hunks)

🧰 Additional context used

📓 Path-based instructions (4)

test/**

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

Place all tests under the test/ directory

Files:

test/js/bun/transpiler/declare-global.test.ts

test/js/**/*.{js,ts}

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

Place JavaScript and TypeScript tests under test/js/

Files:

test/js/bun/transpiler/declare-global.test.ts

test/js/bun/**/*.{js,ts}

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

Place Bun API tests under test/js/bun/, separated by category (e.g., test/js/bun/glob/)

Files:

test/js/bun/transpiler/declare-global.test.ts

test/**/*.{js,ts}

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

test/**/*.{js,ts}: Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test
Prefer data-driven tests (e.g., test.each) to reduce boilerplate
Use shared utilities from test/harness.ts where applicable

Files:

test/js/bun/transpiler/declare-global.test.ts

🧠 Learnings (15)

📓 Common learnings

Learnt from: cirospaciari
Repo: oven-sh/bun PR: 22946
File: test/js/sql/sql.test.ts:195-202
Timestamp: 2025-09-25T22:07:13.851Z
Learning: PR oven-sh/bun#22946: JSON/JSONB result parsing updates (e.g., returning parsed arrays instead of legacy strings) are out of scope for this PR; tests keep current expectations with a TODO. Handle parsing fixes in a separate PR.

📚 Learning: 2025-08-30T00:12:56.803Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/js/bun/**/*.{js,ts} : Place Bun API tests under test/js/bun/, separated by category (e.g., test/js/bun/glob/)

Applied to files:

test/js/bun/transpiler/declare-global.test.ts

📚 Learning: 2025-08-30T00:12:56.803Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/**/*.{js,ts} : Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test

Applied to files:

test/js/bun/transpiler/declare-global.test.ts

📚 Learning: 2025-09-08T00:41:12.052Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-09-08T00:41:12.052Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8.test.ts : Add a corresponding test case in test/v8/v8.test.ts that invokes checkSameOutput with the new function

Applied to files:

test/js/bun/transpiler/declare-global.test.ts

📚 Learning: 2025-10-19T02:44:46.354Z

Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/context-propagation.test.ts:1-1
Timestamp: 2025-10-19T02:44:46.354Z
Learning: In the Bun repository, standalone packages under packages/ (e.g., bun-vscode, bun-inspector-protocol, bun-plugin-yaml, bun-plugin-svelte, bun-debug-adapter-protocol, bun-otel) co-locate their tests with package source code using *.test.ts files. This follows standard npm/monorepo patterns. The test/ directory hierarchy (test/js/bun/, test/cli/, test/js/node/) is reserved for testing Bun's core runtime APIs and built-in functionality, not standalone packages.

Applied to files:

test/js/bun/transpiler/declare-global.test.ts

📚 Learning: 2025-10-19T02:52:37.412Z

Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/tsconfig.json:1-15
Timestamp: 2025-10-19T02:52:37.412Z
Learning: In the Bun repository, packages under packages/ (e.g., bun-otel) can follow a TypeScript-first pattern where package.json exports point directly to .ts files (not compiled .js files). Bun natively runs TypeScript, so consumers import .ts sources directly and receive full type information without needing compiled .d.ts declaration files. For such packages, adding "declaration": true or "outDir" in tsconfig.json is unnecessary and would break the export structure.
<!-- [remove_learning]
ceedde95-980e-4898-a2c6-40ff73913664

Applied to files:

test/js/bun/transpiler/declare-global.test.ts

📚 Learning: 2025-08-30T00:09:39.100Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/bundle.test.ts : bundle.test.ts should contain DevServer-specific bundling tests

Applied to files:

test/js/bun/transpiler/declare-global.test.ts

📚 Learning: 2025-08-30T00:12:56.803Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/cli/**/*.{js,ts} : When testing Bun as a CLI, use spawn with bunExe() and bunEnv from harness, and capture stdout/stderr via pipes

Applied to files:

test/js/bun/transpiler/declare-global.test.ts

📚 Learning: 2025-08-30T00:09:39.100Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/esm.test.ts : esm.test.ts should cover ESM feature behavior in development mode

Applied to files:

test/js/bun/transpiler/declare-global.test.ts

📚 Learning: 2025-08-30T00:09:39.100Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/{dev/*.test.ts,dev-and-prod.ts} : Import testing utilities (devTest, prodTest, devAndProductionTest, Dev, Client) from test/bake/bake-harness.ts

Applied to files:

test/js/bun/transpiler/declare-global.test.ts

📚 Learning: 2025-08-30T00:09:39.100Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/html.test.ts : html.test.ts should contain tests relating to HTML files themselves

Applied to files:

test/js/bun/transpiler/declare-global.test.ts

📚 Learning: 2025-08-30T00:09:39.100Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/bake-harness.ts : Provide and maintain shared test utilities: devTest, prodTest, devAndProductionTest, Dev, Client, and helpers in the harness

Applied to files:

test/js/bun/transpiler/declare-global.test.ts

📚 Learning: 2025-11-06T00:58:23.965Z

Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24417
File: test/js/bun/spawn/spawn.test.ts:903-918
Timestamp: 2025-11-06T00:58:23.965Z
Learning: In Bun test files, `await using` with spawn() is appropriate for long-running processes that need guaranteed cleanup on scope exit or when explicitly testing disposal behavior. For short-lived processes that exit naturally (e.g., console.log scripts), the pattern `const proc = spawn(...); await proc.exited;` is standard and more common, as evidenced by 24 instances vs 4 `await using` instances in test/js/bun/spawn/spawn.test.ts.

Applied to files:

test/js/bun/transpiler/declare-global.test.ts

📚 Learning: 2025-10-08T13:48:02.430Z

Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 23373
File: test/js/bun/tarball/extract.test.ts:107-111
Timestamp: 2025-10-08T13:48:02.430Z
Learning: In Bun's test runner, use `expect(async () => { await ... }).toThrow()` to assert async rejections. Unlike Jest/Vitest, Bun does not require `await expect(...).rejects.toThrow()` - the async function wrapper with `.toThrow()` is the correct pattern for async error assertions in Bun tests.

Applied to files:

test/js/bun/transpiler/declare-global.test.ts

📚 Learning: 2025-10-30T03:48:10.513Z

Learnt from: theshadow27
Repo: oven-sh/bun PR: 24063
File: packages/bun-otel/test/context-propagation.test.ts:1-7
Timestamp: 2025-10-30T03:48:10.513Z
Learning: In Bun test files, `using` declarations at the describe block level execute during module load/parsing, not during test execution. This means they acquire and dispose resources before any tests run. For test-scoped resource management, use beforeAll/afterAll hooks instead. The pattern `beforeAll(beforeUsingEchoServer); afterAll(afterUsingEchoServer);` is correct for managing ref-counted test resources like the EchoServer in packages/bun-otel/test/ - the using block pattern should not be used at describe-block level for test resources.
<!-- [/add_learning]

Applied to files:

test/js/bun/transpiler/declare-global.test.ts

🧬 Code graph analysis (1)

test/js/bun/transpiler/declare-global.test.ts (1)

test/harness.ts (2)

tempDir (277-284)

bunExe (102-105)

🔇 Additional comments (1)

test/js/bun/transpiler/declare-global.test.ts (1)

4-31: LGTM! Test effectively validates the parser fix.

The test correctly reproduces the original bug scenario: a declare global block with type annotation followed by an arrow function. This combination previously caused the scope mismatch panic described in the PR.

test/js/bun/transpiler/declare-global.test.ts

coderabbitai

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between f61c7e1 and c3c02d3.

📒 Files selected for processing (1)

test/js/bun/transpiler/declare-global.test.ts (1 hunks)

🧰 Additional context used

📓 Path-based instructions (4)

test/**

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

Place all tests under the test/ directory

Files:

test/js/bun/transpiler/declare-global.test.ts

test/js/**/*.{js,ts}

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

Place JavaScript and TypeScript tests under test/js/

Files:

test/js/bun/transpiler/declare-global.test.ts

test/js/bun/**/*.{js,ts}

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

Place Bun API tests under test/js/bun/, separated by category (e.g., test/js/bun/glob/)

Files:

test/js/bun/transpiler/declare-global.test.ts

test/**/*.{js,ts}

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

test/**/*.{js,ts}: Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test
Prefer data-driven tests (e.g., test.each) to reduce boilerplate
Use shared utilities from test/harness.ts where applicable

Files:

test/js/bun/transpiler/declare-global.test.ts

🧠 Learnings (17)

📓 Common learnings

Learnt from: cirospaciari
Repo: oven-sh/bun PR: 22946
File: test/js/sql/sql.test.ts:195-202
Timestamp: 2025-09-25T22:07:13.851Z
Learning: PR oven-sh/bun#22946: JSON/JSONB result parsing updates (e.g., returning parsed arrays instead of legacy strings) are out of scope for this PR; tests keep current expectations with a TODO. Handle parsing fixes in a separate PR.

📚 Learning: 2025-08-30T00:12:56.803Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/js/bun/**/*.{js,ts} : Place Bun API tests under test/js/bun/, separated by category (e.g., test/js/bun/glob/)

Applied to files:

test/js/bun/transpiler/declare-global.test.ts

📚 Learning: 2025-08-30T00:12:56.803Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/**/*.{js,ts} : Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test

Applied to files:

test/js/bun/transpiler/declare-global.test.ts

📚 Learning: 2025-10-19T02:44:46.354Z

Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/context-propagation.test.ts:1-1
Timestamp: 2025-10-19T02:44:46.354Z
Learning: In the Bun repository, standalone packages under packages/ (e.g., bun-vscode, bun-inspector-protocol, bun-plugin-yaml, bun-plugin-svelte, bun-debug-adapter-protocol, bun-otel) co-locate their tests with package source code using *.test.ts files. This follows standard npm/monorepo patterns. The test/ directory hierarchy (test/js/bun/, test/cli/, test/js/node/) is reserved for testing Bun's core runtime APIs and built-in functionality, not standalone packages.

Applied to files:

test/js/bun/transpiler/declare-global.test.ts

📚 Learning: 2025-09-08T00:41:12.052Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-09-08T00:41:12.052Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8.test.ts : Add a corresponding test case in test/v8/v8.test.ts that invokes checkSameOutput with the new function

Applied to files:

test/js/bun/transpiler/declare-global.test.ts

📚 Learning: 2025-10-19T02:52:37.412Z

Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/tsconfig.json:1-15
Timestamp: 2025-10-19T02:52:37.412Z
Learning: In the Bun repository, packages under packages/ (e.g., bun-otel) can follow a TypeScript-first pattern where package.json exports point directly to .ts files (not compiled .js files). Bun natively runs TypeScript, so consumers import .ts sources directly and receive full type information without needing compiled .d.ts declaration files. For such packages, adding "declaration": true or "outDir" in tsconfig.json is unnecessary and would break the export structure.
<!-- [remove_learning]
ceedde95-980e-4898-a2c6-40ff73913664

Applied to files:

test/js/bun/transpiler/declare-global.test.ts

📚 Learning: 2025-08-30T00:09:39.100Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/bundle.test.ts : bundle.test.ts should contain DevServer-specific bundling tests

Applied to files:

test/js/bun/transpiler/declare-global.test.ts

📚 Learning: 2025-08-30T00:09:39.100Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/{dev/*.test.ts,dev-and-prod.ts} : Import testing utilities (devTest, prodTest, devAndProductionTest, Dev, Client) from test/bake/bake-harness.ts

Applied to files:

test/js/bun/transpiler/declare-global.test.ts

📚 Learning: 2025-08-30T00:09:39.100Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/html.test.ts : html.test.ts should contain tests relating to HTML files themselves

Applied to files:

test/js/bun/transpiler/declare-global.test.ts

📚 Learning: 2025-08-30T00:09:39.100Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/css.test.ts : css.test.ts should contain CSS bundling tests in dev mode

Applied to files:

test/js/bun/transpiler/declare-global.test.ts

📚 Learning: 2025-08-30T00:09:39.100Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/esm.test.ts : esm.test.ts should cover ESM feature behavior in development mode

Applied to files:

test/js/bun/transpiler/declare-global.test.ts

📚 Learning: 2025-09-08T00:41:12.052Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-09-08T00:41:12.052Z
Learning: Follow existing patterns in similar V8 classes, add comprehensive Node.js parity tests, update all symbol files, and document any special behavior

Applied to files:

test/js/bun/transpiler/declare-global.test.ts

📚 Learning: 2025-08-30T00:12:56.803Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/cli/**/*.{js,ts} : When testing Bun as a CLI, use spawn with bunExe() and bunEnv from harness, and capture stdout/stderr via pipes

Applied to files:

test/js/bun/transpiler/declare-global.test.ts

📚 Learning: 2025-11-06T00:58:23.965Z

Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24417
File: test/js/bun/spawn/spawn.test.ts:903-918
Timestamp: 2025-11-06T00:58:23.965Z
Learning: In Bun test files, `await using` with spawn() is appropriate for long-running processes that need guaranteed cleanup on scope exit or when explicitly testing disposal behavior. For short-lived processes that exit naturally (e.g., console.log scripts), the pattern `const proc = spawn(...); await proc.exited;` is standard and more common, as evidenced by 24 instances vs 4 `await using` instances in test/js/bun/spawn/spawn.test.ts.

Applied to files:

test/js/bun/transpiler/declare-global.test.ts

📚 Learning: 2025-08-30T00:09:39.100Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/bake-harness.ts : Provide and maintain shared test utilities: devTest, prodTest, devAndProductionTest, Dev, Client, and helpers in the harness

Applied to files:

test/js/bun/transpiler/declare-global.test.ts

📚 Learning: 2025-10-08T13:48:02.430Z

Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 23373
File: test/js/bun/tarball/extract.test.ts:107-111
Timestamp: 2025-10-08T13:48:02.430Z
Learning: In Bun's test runner, use `expect(async () => { await ... }).toThrow()` to assert async rejections. Unlike Jest/Vitest, Bun does not require `await expect(...).rejects.toThrow()` - the async function wrapper with `.toThrow()` is the correct pattern for async error assertions in Bun tests.

Applied to files:

test/js/bun/transpiler/declare-global.test.ts

📚 Learning: 2025-10-30T03:48:10.513Z

Learnt from: theshadow27
Repo: oven-sh/bun PR: 24063
File: packages/bun-otel/test/context-propagation.test.ts:1-7
Timestamp: 2025-10-30T03:48:10.513Z
Learning: In Bun test files, `using` declarations at the describe block level execute during module load/parsing, not during test execution. This means they acquire and dispose resources before any tests run. For test-scoped resource management, use beforeAll/afterAll hooks instead. The pattern `beforeAll(beforeUsingEchoServer); afterAll(afterUsingEchoServer);` is correct for managing ref-counted test resources like the EchoServer in packages/bun-otel/test/ - the using block pattern should not be used at describe-block level for test resources.
<!-- [/add_learning]

Applied to files:

test/js/bun/transpiler/declare-global.test.ts

🧬 Code graph analysis (1)

test/js/bun/transpiler/declare-global.test.ts (1)

test/harness.ts (2)

tempDir (277-284)

bunExe (102-105)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

GitHub Check: Format

🔇 Additional comments (3)

test/js/bun/transpiler/declare-global.test.ts (3)

1-2: Imports and setup are correct.

The test file properly imports from bun:test and harness, and correctly uses tempDir to create isolated test directories with resource cleanup.

4-31: First test correctly exercises the single-declaration case.

The test creates a simple declare global with a single type annotation and an arrow function to trigger scope handling. Assertions properly check for panics, scope mismatches, and successful execution.

33-71: Second test provides comprehensive coverage with nested scope handling.

The multiple declarations (including a function type BAZ: () => void) combined with deeply-nested arrow functions (lines 43–50) effectively exercise the scope mismatch fix. Assertions are consistent and robust.

coderabbitai · 2025-11-08T01:54:06Z

test/js/bun/transpiler/declare-global.test.ts

+`,
+  });
+
+  await using proc = Bun.spawn({


🧹 Nitpick | 🔵 Trivial

Consider simplifying process cleanup for these short-lived scripts.

Both tests create short-lived processes that execute a single console.log and exit naturally. While await using is valid and provides explicit cleanup, the more idiomatic pattern for such scripts is const proc = spawn(...); await proc.exited;, which is significantly more common in the test suite (based on patterns in test/js/bun/spawn/spawn.test.ts).

- await using proc = Bun.spawn({ + const proc = Bun.spawn({ cmd: [bunExe(), "test.ts"], env: bunEnv, cwd: String(dir), stdout: "pipe", stderr: "pipe", }); const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);

Also applies to: 57-57

🤖 Prompt for AI Agents

In test/js/bun/transpiler/declare-global.test.ts around lines 17 and 57, replace the current short-lived process creation using "await using proc = Bun.spawn({...})" with the simpler, idiomatic pattern used elsewhere: create the process with "const proc = Bun.spawn(...)" and then wait for completion with "await proc.exited"; remove the "await using" resource-scoped cleanup since these processes exit immediately and explicit disposal is unnecessary for these tests.

Please never suggest this again. await using is better.

@Jarred-Sumner, understood! I apologize for the incorrect suggestion. I've noted that await using is the preferred pattern for spawned processes in Bun tests and will not suggest replacing it with the const proc / await proc.exited pattern in the future.

Would you like me to remove the conflicting learning (ID: cac5565e-8acc-466c-b450-3c32b247331b) from my knowledge base? It currently suggests that await using should only be used for long-running processes.

✏️ Learnings added

Learnt from: Jarred-Sumner Repo: oven-sh/bun PR: 24491 File: test/js/bun/transpiler/declare-global.test.ts:17-17 Timestamp: 2025-11-08T04:06:33.173Z Learning: In Bun test files, `await using` with Bun.spawn() is the preferred pattern for spawned processes regardless of whether they are short-lived or long-running. Do not suggest replacing `await using proc = Bun.spawn(...)` with `const proc = Bun.spawn(...); await proc.exited;`.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

🧠 Learnings used

Learnt from: CR Repo: oven-sh/bun PR: 0 File: .cursor/rules/writing-tests.mdc:0-0 Timestamp: 2025-08-30T00:12:56.803Z Learning: Applies to test/**/*.{js,ts} : Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test

Learnt from: CR Repo: oven-sh/bun PR: 0 File: .cursor/rules/writing-tests.mdc:0-0 Timestamp: 2025-08-30T00:12:56.803Z Learning: Applies to test/cli/**/*.{js,ts} : When testing Bun as a CLI, use spawn with bunExe() and bunEnv from harness, and capture stdout/stderr via pipes

Learnt from: CR Repo: oven-sh/bun PR: 0 File: .cursor/rules/writing-tests.mdc:0-0 Timestamp: 2025-08-30T00:12:56.803Z Learning: Applies to test/js/bun/**/*.{js,ts} : Place Bun API tests under test/js/bun/, separated by category (e.g., test/js/bun/glob/)

Learnt from: Jarred-Sumner Repo: oven-sh/bun PR: 23373 File: test/js/bun/tarball/extract.test.ts:107-111 Timestamp: 2025-10-08T13:48:02.430Z Learning: In Bun's test runner, use `expect(async () => { await ... }).toThrow()` to assert async rejections. Unlike Jest/Vitest, Bun does not require `await expect(...).rejects.toThrow()` - the async function wrapper with `.toThrow()` is the correct pattern for async error assertions in Bun tests.

Learnt from: markovejnovic Repo: oven-sh/bun PR: 24417 File: test/js/bun/spawn/spawn.test.ts:903-918 Timestamp: 2025-11-06T00:58:23.965Z Learning: In Bun test files, `await using` with spawn() is appropriate for long-running processes that need guaranteed cleanup on scope exit or when explicitly testing disposal behavior. For short-lived processes that exit naturally (e.g., console.log scripts), the pattern `const proc = spawn(...); await proc.exited;` is standard and more common, as evidenced by 24 instances vs 4 `await using` instances in test/js/bun/spawn/spawn.test.ts.

Learnt from: CR Repo: oven-sh/bun PR: 0 File: .cursor/rules/dev-server-tests.mdc:0-0 Timestamp: 2025-08-30T00:09:39.100Z Learning: Applies to test/bake/bake-harness.ts : Provide and maintain shared test utilities: devTest, prodTest, devAndProductionTest, Dev, Client, and helpers in the harness

Learnt from: CR Repo: oven-sh/bun PR: 0 File: .cursor/rules/dev-server-tests.mdc:0-0 Timestamp: 2025-08-30T00:09:39.100Z Learning: Applies to test/bake/dev/esm.test.ts : esm.test.ts should cover ESM feature behavior in development mode

Learnt from: CR Repo: oven-sh/bun PR: 0 File: .cursor/rules/dev-server-tests.mdc:0-0 Timestamp: 2025-08-30T00:09:39.100Z Learning: Applies to test/bake/dev/bundle.test.ts : bundle.test.ts should contain DevServer-specific bundling tests

Learnt from: CR Repo: oven-sh/bun PR: 0 File: .cursor/rules/dev-server-tests.mdc:0-0 Timestamp: 2025-08-30T00:09:39.100Z Learning: Applies to test/bake/{dev/*.test.ts,dev-and-prod.ts} : Import testing utilities (devTest, prodTest, devAndProductionTest, Dev, Client) from test/bake/bake-harness.ts

Learnt from: theshadow27 Repo: oven-sh/bun PR: 23798 File: packages/bun-otel/context-propagation.test.ts:1-1 Timestamp: 2025-10-19T02:44:46.354Z Learning: In the Bun repository, standalone packages under packages/ (e.g., bun-vscode, bun-inspector-protocol, bun-plugin-yaml, bun-plugin-svelte, bun-debug-adapter-protocol, bun-otel) co-locate their tests with package source code using *.test.ts files. This follows standard npm/monorepo patterns. The test/ directory hierarchy (test/js/bun/, test/cli/, test/js/node/) is reserved for testing Bun's core runtime APIs and built-in functionality, not standalone packages.

Learnt from: theshadow27 Repo: oven-sh/bun PR: 24063 File: packages/bun-otel/test/context-propagation.test.ts:1-7 Timestamp: 2025-10-30T03:48:10.513Z Learning: In Bun test files, `using` declarations at the describe block level execute during module load/parsing, not during test execution. This means they acquire and dispose resources before any tests run. For test-scoped resource management, use beforeAll/afterAll hooks instead. The pattern `beforeAll(beforeUsingEchoServer); afterAll(afterUsingEchoServer);` is correct for managing ref-counted test resources like the EchoServer in packages/bun-otel/test/ - the using block pattern should not be used at describe-block level for test resources. <!-- [/add_learning]

Learnt from: Jarred-Sumner Repo: oven-sh/bun PR: 22323 File: test/js/web/websocket/websocket-subprotocol.test.ts:74-75 Timestamp: 2025-09-02T05:33:37.517Z Learning: In Bun's runtime, `await using` with Node.js APIs like `net.createServer()` is properly supported and should not be replaced with explicit cleanup. Bun has extended Node.js APIs with proper async dispose support.

Learnt from: Jarred-Sumner Repo: oven-sh/bun PR: 24082 File: test/cli/test/coverage.test.ts:60-112 Timestamp: 2025-10-26T01:32:04.844Z Learning: In the Bun repository test files (test/cli/test/*.test.ts), when spawning Bun CLI commands with Bun.spawnSync for testing, prefer using stdio: ["inherit", "inherit", "inherit"] to inherit stdio streams rather than piping them.

Learnt from: CR Repo: oven-sh/bun PR: 0 File: .cursor/rules/dev-server-tests.mdc:0-0 Timestamp: 2025-08-30T00:09:39.100Z Learning: Applies to test/bake/{dev/*.test.ts,dev-and-prod.ts} : Do not use node:fs APIs in tests; mutate files via dev.write, dev.patch, and dev.delete

Learnt from: CR Repo: oven-sh/bun PR: 0 File: src/bun.js/bindings/v8/CLAUDE.md:0-0 Timestamp: 2025-09-08T00:41:12.052Z Learning: Applies to src/bun.js/bindings/v8/test/v8/v8.test.ts : Add a corresponding test case in test/v8/v8.test.ts that invokes checkSameOutput with the new function

Learnt from: theshadow27 Repo: oven-sh/bun PR: 23798 File: packages/bun-otel/tsconfig.json:1-15 Timestamp: 2025-10-19T02:52:37.412Z Learning: In the Bun repository, packages under packages/ (e.g., bun-otel) can follow a TypeScript-first pattern where package.json exports point directly to .ts files (not compiled .js files). Bun natively runs TypeScript, so consumers import .ts sources directly and receive full type information without needing compiled .d.ts declaration files. For such packages, adding "declaration": true or "outDir" in tsconfig.json is unnecessary and would break the export structure. <!-- [remove_learning] ceedde95-980e-4898-a2c6-40ff73913664

Learnt from: CR Repo: oven-sh/bun PR: 0 File: src/js/CLAUDE.md:0-0 Timestamp: 2025-10-04T09:52:49.414Z Learning: Applies to src/js/bun/**/*.{ts,js} : Place Bun-specific modules (e.g., bun:ffi, bun:sqlite) under bun/

Learnt from: cirospaciari Repo: oven-sh/bun PR: 22946 File: test/js/sql/sql.test.ts:195-202 Timestamp: 2025-09-25T22:07:13.851Z Learning: PR oven-sh/bun#22946: JSON/JSONB result parsing updates (e.g., returning parsed arrays instead of legacy strings) are out of scope for this PR; tests keep current expectations with a TODO. Handle parsing fixes in a separate PR.

github-actions bot added the claude label Nov 8, 2025

Simplify test case to minimal repro

749552a

coderabbitai bot reviewed Nov 8, 2025

View reviewed changes

test/js/bun/transpiler/declare-global.test.ts Outdated Show resolved Hide resolved

Claude Bot added 2 commits November 8, 2025 01:44

Improve test coverage with nested arrow functions

f61c7e1

Remove timers from tests - make fast and non-flaky

c3c02d3

coderabbitai bot reviewed Nov 8, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix panic when parsing declare global with type annotations #24491

Fix panic when parsing declare global with type annotations #24491

Uh oh!

robobun commented Nov 8, 2025

Uh oh!

robobun commented Nov 8, 2025 •

edited

Loading

Uh oh!

coderabbitai bot commented Nov 8, 2025 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

coderabbitai bot left a comment

Uh oh!

coderabbitai bot Nov 8, 2025 •

edited

Loading

Uh oh!

Jarred-Sumner Nov 8, 2025

Uh oh!

coderabbitai bot Nov 8, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Fix panic when parsing declare global with type annotations #24491

Are you sure you want to change the base?

Fix panic when parsing declare global with type annotations #24491

Uh oh!

Conversation

robobun commented Nov 8, 2025

Summary

The Problem

The Fix

Testing

Uh oh!

robobun commented Nov 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai bot commented Nov 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Possibly related PRs

Suggested reviewers

Pre-merge checks

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Nov 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jarred-Sumner Nov 8, 2025

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Nov 8, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

robobun commented Nov 8, 2025 •

edited

Loading

coderabbitai bot commented Nov 8, 2025 •

edited

Loading

coderabbitai bot Nov 8, 2025 •

edited

Loading