fix: re-render memoized components when their output changes - #584
Conversation
|
Someone is attempting to deploy a commit to the Vercel Team on Vercel. A member of the Team first needs to authorize it. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
8915651 to
adef44e
Compare
|
Rebased onto Worth noting for review: The general case #581 does not cover still reproduces on |
95fd400 to
ee6c460
Compare
The default markdown components memoize on `className` plus the source position of their hast node. Source position is a proxy for render state, and it is not a sound one: a replacement of the same length occupies the same lines and columns, so the comparator reports "unchanged" for content that changed and the component keeps rendering the previous text. Replace the position comparison with React's own shallow prop comparison — what `memo` does with no comparator at all — minus the `node` prop, which is a fresh object on every parse. The one value read off `node` that reaches the output, a code fence's meta string, is compared explicitly. The skip that matters is preserved: an unchanged block is memoized a level up and is never re-rendered, so these comparators only run for a block that was re-parsed. Co-Authored-By: Ian Heidt <71056976+dasheidt@users.noreply.github.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ee6c460 to
12b1cd6
Compare
The contract
A memoized markup component may skip rendering only when every prop capable of changing its rendered output is equivalent.
The default components memoize on
classNameplus the source position of their hast node. Source position is a proxy for render state, and it is not a sound one.The bug
A replacement of the same length occupies the same lines and the same columns, so the comparator reports "unchanged" for content that changed. The component keeps rendering the previous text. No plugins, no custom components — this is
<Streamdown>with default settings, onmainat3327c18:Same for a heading, a list item and a
<strong>run — I re-ran all four againstmainafter #581 landed, and all four are still stale. Table cells and blockquotes behave the same way.pis not exempt either: the paragraph above is aMemoParagraph.Relation to #581
#581 fixed #570 by having the animate plugin stamp
data-sd-animatedon ancestors and adding that stamp as a third conjunct tosameClassAndNode. That closes the animation-state transition it targets, and this PR keeps it closed —__tests__/animate-issues.test.tsxpasses here unmodified, because the stamp is delivered as a prop and a shallow prop comparison therefore includes it for free.What #581 does not change is the proxy itself. Position is still standing in for render state for every non-animated path, which is why the repro above survives it. #570 was one manifestation of that class; this PR closes the class.
The change
sameClassAndNodebecomes React's own shallow prop comparison — whatmemodoes with no comparator at all — minus thenodeprop, which is a fresh object on every parse and would defeat memoization if compared by identity. The one value read offnodethat reaches the output, a code fence's meta string, is compared explicitly byMemoCode.Three components inlined the position expression instead of calling the helper (
MemoLi,MemoCode,MemoImg); they now use it too.MemoAno longer needs its extrahrefconjunct, becausehrefis a prop.This is not "removing the memo"
The skip that matters is a level up. An unchanged block is memoized by
Blockand is never re-rendered, so these comparators only run for a block that was re-parsed — exactly the case where the output can differ.__tests__/code-block-memo.test.tsxis unmodified and still green.The new tests assert both directions, so this cannot quietly become "always re-render":
mainBenchmarks
Added
__benchmarks__/streaming-rerender.bench.tsx, because the existing benchmarks measure parsing and this change does not touch the parser — the comparators only cost anything on the mount-then-re-render path.Interleaved A/B against
mainat3327c18, 5 pairs each, medians of hz (higher is better):mainRun-to-run spread within a single arm was larger than the gap between arms, so I would read this as no measurable difference rather than as a win or a loss.
The comparator itself is slower per call, as expected — it allocates a key list where the old one compared four scalars:
That is ~0.1µs more per comparison. A document with fifty elements re-rendering twenty times a second spends about 100µs/s more in comparators, which is why it does not show up end to end.
One test changed, and it is worth a look
__tests__/list-animation-retrigger.test.tsx(#410) asserted that thedata-sd-animatespan objects survive a tight → loose list transition. They survived because the transition was never rendered: the list keeps its source positions, soMemoLiskipped and the DOM kept a tight list for markdown that had become loose.With this change the transition renders, so the spans are rebuilt. What #410 actually reported — already-visible characters re-running their entry animation — is still prevented, by the second layer the same file documents:
prevContentLengthsets--sd-duration: 0msfor text already rendered. So the test now asserts durations rather than object identity, and additionally asserts the loose list actually renders.One caveat I would rather flag than bury: at that transition, one character at the boundary is not suppressed and re-animates. The character accounting in
lib/animate.tscounts inter-element whitespace, and tight → loose changes how much of it the block contains, so the boundary lands one character early. Ordinary streaming appends are unaffected. That accounting is untouched here; it was simply unreachable before, because the block never re-rendered — and I re-confirmed it is still one character off on top of #581's rewritten animate pipeline.Relation to other open work
offsetto the position comparison does not cover it: a same-length replacement moves no offset either.MemoImg/MemoLiconsolidation if you would rather keep those comparators inline.Rebased onto
mainat3327c18(post-#581). Verified withpnpm build:packages,vitest run(79 files, 1076 tests, all green including #581's animation suites),biome checkon the changed files.