fix: buffer ReplacementStream chunks before applying replacements#1756
Open
ZLeventer wants to merge 1 commit intoforcedotcom:mainfrom
Open
fix: buffer ReplacementStream chunks before applying replacements#1756ZLeventer wants to merge 1 commit intoforcedotcom:mainfrom
ZLeventer wants to merge 1 commit intoforcedotcom:mainfrom
Conversation
ReplacementStream ran the replacement regex against each chunk independently, so any token that was split across a chunk boundary (e.g. `#SOME` + `_REPLACEMENT#`) would fail to match and survive into the converted output unmodified. Buffer all chunks and run the replacement once in `_flush`. Metadata files are bounded in size so the memory cost is negligible compared to the correctness win, and the warning bookkeeping for `singleFile` replacements collapses naturally to a single pass. Adds a regression test that splits a token at its midpoint across two chunks via `Readable.from` and asserts the token is replaced and no "not found" warning is emitted. Refs forcedotcom/cli#3461
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
1 participant
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.
What
ReplacementStream._transformran the replacement regex against each chunk independently. Any token that was split across a chunk boundary (e.g.#SOME+_REPLACEMENT#) failed to match and survived into the converted output unmodified — and because the warning bookkeeping was per-chunk, thesingleFile"not found" warning could also misfire.This change buffers all chunks in
_transformand runsreplacementIterationsonce in_flush. Metadata files are bounded in size, so the memory cost is negligible compared to the correctness win. The per-chunk warning bookkeeping collapses to a single pass.Why
Refs forcedotcom/cli#3461 — users report tokens silently surviving conversion. Reproducible whenever Node's stream layer splits a file mid-token (large XML files, slow disks, mixed encodings).
Test plan
test/convert/replacements.test.tsthat splits#SOME_REPLACEMENT#at its midpoint across two chunks viaReadable.from([chunk1, chunk2])and asserts both that the token is replaced and that no "not found" warning is emitted.ReplacementStreamtests still pass, including the warning-emission tests forsingleFilereplacements.yarn compile,yarn lint,yarn test:onlyclean locally on Node 22.