fix: YAML.stringify spacing for multiline values #23510

robobun · 2025-10-11T23:38:43Z

What does this PR do?

Fixes two formatting issues in Bun.YAML.stringify() to match standard YAML format (as used by js-yaml):

Removed extra space after colon for multiline values
- Before: key: \n value
- After: key:\n value
Keep empty arrays/objects inline
- Before: arr: \n []
- After: arr: []

The fix modifies YAMLObject.zig to:

Unwrap values before checking if they need a newline
Check if arrays/objects are empty before deciding on formatting
Only add space after colon (: ) for primitives and empty collections
Use colon without space (:\n) for non-empty arrays and objects

How did you verify your code works?

Created comprehensive regression test (test/regression/issue/23501-yaml-spacing.test.ts) covering:
- Empty arrays and objects staying inline
- Non-empty arrays and objects with proper multiline formatting
- Nested structures preserving correct formatting
- Primitive values with proper spacing
Updated existing YAML tests (test/js/bun/yaml/yaml.test.ts) to match the correct standard YAML format
Verified output matches js-yaml behavior by comparing side-by-side with the js-yaml package

All tests pass ✅ (152 pass, 2 pre-existing timeout failures unrelated to this change)

Fixes #23501

🤖 Generated with Claude Code

robobun · 2025-10-11T23:38:55Z

^{Updated 5:41 PM PT - Oct 11th, 2025}

❌ Your commit 8692c363 has 1 failures in Build #28919 (All Failures):

test/cli/run/require-cache.test.ts - 1 failing on 🍎 14 aarch64

🧪 To try this PR locally:

bunx bun-pr 23510

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

bun-23510 --bun

coderabbitai · 2025-10-11T23:42:43Z

Walkthrough

Refines YAML serialization formatting: unwraps property values once to determine whether a newline is needed, prints a colon followed by newline for complex/non-empty collections and a colon+space for inline primitives/empty collections, and updates tests/snapshots to reflect the new no-space-before-newline style.

Changes

Cohort / File(s)	Summary
YAML formatting logic `src/bun.js/api/YAMLObject.zig`	Unwraps property values once and computes `needs_newline` from the unwrapped value; replaces inline calls to newline checks with the computed result; adjusts printing to use `":" + "\n"` for newline cases and `": "` for inline cases; preserves inline empty arrays/objects.
New regression tests `test/regression/issue/23501-yaml-spacing.test.ts`	Adds three tests verifying Bun.YAML.stringify: no trailing space before a multiline value, empty arrays/objects render inline, and non-empty/nested structures render with colon+newline.
Updated YAML tests & snapshots `test/js/bun/yaml/yaml.test.ts`	Updated numerous snapshots and test literals to remove the space after colons when the value starts on the next line (e.g., `"key: \n ..." -> "key:\n ..."`) to match the new formatting.

Possibly related PRs

Implement Bun.YAML.stringify #22183 — Modifies YAML stringify logic in src/bun.js/api/YAMLObject.zig, adjusting property-value newline/colon formatting; directly related to these changes.

Suggested reviewers

alii
dylan-conway

Pre-merge checks

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description Check	⚠️ Warning	The PR description does not follow the repository template since it uses custom headings (“## Summary”, “## Description”, “## Implementation”, “## Test Plan”) instead of the required “### What does this PR do?” and “### How did you verify your code works?” sections.	Please update the description to include the required “### What does this PR do?” section summarizing the changes and “### How did you verify your code works?” section describing verification steps to match the repository template.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The title accurately summarizes the key change to Bun.YAML.stringify by describing the spacing fix for values rendered on multiple lines, and it is concise, clear, and focused on the primary change without extraneous detail.
Linked Issues Check	✅ Passed	The implementation in YAMLObject.zig now removes the space before newlines for multiline values and keeps empty arrays and objects inline as specified, and the added regression tests confirm these behaviors, fully satisfying the requirements of issue #23501.
Out of Scope Changes Check	✅ Passed	All modifications are confined to YAMLObject.zig and related tests for YAML.stringify spacing, with no unrelated code or additional features introduced beyond the scope of the YAML spacing issue.

📜 Recent 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 2c11778 and 8692c36.

📒 Files selected for processing (3)

src/bun.js/api/YAMLObject.zig (2 hunks)
test/js/bun/yaml/yaml.test.ts (13 hunks)
test/regression/issue/23501-yaml-spacing.test.ts (1 hunks)

🧰 Additional context used

📓 Path-based instructions (13)

test/**

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

Place all tests under the test/ directory

Files:

test/regression/issue/23501-yaml-spacing.test.ts
test/js/bun/yaml/yaml.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/regression/issue/23501-yaml-spacing.test.ts
test/js/bun/yaml/yaml.test.ts

test/**/*.test.ts

📄 CodeRabbit inference engine (test/CLAUDE.md)

test/**/*.test.ts: Name test files *.test.ts and use bun:test
Do not write flaky tests: never wait for arbitrary time; wait for conditions instead
Never hardcode port numbers in tests; use port: 0 to get a random port
When spawning Bun in tests, use bunExe() and bunEnv from harness
Prefer async/await in tests; for a single callback, use Promise.withResolvers()
Do not set explicit test timeouts; rely on Bun’s built-in timeouts
Use tempDir/tempDirWithFiles from harness for temporary files and directories in tests
For large/repetitive strings in tests, prefer Buffer.alloc(count, fill).toString() over "A".repeat(count)
Import common test utilities from harness (e.g., bunExe, bunEnv, tempDirWithFiles, tmpdirSync, platform checks, GC helpers)
In error tests, assert non-zero exit codes for failing processes and use toThrow for synchronous errors
Use describe blocks for grouping, describe.each for parameterized tests, snapshots with toMatchSnapshot, and lifecycle hooks (beforeAll, beforeEach, afterEach); track resources for cleanup in afterEach
Use using/await using with Bun resources (e.g., Bun.listen/connect/spawn/serve) to ensure cleanup in tests

Files:

test/regression/issue/23501-yaml-spacing.test.ts
test/js/bun/yaml/yaml.test.ts

test/regression/issue/**

📄 CodeRabbit inference engine (test/CLAUDE.md)

Place regression tests under test/regression/issue/ and organize by issue number

Place regression tests under test/regression/issue/ (one per bug fix)

Files:

test/regression/issue/23501-yaml-spacing.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 roll your own random port
Prefer normalizeBunSnapshot for snapshotting test output instead of asserting raw strings
Do not write tests that assert absence of crashes (e.g., 'no panic' or 'no uncaught exception')
Use Bun’s Jest-compatible runner (import { test, expect } from "bun:test") for tests
Avoid shell commands like find or grep in tests; use Bun’s Glob and built-in tools instead
Prefer running tests via bun bd test and use provided harness utilities (bunEnv, bunExe, tempDir)
Use Bun.spawn with proper stdio handling and await proc.exited in process-spawning tests

Files:

test/regression/issue/23501-yaml-spacing.test.ts
test/js/bun/yaml/yaml.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/yaml/yaml.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/yaml/yaml.test.ts

test/js/**

📄 CodeRabbit inference engine (test/CLAUDE.md)

Organize unit tests for specific features under test/js/ by module

Files:

test/js/bun/yaml/yaml.test.ts

test/js/bun/**

📄 CodeRabbit inference engine (CLAUDE.md)

Place Bun-specific API tests under test/js/bun/

Files:

test/js/bun/yaml/yaml.test.ts

**/*.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/bun.js/api/YAMLObject.zig

src/{**/js_*.zig,bun.js/api/**/*.zig}

📄 CodeRabbit inference engine (.cursor/rules/registering-bun-modules.mdc)

Use bun.JSError!JSValue for proper error propagation in JS-exposed Zig functions

Files:

src/bun.js/api/YAMLObject.zig

src/bun.js/**/*.zig

📄 CodeRabbit inference engine (.cursor/rules/zig-javascriptcore-classes.mdc)

src/bun.js/**/*.zig: In Zig binding structs, expose generated bindings via pub const js = JSC.Codegen.JS and re-export toJS/fromJS/fromJSDirect
Constructors and prototype methods should return bun.JSError!JSC.JSValue to integrate Zig error handling with JS exceptions
Use parameter name globalObject (not ctx) and accept (*JSC.JSGlobalObject, *JSC.CallFrame) in binding methods/constructors
Implement getters as get(this, globalObject) returning JSC.JSValue and matching the .classes.ts interface
Provide deinit() for resource cleanup and finalize() that calls deinit(); use bun.destroy(this) or appropriate destroy pattern
Access JS call data via CallFrame (argument(i), argumentCount(), thisValue()) and throw errors with globalObject.throw(...)
For properties marked cache: true, use the generated Zig accessors (NameSetCached/GetCached) to work with GC-owned values
In finalize() for objects holding JS references, release them using .deref() before destroy

Files:

src/bun.js/api/YAMLObject.zig

src/**/*.zig

📄 CodeRabbit inference engine (CLAUDE.md)

In Zig code, manage memory carefully and use defer for cleanup of allocations/resources

src/**/*.zig: Use the # prefix to declare private fields in Zig structs (e.g., struct { #foo: u32 })
Prefer decl literals when initializing values in Zig (e.g., const decl: Decl = .{ .binding = 0, .value = 0 })
Place @import directives at the bottom of Zig files
Use @import("bun") instead of @import("root").bun

When adding debug logs in Zig, create a scoped logger and log via Bun APIs: const log = bun.Output.scoped(.${SCOPE}, .hidden); then log("...", .{})

Files:

src/bun.js/api/YAMLObject.zig

⏰ 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 (2)

src/bun.js/api/YAMLObject.zig (1)

481-525: Good call on precomputing newline needs

Lifting the unwrap plus the targeted empty-check keeps primitives inline while still forcing block style for real collections. Matches the spacing fixes without upsetting anchors. Nicely done.

test/regression/issue/23501-yaml-spacing.test.ts (1)

3-64: Regression coverage looks great

This trio nails the reported spacing regressions (inline empties, nested multiline, primitive spacing). Happy to see it locked in.

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

Fixed two issues with YAML.stringify output formatting: 1. Removed extra space after colon when value is on next line - Before: `key: \n value` - After: `key:\n value` 2. Keep empty arrays/objects inline instead of newline - Before: `arr: \n []` - After: `arr: []` The fix checks if a value is an empty array or object before deciding whether to put it on a newline. Empty collections stay inline with proper spacing, while non-empty collections go on the next line without a space after the colon. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>

github-actions bot added the claude label Oct 11, 2025

robobun force-pushed the claude/fix-yaml-stringify-spacing branch from 2c11778 to 8692c36 Compare October 11, 2025 23:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: YAML.stringify spacing for multiline values #23510

fix: YAML.stringify spacing for multiline values #23510

robobun commented Oct 11, 2025 •

edited

Loading

Uh oh!

robobun commented Oct 11, 2025 •

edited

Loading

Uh oh!

coderabbitai bot commented Oct 11, 2025 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

fix: YAML.stringify spacing for multiline values #23510

Are you sure you want to change the base?

fix: YAML.stringify spacing for multiline values #23510

Conversation

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

Uh oh!

What does this PR do?

How did you verify your code works?

Uh oh!

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

Uh oh!

Uh oh!

coderabbitai bot commented Oct 11, 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

robobun commented Oct 11, 2025 •

edited

Loading

robobun commented Oct 11, 2025 •

edited

Loading

coderabbitai bot commented Oct 11, 2025 •

edited

Loading