Repository navigation
[87][88] Decode UTF-8 across Buffer chunks in the Node adapters, and document a faster encoder for the web pipeline - #93
Merged
Conversation
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
The README’s encoder safety guidance needs qualification for strategies that can target surrogate halves.
1 open finding
What changed in this PR
Fixes UTF-8 corruption across Node Buffer chunks, preserves surrogate pairs during regex processing, and documents faster web encoding.
Changes:
- Added streaming UTF-8 decoding to Node adapters.
- Updated regex zero-length handling to advance by code point.
- Added tests, shared utilities, documentation, and changelog entries.
| File | Summary |
|---|---|
test/utilities.ts |
Shares engine-output test helpers. |
src/search-strategies/regex/skip-one-code-unit-past-zero-length-match.ts |
Removes obsolete helper. |
src/search-strategies/regex/skip-one-code-point-past-zero-length-match.ts |
Adds code-point advancement. |
src/search-strategies/regex/search-strategy.test.ts |
Tests surrogate-pair preservation. |
src/search-strategies/regex/README.md |
Documents regex behavior. |
src/search-strategies/regex/lean-search-strategy.ts |
Uses code-point advancement. |
src/adapters/node/transform-base.ts |
Adds stateful UTF-8 decoding. |
src/adapters/node/sync-transform.ts |
Decodes chunks and flushes remainders. |
src/adapters/node/sync-transform.test.ts |
Tests synchronous decoding. |
src/adapters/node/sync-transform.integration.test.ts |
Tests synchronous integration behavior. |
src/adapters/node/async-transform.ts |
Handles asynchronous decoding and remainders. |
src/adapters/node/async-transform.test.ts |
Tests asynchronous decoding. |
src/adapters/node/async-transform.integration.test.ts |
Tests asynchronous integration behavior. |
README.md |
Documents encoding and performance guidance. |
docs/CHANGELOG.md |
Records the fixes and documentation updates. |
🧠 Review effort: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.

Issue
resolves #87
resolves #88
Details
#87: the Node adapters split multi-byte characters.
ReplaceContentTransformandAsyncReplaceContentTransformcalledchunk.toString()on eachBuffer, so a character split across two chunks became two U+FFFD. Each transform now has its ownStringDecoder:"héllo {{x}} wörld"as two Buffers split insideé, throughReplaceContentTransformwithStringAnchorSearchStrategy(["{{", "}}"])→"X"(Node 24.20.0):The output side had the same flaw. Each pushed string was encoded to UTF-8 on its own, so an output chunk ending between the two halves of a surrogate pair (e.g. a match for
/a./, nouflag, in"a😀") became two U+FFFD. The sink now holds a trailing high surrogate back until the next chunk, asTextEncoderStreamdoes:TransformBase engine.start({ - enqueue: chunk → push(chunk) + enqueue: chunk → text = held + chunk + held = text ends in a high surrogate ? its last code unit : "" + push(text without held) }) _flush() ... await engine.end() + push(held) # a lone high surrogate → U+FFFD"a😀b"throughReplaceContentTransformwithRegexSearchStrategy(/a./), echoing the match:decodeStrings: false, string chunks reach the engine unchanged. The defaultdecodeStrings: trueis still in place.TransformBaseJSDoc now describe how decoding works. For other charsets they say to decode upstream with a streamingTextDecoderand write strings.iconv-liteis mentioned only for re-encoding the output to a legacy charset.#88:
TextEncoderStreamis slow on older Node. On Node 22, Node 24 before 24.21.0 and Node 26 before 26.8.0,TextEncoderStreamcosts about 5–10 ms per MB (nodejs/node#65414). The README's WHATWG pipeline now has a tip with autf8EncoderStream()alternative: aTextEncoderin aTransformStreamthat holds a trailing high surrogate back, likeTextEncoderStream, so it is safe whatever the strategy or replacement emits. On ~2 MB in 16 KiB chunks (Node 24.20.0) it took 4.34 ms, against 4.50 ms for a plain per-chunkencodeand 35.91 ms forTextEncoderStream. The Compatibility section points to the tip.Scout rule
Regex zero-length skip splits surrogate pairs. Not part of either issue; found while checking #88's suggestion that no search strategy ends an output chunk part-way through a surrogate pair. When
RegexSearchStrategyskipped a zero-length match (e.g. from/a*/), it advanced by one code unit, so an emoji was split across two non-match chunks, which a per-chunk encoder turned into two U+FFFD. Both encoders in this PR now cope with a split pair, but the skip now advances by one code point, so the emoji stays in one chunk:"x😀y"throughReplaceContentTransformwithRegexSearchStrategy(/a*/)→"-":Footnote 10 in the regex README now describes the new behaviour and how it differs from
matchAllwithout theu/vflag.Tests:
This replaces the old test "advances by one code unit rather than one code point, splitting a surrogate pair but remaining lossless".
textWrittenTo(engine)moves totest/utilities.ts. The sync and async Node adapter tests now share it; before, each file had its own copy.Semantic Version Impact
No public API is added or removed, and reverting restores the old decoding and the old skip. Output text only changes where it was corrupted before. Behaviour that does change:
Bufferthat ends part-way through a character now has those bytes held back until the next chunk, soengine.writegets slightly less text and the next write gets the rest.engine.end(), so readers see that character one chunk later./a*|\uDE00/) no longer matches the second half of a pair, which it did before, asmatchAllwithoutu/vdoes.Checklist
[Unreleased]section inCHANGELOG.md, if the published package will change