-
Couldn't load subscription status.
- Fork 3.5k
fix: inconsistent tabs used during library initialization
#24099
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.
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughAdded a function to remove a JSON property line by key, used when generating TypeScript library configs to strip JSX from tsconfig, and introduced tests to validate the new function and the "bun init --library" flow. Changes
Suggested reviewers
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
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.
📒 Files selected for processing (2)
src/cli/init_command.zig(4 hunks)test/cli/init/init.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)
**/*.zig: Declare the extern C symbol in Zig and export a Zig-friendly alias for use
Wrap the Bun____toJS extern in a Zig method that takes a JSGlobalObject and returns JSC.JSValue
Files:
src/cli/init_command.zig
src/**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)
When adding debug logs in Zig, create a scoped logger and log via Bun APIs:
const log = bun.Output.scoped(.${SCOPE}, .hidden);thenlog("...", .{})
src/**/*.zig: Use Zig private fields with the # prefix for encapsulation (e.g., struct { #foo: u32 })
Prefer Decl literals for initialization (e.g., const decl: Decl = .{ .binding = 0, .value = 0 };)
Place @import statements at the bottom of the file (formatter will handle ordering)
src/**/*.zig: In Zig code, manage memory carefully: use appropriate allocators and defer for cleanup
Cache JavaScriptCore class structures in ZigGlobalObject when adding new classes
Files:
src/cli/init_command.zig
test/**
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Place all tests under the test/ directory
Files:
test/cli/init/init.test.ts
test/cli/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
test/cli/**/*.{js,ts}: Place CLI command tests (e.g., bun install, bun init) under test/cli/
When testing Bun as a CLI, use spawn with bunExe() and bunEnv from harness, and capture stdout/stderr via pipes
Files:
test/cli/init/init.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/cli/init/init.test.ts
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}
📄 CodeRabbit inference engine (test/CLAUDE.md)
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Usebun:testfor files ending with*.test.{ts,js,jsx,tsx,mjs,cjs}
Prefer concurrent tests (test.concurrent/describe.concurrent) over sequential when feasible
Organize tests withdescribeblocks to group related tests
Use utilities likedescribe.each,toMatchSnapshot, and lifecycle hooks (beforeAll,beforeEach,afterEach) and track resources for cleanup
Files:
test/cli/init/init.test.ts
test/**/*.{ts,tsx,js,jsx,mjs,cjs}
📄 CodeRabbit inference engine (test/CLAUDE.md)
For large/repetitive strings, use
Buffer.alloc(count, fill).toString()instead of"A".repeat(count)
Files:
test/cli/init/init.test.ts
test/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
test/**/*.test.{ts,tsx}: Test files must live under test/ and end with .test.ts or .test.tsx
In tests, always use port: 0; do not hardcode ports or invent random port functions
Prefer snapshot assertions and use normalizeBunSnapshot for snapshot output in tests
Never write tests that assert absence of crashes (e.g., no "panic" or "uncaught exception") in output
Use tempDir from "harness" for temporary directories; do not use tmpdirSync or fs.mkdtempSync in tests
When spawning processes in tests, assert on stdout before asserting exitCode
Do not use setTimeout in tests; await conditions instead to avoid flakiness
Files:
test/cli/init/init.test.ts
test/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid shell commands in tests (e.g., find, grep); use Bun's Glob and built-in tools
Files:
test/cli/init/init.test.ts
🧠 Learnings (7)
📚 Learning: 2025-10-19T02:44:46.354Z
Learnt from: theshadow27
PR: oven-sh/bun#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/cli/init/init.test.ts
📚 Learning: 2025-10-12T02:22:34.373Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.373Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `bun:test` for files ending with `*.test.{ts,js,jsx,tsx,mjs,cjs}`
Applied to files:
test/cli/init/init.test.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#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/cli/init/init.test.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#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/cli/init/init.test.ts
📚 Learning: 2025-10-12T02:22:34.373Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.373Z
Learning: Applies to test/{test/**/*.test.{ts,js,jsx,tsx,mjs,cjs},test/js/node/test/{parallel,sequential}/*.js} : When spawning Bun in tests, use `bunExe()` and `bunEnv` from `harness`
Applied to files:
test/cli/init/init.test.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#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/cli/init/init.test.ts
📚 Learning: 2025-10-12T02:22:34.373Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.373Z
Learning: Applies to test/{test/**/*.test.{ts,js,jsx,tsx,mjs,cjs},test/js/node/test/{parallel,sequential}/*.js,test/**/*-fixture.ts} : Use `using`/`await using` for resource cleanup with Bun APIs (e.g., `Bun.spawn`, `Bun.listen`, `Bun.serve`)
Applied to files:
test/cli/init/init.test.ts
🧬 Code graph analysis (1)
test/cli/init/init.test.ts (1)
test/harness.ts (3)
tempDirWithFiles(259-266)bunExe(102-105)bunEnv(49-64)
🔇 Additional comments (4)
src/cli/init_command.zig (3)
856-868: TS library tsconfig filtering looks correctUsing the embedded tsconfig and writing a filtered copy for
.typescript_libraryavoids JSX in library templates without new assets. The file handling and messaging are consistent with the rest of InitCommand.
1291-1314: Imports-at-bottom and string aliases LGTMBottom-placed
@imports and addedstring/stringZaliases align with the repository Zig style. Forward references are fine at top-level.
1315-1341: Zig tests are solid; add one more case if you adopt whitespace-tolerant matchThe tests validate removal and no-op paths and free allocations. If you accept the whitespace-tolerant match, consider an extra test with
"jsx" : "react-jsx".test/cli/init/init.test.ts (1)
299-338: Pipe stdio in CLI tests for consistency and quieter CI logsTests under test/cli prefer capturing via pipes. Switch
stdout/stderrto"pipe"(you can keepstdinas a Blob) to reduce noise and match harness conventions.As per coding guidelines
- const { exited } = Bun.spawn({ + const { exited } = Bun.spawn({ cmd: [bunExe(), "init"], cwd: temp, // Simulate: arrow down twice to "Library", enter, "mylib" as package name, enter for default entry point stdin: new Blob(["\x1B[B\x1B[B\nmylib\n\n"]), - stdout: "inherit", - stderr: "inherit", + stdout: "pipe", + stderr: "pipe", env: bunEnv, });⛔ Skipped due to learnings
Learnt from: Jarred-Sumner PR: oven-sh/bun#24082 File: test/cli/test/coverage.test.ts:60-112 Timestamp: 2025-10-26T01:32:04.836Z 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 PR: oven-sh/bun#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 pipesLearnt from: CR PR: oven-sh/bun#0 File: test/CLAUDE.md:0-0 Timestamp: 2025-10-12T02:22:34.373Z Learning: Applies to test/{test/**/*.test.{ts,js,jsx,tsx,mjs,cjs},test/js/node/test/{parallel,sequential}/*.js} : When spawning Bun in tests, use `bunExe()` and `bunEnv` from `harness`Learnt from: CR PR: oven-sh/bun#0 File: .cursor/rules/writing-tests.mdc:0-0 Timestamp: 2025-08-30T00:12:56.803Z Learning: Applies to test/cli/**/*.{js,ts} : Place CLI command tests (e.g., bun install, bun init) under test/cli/Learnt from: CR PR: oven-sh/bun#0 File: CLAUDE.md:0-0 Timestamp: 2025-10-26T05:04:50.682Z Learning: Applies to test/**/*.test.{ts,tsx} : When spawning processes in tests, assert on stdout before asserting exitCodeLearnt from: CR PR: oven-sh/bun#0 File: CLAUDE.md:0-0 Timestamp: 2025-10-26T05:04:50.682Z Learning: Applies to test/**/*.{ts,tsx} : Avoid shell commands in tests (e.g., find, grep); use Bun's Glob and built-in toolsLearnt from: theshadow27 PR: oven-sh/bun#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: CR PR: oven-sh/bun#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
Closes #19319
What does this PR do?
Fixes an inconsistency when running bun init with the Library template:
package.jsonwas configured for a library whiletsconfig.jsonstill included a React JSX setting. This change removes the JSX property for the Library template without introducing a new asset file and aligns imports with project conventions.How did you verify your code works?
Integration coverage for “bun init —library” behaviour and tsconfig checks
Screenshots
