Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 11 additions & 0 deletions src/ast/parseStmt.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1185,6 +1185,17 @@ pub fn ParseStmt(
switch (expr.data) {
.e_identifier => |ident| {
if (p.lexer.token == .t_colon and !opts.hasDecorators()) {
// In TypeScript declare contexts, "identifier: Type" is a type annotation, not a label
if (comptime is_typescript_enabled) {
if (opts.is_typescript_declare) {
// Skip the colon and type annotation
try p.lexer.next();
try p.skipTypeScriptType(.lowest);
try p.lexer.expectOrInsertSemicolon();
return p.s(S.TypeScript{}, loc);
}
}

_ = try p.pushScopeForParsePass(.label, loc);
defer p.popScope();

Expand Down
71 changes: 71 additions & 0 deletions test/js/bun/transpiler/declare-global.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
import { expect, test } from "bun:test";
import { bunEnv, bunExe, tempDir } from "harness";

test("declare global with type annotation should not crash", async () => {
using dir = tempDir("declare-global-test", {
"test.ts": `
declare global {
A: 'a';
}

() => {};

console.log("SUCCESS");
`,
});

await using proc = Bun.spawn({
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 8, 2025

Choose a reason for hiding this comment

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

🧹 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.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please never suggest this again. await using is better.

Copy link
Contributor

Choose a reason for hiding this comment

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

@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.

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]);

expect(stderr).not.toContain("panic");
expect(stderr).not.toContain("Scope mismatch");
expect(stdout).toContain("SUCCESS");
expect(exitCode).toBe(0);
});

test("declare global with multiple type annotations and nested arrow functions", async () => {
using dir = tempDir("declare-global-multi", {
"test.ts": `
declare global {
TIMER: NodeJS.Timeout;
FOO: string;
BAR: number;
BAZ: () => void;
}

// Test nested arrow functions to ensure scope handling is correct
const fn = () => {
const nested = () => {
const deeplyNested = () => "nested";
return deeplyNested();
};
return nested();
};

console.log(fn());
console.log("SUCCESS");
`,
});

await using 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]);

expect(stderr).not.toContain("panic");
expect(stderr).not.toContain("Scope mismatch");
expect(stdout).toContain("SUCCESS");
expect(exitCode).toBe(0);
});