[ronistrom]

Closes #596

What was wrong

The writer emitted the fit-to-page counts only when fitToPage: true was also set. Without it, they were dropped without a warning:

for (const pageSetup of [
  { fitToWidth: 1, fitToHeight: 0 },
  { fitToHeight: 2 },
  { fitToPage: false, fitToWidth: 1, fitToHeight: 0 },
]) {
  await writeXlsx({ sheets: [{ name: "Sheet1", rows: [["x"]], pageSetup }] })
}

<!-- { fitToWidth: 1, fitToHeight: 0 } — before: no <sheetPr>, no <pageSetup> -->
<!-- after -->
<sheetPr><pageSetUpPr fitToPage="1"/></sheetPr>
<pageSetup fitToWidth="1" fitToHeight="0"/>

<!-- { fitToHeight: 2 } — before: no <sheetPr>, no <pageSetup> -->
<!-- after -->
<sheetPr><pageSetUpPr fitToPage="1"/></sheetPr>
<pageSetup fitToHeight="2"/>

<!-- { fitToPage: false, fitToWidth: 1, fitToHeight: 0 } — before and after: neither element -->

So the sheet printed at 100%. Reading the file back gave a pageSetup with only the default margins. With fitToPage: true added, the output was correct and it still is.

The reader already treats either count as turning fit mode on. The writer also already lets firstPageNumber imply useFirstPageNumber. Fit to page was the only setting where the writer required the flag.

Why the tests didn't catch it

Every fit-to-page test in print-settings.test.ts, real-world.test.ts, xlsx-write-read-parity.test.ts and the page-setup-attributes.test.ts register set fitToPage: true together with the counts. No test wrote the counts on their own. The reader test for a lone fitToHeight (coverage-xlsx-worksheet.test.ts) parses hand-written XML, so it never touches the writer.

What landed

• A small isFitToPage(ps) helper in worksheet-writer.ts: fitToPage ?? (fitToWidth !== undefined || fitToHeight !== undefined). The <pageSetUpPr> check in writeWorksheetXml and the count attributes in serializePageSetup both use it. An explicit fitToPage: false still wins and writes neither the flag nor the counts, because the counts only take effect in fit mode.
• Doc comments on PageSetup.fitToPage, fitToWidth and fitToHeight describe the implication and say that 0 means "as many pages as needed".
• writeXlsxStream / writeXlsxStreamSheets have no pageSetup support, so nothing changes there. docs/PARITY.md already says every CT_PageSetup attribute round-trips, so this brings the code in line with it and the page needs no edit.

Tests: three of the four new tests in print-settings.test.ts fail against main: counts only, fitToHeight only, and the round trip through readXlsx. The fourth checks that fitToPage: false with counts writes nothing, which already worked and is there to stop the implication from overriding an explicit false. bun run test passes on Node 24.22.0, with 240 files and 10,702 tests.

Checked in Excel

In Excel for Mac, with a 20-column sheet written by main and by this branch. Both files open without a repair prompt.

• After: Excel's page setup reports fit to 1 page wide with height automatic. The Page Layout tab shows Width "1 page" and Height "Automatic".
• Before: scaling is 100%.

The XML matches what the existing fitToPage: true path already writes, so this change only makes that path reachable without the flag.

Not verified

• Print preview. I checked the page setup, not that all 20 columns actually land on one page width.
• LibreOffice and Windows Excel. Not opened.

Summary by CodeRabbit

• Documentation

  • Clarified how page fitting and page-count settings interact.

• Bug Fixes

  • Page setup now automatically enables fit-to-page mode when fit-to-width or fit-to-height is configured.
  • Explicitly disabling fit-to-page prevents related page setup properties from being emitted.
  • Page-count settings are preserved when reading and writing worksheet files.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 7277c7a6-cc9d-4fac-aa4d-1c49ef2b5074

📥 Commits

Reviewing files that changed from the base of the PR and between c991ad0 and 1adfd6d.

📒 Files selected for processing: 3

• src/_types.ts
• src/xlsx/worksheet-writer.ts
• test/print-settings.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

📝 Walkthrough

The worksheet writer now enables fit-to-page mode when fitToWidth or fitToHeight is set, unless fitToPage is explicitly false. Documentation and tests describe and verify the behavior.

Changes: Page setup fitting

Layer / File(s)	Summary
Infer and serialize fit-to-page settings src/_types.ts, src/xlsx/worksheet-writer.ts, test/print-settings.test.ts	The documentation describes how fit counts interact with fit-to-page settings. The writer infers the mode from either defined count and uses it when writing worksheet properties and page setup attributes. Tests cover inferred mode, explicit disabling, and round-trip counts.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: productdevbook

Merge Risk: ⚪ Minimal · up to 1adfd

Page-count settings now enable fitting unless explicitly disabled. No actionable issue identified here prevents merging after normal checks.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: fitToWidth and fitToHeight now imply fitToPage in XLSX output.
Linked Issues check		Issue #596 is an active direct target. The supplied PR evidence and reviewed files show that isFitToPage enables fit mode when fitToWidth or fitToHeight is defined, while fitToPage: false rema…
Out of Scope Changes check		The changes stay within issue #596. They modify worksheet page-setup XML, add focused PageSetup documentation, and add focused writer and round-trip tests. These changes directly support the request…

Full details: Linked Issues check

Explanation

Issue #596 is an active direct target. The supplied PR evidence and reviewed files show that isFitToPage enables fit mode when fitToWidth or fitToHeight is defined, while fitToPage: false remains authoritative. The writer uses this behavior for pageSetUpPr and page-setup attributes. Tests cover width and height counts, round-trip behavior, and explicit false. The PageSetup comments document the API behavior and zero-count meaning.

Full details: Out of Scope Changes check

Explanation

The changes stay within issue #596. They modify worksheet page-setup XML, add focused PageSetup documentation, and add focused writer and round-trip tests. These changes directly support the requested behavior. No unrelated implementation or test changes are identified in the supplied scope evidence.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[ronistrom]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.