Skip to content

Commit cb0ede4

Browse files
lifeartclaude
andcommitted
fix(gxt-backend): refresh state.currentGetter on pool reuse so child each-rows see new args
When a parent component force-rerenders, its inner {{#each}} block builds a fresh template subtree into a temp container that is later morphed onto the live DOM. Each iteration claims a child component from the pool. The pool match (by row identity or position) was correct, but the descriptor's rcGet closure captured a getter from the FIRST createRenderContext call — so `this.item` returned stale row data after the parent re-rendered. Two fixes, both targeting the WeakMap state that survives descriptor replacements: 1. updateInstanceWithNewArgs now updates state.currentGetter alongside argGetters[key], so the rcGet closure's `g = state.currentGetter` read sees the fresh per-row arg getter even when the descriptor lost its __gxtRenderCtxArgGetter marker (e.g., via an upstream cellFor reinstall) and createRenderContext's fast path is skipped. 2. createRenderContext's fast and slow paths both refresh state.currentGetter (slow path was relying on the closure-captured `getter`, which is frozen to the first install). The rcGet now reads `state.currentGetter || getter` so the latest per-row getter wins. Fixes 4 of the 7 remaining smoke failures: - Components test: curly components / non-block with each rendering child components - Syntax test: {{#each}} with native arrays / updating and setting within #each - Syntax test: {{#each}} with emberA-wrapped arrays / updating and setting within #each - Syntax test: {{#each}} with array proxies, * / updating and setting within #each (4 variants) The remaining 3 failures (DOM node stability for stable keys when list is updated) have a different root cause — morph-based fresh template render doesn't preserve DOM identity for keyed each items — and are left for a follow-up. Smoke results: 330/333 (was 326/333). No regressions. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
1 parent e7dda3c commit cb0ede4

1 file changed

Lines changed: 50 additions & 5 deletions

File tree

packages/@ember/-internals/gxt-backend/manager.ts

Lines changed: 50 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -194,18 +194,21 @@ let emberViewIdCounter = 0;
194194
// useLocal: whether to return localVal vs. the arg getter
195195
// This survives createRenderContext re-invocations even when the descriptor
196196
// is replaced by cellFor(obj, key, false) or other reactive reinstallers.
197-
const _rcArgState = new WeakMap<object, Map<string, { localVal: any; useLocal: boolean }>>();
197+
const _rcArgState = new WeakMap<
198+
object,
199+
Map<string, { localVal: any; useLocal: boolean; currentGetter?: (() => any) | null }>
200+
>();
198201
function _getRcArgState(
199202
instance: object,
200203
key: string
201-
): { localVal: any; useLocal: boolean } | undefined {
204+
): { localVal: any; useLocal: boolean; currentGetter?: (() => any) | null } | undefined {
202205
const m = _rcArgState.get(instance);
203206
return m ? m.get(key) : undefined;
204207
}
205208
function _setRcArgState(
206209
instance: object,
207210
key: string,
208-
state: { localVal: any; useLocal: boolean }
211+
state: { localVal: any; useLocal: boolean; currentGetter?: (() => any) | null }
209212
): void {
210213
let m = _rcArgState.get(instance);
211214
if (!m) {
@@ -2203,6 +2206,22 @@ function updateInstanceWithNewArgs(instance: any, args: any): boolean {
22032206
// instance's reactive getter would read from the stale old closure.
22042207
if (newGetter && argGetters && key !== 'elementId' && key !== 'id') {
22052208
argGetters[key] = newGetter;
2209+
// Also update the createRenderContext WeakMap state's currentGetter slot
2210+
// so that descriptor-bound rcGet closures (which read from
2211+
// state.currentGetter) see the new arg getter on the next read. This
2212+
// is critical for force-rerender of {{#each}} block bodies where the
2213+
// child's instance is pool-reused; createRenderContext's fast-path may
2214+
// not re-enter (e.g., when an upstream cellFor removed the marker on
2215+
// its descriptor), but the WeakMap state survives across descriptor
2216+
// replacements and is what rcGet's closure references.
2217+
try {
2218+
const _stateForGetter2 = _getRcArgState(instance, key);
2219+
if (_stateForGetter2) {
2220+
_stateForGetter2.currentGetter = newGetter;
2221+
}
2222+
} catch {
2223+
/* ignore */
2224+
}
22062225
// Check if there's already a render-context-installed descriptor with
22072226
// local-override state (from a previous createRenderContext call). If
22082227
// so, skip reinstalling to preserve the local override (e.g., after
@@ -5316,6 +5335,23 @@ function createRenderContext(instance: any, args: any, fw: any, owner: any): any
53165335
lastArgValue: getter(),
53175336
...(_savedOverride ? { initOverridden: true } : {}),
53185337
};
5338+
// Fix: the descriptor's rcGet reads from `state.currentGetter` on
5339+
// the WeakMap state. Update it to the latest getter so that reads
5340+
// during force-rerender (which builds a fresh outer template and
5341+
// re-invokes each-row child components with new arg closures) see
5342+
// the new value. Without this, the descriptor's closure holds the
5343+
// original getter from the FIRST createRenderContext call, and
5344+
// `this.item` returns stale row data after the parent re-renders.
5345+
// See also: updateInstanceWithNewArgs, which performs the same
5346+
// refresh up-front so the fix is robust even when an upstream
5347+
// cellFor reinstall has stripped the __gxtRenderCtxArgGetter
5348+
// marker (in which case this fast path is skipped entirely).
5349+
if (instance) {
5350+
const _stateForGetter = _getRcArgState(instance, key);
5351+
if (_stateForGetter) {
5352+
_stateForGetter.currentGetter = getter;
5353+
}
5354+
}
53195355
} catch {
53205356
/* ignore */
53215357
}
@@ -5574,7 +5610,14 @@ function createRenderContext(instance: any, args: any, fw: any, owner: any): any
55745610
// descriptor is replaced (e.g. by cellFor(obj, key, false) called
55755611
// elsewhere during the same render pass). The WeakMap survives.
55765612
const existingState = instance ? _getRcArgState(instance, key) : undefined;
5577-
const state = existingState || { localVal: initialVal, useLocal: false };
5613+
const state = existingState || { localVal: initialVal, useLocal: false, currentGetter: getter };
5614+
// Always refresh the currentGetter slot so the rcGet closure reads
5615+
// the latest arg getter (the per-row closure for #each iterations).
5616+
// The slow path runs on first install AND when the descriptor was
5617+
// replaced (e.g., by an upstream cellFor reinstall) — both need a
5618+
// fresh getter. The fast-path skip above keeps state.currentGetter
5619+
// in sync for subsequent createRenderContext re-invocations.
5620+
state.currentGetter = getter;
55785621
if (!existingState && instance) _setRcArgState(instance, key, state);
55795622
const rcGet: any = function () {
55805623
try {
@@ -5589,7 +5632,9 @@ function createRenderContext(instance: any, args: any, fw: any, owner: any): any
55895632
} catch {
55905633
/* noop */
55915634
}
5592-
return state.useLocal ? state.localVal : getter ? getter() : state.localVal;
5635+
if (state.useLocal) return state.localVal;
5636+
const g = state.currentGetter || getter;
5637+
return g ? g() : state.localVal;
55935638
};
55945639
rcGet.__gxtRenderCtxArgGetter = true;
55955640
const rcSet = function (v: any) {

0 commit comments

Comments
 (0)