Repository navigation
Conversation
buildReplacementHandlers cuts a range element by element after a marker pass over a clone, a callback per element in the container, twice. When every range's markers are direct children of one container, spliceRangeReplacements marks each cut with comments in one pass, slices the string, and leaves a placeholder component so ContentElementHandler still renders replaceWith. Any other shape returns null and the per-element path runs unchanged. A parity test runs the range fixtures and edge cases through both paths and asserts identical HTML. On a 200 KB article the range replacement drops from ~30 ms to ~3 ms.
🦋 Changeset detectedLatest commit: 8b20ce7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved output-parity regressions affect range boundaries, component matching, rendering, and encoding.
6 open findings
What changed in this PR
Optimizes simple replaceRange operations in the HTML component pipeline using string splicing, while retaining the existing per-element fallback.
Changes:
- Adds marker-based splicing with placeholder rendering.
- Adds 19 parity cases covering supported shapes and fallbacks.
- Documents the optimization in a patch changeset.
| File | Description |
|---|---|
| test/stages/spliceRangeReplacements.spec.ts | Tests output parity and path selection. |
| src/stages/surfaceComponents.ts | Routes eligible requests through splicing. |
| src/stages/spliceRangeReplacements.ts | Implements eligibility checks, cuts, and placeholders. |
| .changeset/range-splice.md | Records the performance improvement. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if (!eligible) { | ||
| return null | ||
| } | ||
| marked = await session.transform(new Response(await response.clone().text(), { headers: { 'content-type': 'text/html' } })).text() |
Comment on lines
+24
to
+26
| if (!ranges.length || !response.body) { | ||
| return null | ||
| } |
| let session = rewriter.create() | ||
| ranges.forEach(([, behavior], k) => { | ||
| const selector = behavior.metadata.cssSelector | ||
| const { fromMarker, toMarker } = behavior.content.replaceRange |
Comment on lines
+50
to
+53
| if (open || !element.onEndTag) { | ||
| eligible = false // nested containers | ||
| return | ||
| } |
Comment on lines
+147
to
+151
| let html = marked | ||
| for (const span of [...spans].reverse()) { | ||
| html = html.slice(0, span.from) + (span.hasReplacement ? placeholder(nonce, span.k) : '') + html.slice(span.to) | ||
| } | ||
| html = html.replaceAll(new RegExp(`<!--mos-range-${nonce}-[a-z]+-\\d+-\\d+-->`, 'g'), '') |
| } | ||
| const spliced = rewriter.capabilities.onEndTag ? await spliceRangeReplacements(response, surfaceDecisions, rewriter) : null | ||
| return spliced | ||
| ? transformComponents(ctx, spliced.response, spliced.surfaceDecisions, rewriter) |
benney
deleted the
feature/md-2768-mos-proxy-splice-replacerange-in-one-pass
branch
October 15, 2026 14:54
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.
buildReplacementHandlerscuts a range element by element after a marker pass over a clone: a JS callback per element in the container, twice. On a 200 KB article that is ~30 ms.When every range's markers are direct children of one container,
spliceRangeReplacementsmarks the cut with comments in one rewriter pass, slices the string, and leaves a placeholder component whosebeforerendersreplaceWith, so rendering still goes throughContentElementHandler. 200 KB range: 29.6 → 2.5 ms.Any other shape (markers nested below a direct child, nested or overlapping containers, identical markers, an invalid selector) returns
nulland the existing per-element path, nowtransformComponents, runs on the whole decision. Only used whenrewriter.capabilities.onEndTag.Patch changeset.