fix(render-props): keep render prop state alive across renders - #193
Open
rkaraivanov wants to merge 7 commits into
Open
rkaraivanov wants to merge 7 commits into
rkaraivanov wants to merge 7 commits into
Conversation
The render prop machinery kept its state in per-render locals while the
element held on to the patched templates indefinitely, so the two drifted
apart the moment the component re-rendered. Three defects came out of that:
* Templates rendered against a stale closure. The patched function handed
to the element was cached on first sight - but so was the callback it
invoked, so a template closing over React state kept rendering the value
it saw on the very first render.
* An `undefined` render prop spun the render loop. A conditional prop
(`headerTemplate={enabled ? tpl : undefined}`) was still treated as a
template: it installed an empty portal and recreated the patched function
on every render. Each new identity re-rendered the element, which
re-requested the template, at roughly 97 cycles a second. The churn also
blanked sibling templates on the same component.
* Portal updates were lost. The map of active slots was cloned per render,
so a request arriving from the element mutated its own copy and a later
render overwrote it.
That state now lives on a `TemplateBridge` owned by the component instance -
patched templates keyed by prop path, current callbacks keyed by renderer
name, the slots, and the nested prop containers. Callbacks are refreshed on
every render while the patched templates keep their identity, so the element
sees a stable prop and the template always runs the current closure. Portals
are built when the element requests a slot, and rebuilt only when the render
prop's identity changes.
Pruning is driven by a single predicate - is there still a function at this
prop path? - which covers both a removed prop and one that turned
`undefined`. It also drops the matching slots: the directive's remove request
travels through a `WeakRef` and may never arrive once the patched template is
gone, leaving the portal to linger for the component's lifetime.
Nested prop containers reuse their previous object while shallow-equal, so a
config object carrying a template stops changing identity every render.
The rest of the file follows the same split. Ref forwarding and the Angular
re-parenting effect move into `useForwardedRef` and `useReparenting`, leaving
`createComponent` with registration and a nine-line component.
In `render-props.ts`: `_renderNode` could return `undefined` behind an
`as Element` cast; requests now go through a helper that bails when there is
no node. `render()` is typed as `RendererCallback<T>` so `any` stops leaking
into `update()`. `_state.previous` resets on every disconnect, not just some.
Adds regression tests for the three defects, each confirmed to fail against
the previous code.
Code Coverage OverviewLanguages: TypeScript TypeScript / React WrappersThe overall line coverage in commit 2a17c86 in the Show a line coverage summary of the most impacted files.
Updated |
Render prop state lived in per-render locals, while the element kept its patched templates indefinitely. The two drifted apart on every re-render: * Stale closures. A template reading React state kept the callback it saw first. * `undefined` render props rendered blank instead of the element's default. * A render prop added later evicted the others: requests updated a stale copy of the slot map. * Nested config objects, like `igc-chat`'s `options`, changed identity every render, so the element re-rendered with them. A `TemplateBridge` per component instance now owns that state. Templates keep their identity and close over nothing but the renderer name. Element requests only record slots; render fills them from the current props. Async render props are resolved by the bridge, which keeps the previous content until the new one settles. Suspending on them would never settle, since inline render props yield a new promise per render. Rejections rethrow on render for error boundaries; the nearest `<Suspense>` no longer sees them. `WithJsxRenderProps` recurses into nested renderer maps and keeps optional render props optional. Also fixes: * `equal` let failed Set/Map probes mark objects as visited, so collections with different contents compared equal. It now tracks pairs, matches entries one to one, and handles null-prototype objects. * Templates invoked without a context threw: `withDataContext` proxied non-objects.
3 tasks done
This branch has not been deployed
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
2 participants
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.
Fix render prop state across renders
Render prop state lived in per-render locals, while the element kept
its patched templates indefinitely. The two drifted apart on every
re-render:
it saw first.
undefinedrender props rendered blank instead of the element'sdefault.
stale copy of the slot map.
igc-chat'soptions, changedidentity every render, so the element re-rendered with them.
A
TemplateBridgeper component instance now owns that state.Templates keep their identity and close over nothing but the
renderer name. Element requests only record slots; render fills them
from the current props.
Async render props are resolved by the bridge, which keeps the
previous content until the new one settles. Suspending on them would
never settle, since inline render props yield a new promise per
render. Rejections rethrow on render for error boundaries; the
nearest
<Suspense>no longer sees them.WithJsxRenderPropsrecurses into nested renderer maps and keepsoptional render props optional.
Also fixes:
equallet failed Set/Map probes mark objects as visited, socollections with different contents compared equal. It now tracks
pairs, matches entries one to one, and handles null-prototype
objects.
withDataContextproxied non-objects.
Additional information (check all that apply):
Checklist: