Skip to content

fix: skip empty matches at a chunk boundary inside removed content - #366

Merged
antfu merged 1 commit into
Rich-Harris:masterfrom
theRizwan:fix-replace-empty-match-removed-boundary
Oct 26, 2026
Merged

antfu merged 1 commit into
Rich-Harris:masterfrom
theRizwan:fix-replace-empty-match-removed-boundary

Conversation

@theRizwan

Copy link
Copy Markdown
Contributor

Since #359, an empty match inside removed content is skipped by replace and replaceAll. That only holds while no chunk boundary sits at the match: removing the same range in two calls gives the same output but a different result.

new MagicString('abcd').remove(1, 3).replaceAll('', '-').toString()            // '-a--d-'
new MagicString('abcd').remove(1, 2).remove(2, 3).replaceAll('', '-').toString() // '-a---d-'

new MagicString('abcd').remove(1, 3).replace(/(?=c)/, '-').toString()            // 'ad'
new MagicString('abcd').remove(1, 2).remove(2, 3).replace(/(?=c)/, '-').toString() // 'a-d'

Any split at the match does the same, including one from an earlier appendLeft(2, '').

_hasRemovedContent(start, end) walks the chunks from the one at start while chunk.start < end. For an empty range on a boundary, start === end === chunk.start, so the loop never runs and the match is reported as present. Inside a chunk the search finds the chunk containing start instead, whose start is before end, which is why the unsplit case works.

This handles that one case: an empty range at a chunk boundary is removed when the chunks on both sides of it are. That is the same rule the unsplit case already follows, and it keeps the existing behaviour at the edges of a removed range ([0, 1, 3, 4] in the #359 test) and at the ends of the string.

I checked it exhaustively against that rule: every string of length 1 to 6, no removal or every combination of one or two remove() calls, and every set of no-op splits, comparing the indices passed to a replaceAll('', fn) replacer and to a replaceAll(/x*/g, fn) replacer. That is 19,761 cases; 17,070 disagree with the rule on master and none do with this change. A random fuzz of the other methods (chunk-split invariance, clone(), self-consistency of lastChar/lastLine/isEmpty/hasChanged) gives identical results before and after.

Three tests are added, all failing on master. The suite goes from 454 to 457 and ESLint is clean; tsc reports the same 23 errors as master, none in the changed files.

An empty range on a chunk boundary overlaps neither chunk, so _hasRemovedContent never looked at them and reported the match as present. Removing a range in two calls, or any split inside it, made replace and replaceAll insert there although removing it in one call did not.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants