fix(test): preserve string-literal tabs in convertTabsToSpaces#1559
Merged
Conversation
✅ Deploy Preview for viteplus-preview canceled.
|
Member
Author
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 4eb4ed1. Configure here.
The build step `convertTabsToSpaces()` did a blanket `\t` → 2-space
rewrite over every dist .js file. Upstream `@vitest/snapshot` decides
multi-line inline snapshot indentation via `indent.includes("\t")`,
where the `"\t"` is a literal tab byte in the bundled source. The
blanket replace turned that into `indent.includes(" ")`, so every
2-space indent matched and the tab-appending branch always ran —
producing tab-indented snapshots in 2-space files.
Restrict the replacement to leading tabs only via `/^\t+/gm`.
Indentation is still normalized (downstream patches that rely on
space indent keep working), but tabs inside string and template
literals are now preserved.
Adds two regression tests:
- `packages/test/__tests__/build-artifacts.spec.ts` asserts the
bundled snapshot file keeps the literal tab inside
`indent.includes("\t")`.
- `packages/cli/snap-tests/test-inline-snapshot-indent/` reproduces
the issue's exact `vp test --update` flow on a 2-space indented
file and captures the rewritten content.
Closes #1553
The pre-check `if (!/^\t/m.test(content)) continue` in convertTabsToSpaces duplicates the work that `replace()` already performs, and the subsequent `if (converted !== content)` guard already short-circuits no-op writes. Drop the pre-check. The inline comment inside the #1553 regression test restated the same reasoning as the describe-level JSDoc — drop the inline copy.
3fbf672 to
8395fe0
Compare
Brooooooklyn
approved these changes
May 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
convertTabsToSpaces()inpackages/test/build.tsdid a blanket\t→ 2-space rewrite over every dist.jsfile. Upstream@vitest/snapshotdecides multi-line inline snapshot indentation withindent.includes("\t")where the"\t"is a literal tab byte in the bundled source. The blanket replace turned that intoindent.includes(" "), so any 2+ space indent matched and the tab-appending branch always ran — producing tab-indented snapshots inside 2-space-indented test files./^\t+/gm. Indentation is still normalized (downstream patches that rely on space indent keep working), but tabs inside string and template literals are now preserved.Root cause (byte-level)
Upstream
@vitest/snapshot/dist/index.js:369:The
<TAB>inside theincludes(...)string is a real 0x09 byte. The\tin the template literal is the 2-char escape sequence (\+t).Before the fix,
convertTabsToSpacesrewrote both the leading tab AND the literal tab byte inside the string to two spaces. The escape sequence was untouched. Result at runtime:indent.includes(" ")is true for any 2+ space indent${indent}\t→ a real tab byteAfter the fix only leading tabs are rewritten; the string-literal tab survives.
Test plan
packages/test/__tests__/build-artifacts.spec.tsasserts the bundled snapshot file preservesindent.includes("\t")with a literal tab byte (passes after fix, fails before).packages/cli/snap-tests/test-inline-snapshot-indent/reproduces the issue's exact reproduction steps (vp test run -uon a 2-space-indented file) and captures the rewritten file content. Verified that reverting the fix produces asnap.txtcontaining real tabs (<4 spaces><TAB>"alpha) while the fixed build produces 6-space indentation.packages/testunit tests pass.vp check --fixis clean.Closes #1553
Note
Medium Risk
Touches the test package build post-processing (
convertTabsToSpaces) which rewrites generated dist artifacts; a regex mistake could subtly change bundled code semantics beyond formatting, but the change is narrowly scoped and covered by new regression tests.Overview
Fixes a dist-build formatting step so it only converts leading tab indentation to spaces, instead of rewriting all tab bytes and accidentally altering string/template literal contents.
Adds regression coverage for inline snapshot indentation: a new build-artifact assertion ensures
@vitest/snapshotstill containsindent.includes("\t"), and a new CLI snap-test reproducesvp test run -uupdating an inline snapshot in a 2-space-indented file to verify the written snapshot stays space-indented (not tab-indented).Reviewed by Cursor Bugbot for commit 4eb4ed1. Configure here.