fix(testing): use surgical text replacement in Jest matcher alias migration#34350
Merged
jaysoo merged 2 commits intonrwl:masterfrom Feb 26, 2026
Merged
Conversation
|
| Name | Link |
|---|---|
| 🔨 Latest commit | 4a65b65 |
✅ Deploy Preview for nx-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
baer
force-pushed
the
fix/jest-migration-surgical-text-replacement
branch
from
38852f2 to
881b479
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Contributor
|
View your CI Pipeline Execution ↗ for commit 4a65b65
☁️ Nx Cloud last updated this comment at |
jaysoo
requested changes
Feb 26, 2026
packages/jest/src/migrations/update-21-3-0/replace-removed-matcher-aliases.spec.ts
Outdated
Show resolved
Hide resolved
…ration
The `replace-removed-matcher-aliases` migration was corrupting TypeScript
files by using `tsquery.replace()`, which reprints the entire AST through
TypeScript's Printer. This caused several issues:
1. **Syntax corruption**: The Printer would mangle complex patterns like
destructuring (`{ result }` → `{result}:`), arrow functions (missing
opening braces), and nested callbacks (collapsed blocks).
2. **Unnecessary reformatting**: Every test file was written back to disk
even if no matchers were replaced, causing `formatFiles()` to reformat
files that didn't need changes.
This fix replaces the AST-reprinting approach with surgical text replacement:
- Use `tsquery.query()` to find matching AST nodes
- Collect text positions (start/end) for each node to replace
- Apply replacements in reverse order using string slicing
- Only write files that actually changed
This pattern is already used successfully in other Nx migrations (e.g.,
`rename-cy-exec-code-property.ts` in the Cypress package).
Additional improvements:
- Single AST parse with regex selector vs. 11 separate passes
- Quick string check skips parsing files without deprecated matchers
- New regression test covers complex patterns that triggered corruption
Fixes nrwl#32062
jaysoo
force-pushed
the
fix/jest-migration-surgical-text-replacement
branch
from
881b479 to
4a65b65
Compare
jaysoo
approved these changes
Feb 26, 2026
FrozenPandaz
pushed a commit
that referenced
this pull request
Feb 26, 2026
…ration (#34350) ## Current Behavior The `replace-removed-matcher-aliases` migration uses `tsquery.replace()` which reprints the entire AST through TypeScript's Printer. This causes two problems: 1. **Syntax corruption**: Valid TypeScript files are mangled: - Destructuring patterns: `{ result }` becomes `{result}:` - Arrow functions: missing opening braces - Nested callbacks: collapsed/merged code blocks 2. **Unnecessary file changes**: Every test file is written back to disk even when no matchers are replaced. This triggers `formatFiles()` to reformat unchanged files, creating large whitespace-only diffs. In large codebases, this can result in hundreds or thousands of files being modified unnecessarily, making the migration PR difficult to review. **Why I care a Lot** I was running this on a multi-million-LOC monorepo and ran into two issues: * I got ~10k modified files with whitespace-only changes from the removal of newlines. These changes couldn't be fixed with Prettier because it didn't care about the number of newlines, so the diff was unmergeable. * I got ~8 files with malformed Typescript, causing commit hooks, CI, etc. to fail without manual intervention. ## Expected Behavior The migration should: 1. Only replace the deprecated matcher names (e.g., `toBeCalled` → `toHaveBeenCalled`) 2. Preserve all surrounding code exactly as written 5. Only touch files that actually contain deprecated matchers ## Solution Replace AST-reprinting with surgical text replacement: - Use `tsquery.query()` to find matching AST nodes - Collect text positions (start/end) for each node to replace - Apply replacements in reverse order using string slicing - Only write files that actually changed This pattern is already used successfully in other Nx migrations (e.g., `rename-cy-exec-code-property.ts` in the Cypress package). **Additional improvements:** - Single AST parse with regex selector vs. 11 separate passes - Quick string check skips parsing files without deprecated matchers - New regression test covers complex patterns that triggered corruption ## Related Issue(s) Fixes #32062 --------- Co-authored-by: Jack Hsu <jack.hsu@gmail.com> (cherry picked from commit b8b6ed8)
FrozenPandaz
pushed a commit
that referenced
this pull request
Feb 26, 2026
…ration (#34350) ## Current Behavior The `replace-removed-matcher-aliases` migration uses `tsquery.replace()` which reprints the entire AST through TypeScript's Printer. This causes two problems: 1. **Syntax corruption**: Valid TypeScript files are mangled: - Destructuring patterns: `{ result }` becomes `{result}:` - Arrow functions: missing opening braces - Nested callbacks: collapsed/merged code blocks 2. **Unnecessary file changes**: Every test file is written back to disk even when no matchers are replaced. This triggers `formatFiles()` to reformat unchanged files, creating large whitespace-only diffs. In large codebases, this can result in hundreds or thousands of files being modified unnecessarily, making the migration PR difficult to review. **Why I care a Lot** I was running this on a multi-million-LOC monorepo and ran into two issues: * I got ~10k modified files with whitespace-only changes from the removal of newlines. These changes couldn't be fixed with Prettier because it didn't care about the number of newlines, so the diff was unmergeable. * I got ~8 files with malformed Typescript, causing commit hooks, CI, etc. to fail without manual intervention. ## Expected Behavior The migration should: 1. Only replace the deprecated matcher names (e.g., `toBeCalled` → `toHaveBeenCalled`) 2. Preserve all surrounding code exactly as written 5. Only touch files that actually contain deprecated matchers ## Solution Replace AST-reprinting with surgical text replacement: - Use `tsquery.query()` to find matching AST nodes - Collect text positions (start/end) for each node to replace - Apply replacements in reverse order using string slicing - Only write files that actually changed This pattern is already used successfully in other Nx migrations (e.g., `rename-cy-exec-code-property.ts` in the Cypress package). **Additional improvements:** - Single AST parse with regex selector vs. 11 separate passes - Quick string check skips parsing files without deprecated matchers - New regression test covers complex patterns that triggered corruption ## Related Issue(s) Fixes #32062 --------- Co-authored-by: Jack Hsu <jack.hsu@gmail.com> (cherry picked from commit b8b6ed8)
Contributor
|
This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request. |
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
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.
Current Behavior
The
replace-removed-matcher-aliasesmigration usestsquery.replace()which reprints the entire AST through TypeScript's Printer. This causes two problems:Syntax corruption: Valid TypeScript files are mangled:
{ result }becomes{result}:Unnecessary file changes: Every test file is written back to disk even when no matchers are replaced. This triggers
formatFiles()to reformat unchanged files, creating large whitespace-only diffs. In large codebases, this can result in hundreds or thousands of files being modified unnecessarily, making the migration PR difficult to review.Why I care a Lot
I was running this on a multi-million-LOC monorepo and ran into two issues:
Expected Behavior
The migration should:
toBeCalled→toHaveBeenCalled)Solution
Replace AST-reprinting with surgical text replacement:
tsquery.query()to find matching AST nodesThis pattern is already used successfully in other Nx migrations (e.g.,
rename-cy-exec-code-property.tsin the Cypress package).Additional improvements:
Related Issue(s)
Fixes #32062