Skip to content

Commit 67a52d5

Browse files
NullVoxPopuliclaude
andcommitted
Use childRefFor transform instead of wrapper ComputeRef
Pass a bindTransform callback to childRefFor so that binding happens inside the same ref — same tracking, same caching, same debug labels. Eliminates the wrapper ComputeRef that was causing autotracking divergence and identity issues. The transform wraps function values in a stable callable that defers property lookup to invocation time. Non-function values (strings, objects, components, modifiers) pass through unchanged. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
1 parent 5465e5a commit 67a52d5

3 files changed

Lines changed: 51 additions & 54 deletions

File tree

packages/@glimmer/reference/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ export {
99
} from './lib/iterable';
1010
export {
1111
childRefFor,
12+
type ChildRefTransform,
1213
childRefFromParts,
1314
createComputeRef,
1415
createConstRef,

packages/@glimmer/reference/lib/reference.ts

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -193,18 +193,28 @@ export function updateRef(_ref: Reference, value: unknown) {
193193
update(value);
194194
}
195195

196-
export function childRefFor(_parentRef: Reference, path: string): Reference {
196+
export type ChildRefTransform = (parent: object, path: string, value: unknown) => unknown;
197+
198+
export function childRefFor(
199+
_parentRef: Reference,
200+
path: string,
201+
transform?: ChildRefTransform
202+
): Reference {
197203
const parentRef = _parentRef as ReferenceImpl;
198204

199205
const type = parentRef[REFERENCE];
200206

201207
let children = parentRef.children;
202208
let child: Reference;
203209

210+
// Use a distinct cache key when a transform is provided so that the same
211+
// (parent, path) pair can have both a plain and a transformed child ref.
212+
const cacheKey = transform ? '\0bound:' + path : path;
213+
204214
if (children === null) {
205215
children = parentRef.children = new Map();
206216
} else {
207-
const next = children.get(path);
217+
const next = children.get(cacheKey);
208218

209219
if (next) return next;
210220
}
@@ -213,10 +223,9 @@ export function childRefFor(_parentRef: Reference, path: string): Reference {
213223
const parent = valueForRef(parentRef);
214224

215225
if (isDict(parent)) {
216-
child = createUnboundRef(
217-
(parent as Record<string, unknown>)[path],
218-
DEBUG && `${parentRef.debugLabel}.${path}`
219-
);
226+
let value: unknown = (parent as Record<string, unknown>)[path];
227+
if (transform) value = transform(parent, path, value);
228+
child = createUnboundRef(value, DEBUG && `${parentRef.debugLabel}.${path}`);
220229
} else {
221230
child = UNDEFINED_REFERENCE;
222231
}
@@ -226,7 +235,8 @@ export function childRefFor(_parentRef: Reference, path: string): Reference {
226235
const parent = valueForRef(parentRef);
227236

228237
if (isDict(parent)) {
229-
return getProp(parent, path);
238+
const value = getProp(parent, path);
239+
return transform ? transform(parent, path, value) : value;
230240
}
231241
},
232242
(val) => {
@@ -243,7 +253,7 @@ export function childRefFor(_parentRef: Reference, path: string): Reference {
243253
}
244254
}
245255

246-
children.set(path, child);
256+
children.set(cacheKey, child);
247257

248258
return child;
249259
}

packages/@glimmer/runtime/lib/compiled/opcodes/expressions.ts

Lines changed: 32 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,14 @@ import {
3939
} from '@glimmer/debug';
4040
import { debugToString, localAssert } from '@glimmer/debug-util';
4141
import { _hasDestroyableChildren, associateDestroyableChild, destroy } from '@glimmer/destroyable';
42-
import { debugAssert, setProp, toBool } from '@glimmer/global-context';
42+
import { debugAssert, toBool } from '@glimmer/global-context';
4343
import {
4444
getInternalComponentManager,
4545
getInternalHelperManager,
4646
getInternalModifierManager,
4747
} from '@glimmer/manager';
4848
import type { Reference } from '@glimmer/reference';
49+
import type { ChildRefTransform } from '@glimmer/reference';
4950
import {
5051
childRefFor,
5152
createComputeRef,
@@ -54,7 +55,7 @@ import {
5455
UNDEFINED_REFERENCE,
5556
valueForRef,
5657
} from '@glimmer/reference';
57-
import { assign, isDict, isIndexable } from '@glimmer/util';
58+
import { assign, isIndexable } from '@glimmer/util';
5859
import { $v0 } from '@glimmer/vm';
5960

6061
import { isCurriedType, resolveCurriedValue } from '../../curried-value';
@@ -75,58 +76,43 @@ import {
7576

7677
// Returns true if the value has an associated component or modifier manager —
7778
// meaning it is a definition object that must not be bound. Helper managers are
78-
// excluded because ALL functions have a default helper manager (RFC #756), and
79-
// binding a helper function is harmless.
79+
// excluded because ALL functions have a default helper manager (RFC #756).
8080
function hasDefinitionManager(value: object): boolean {
8181
return (
8282
getInternalModifierManager(value, true) !== null ||
8383
getInternalComponentManager(value, true) !== null
8484
);
8585
}
8686

87-
// Like childRefFor, but for function values returns a stable wrapper that
88-
// defers the property read to invocation time, calling it with the parent
89-
// as `this`. Non-function values pass through unchanged.
90-
//
91-
// Uses childRefFor internally for tracking — the property is only tracked
92-
// the same way it would be without binding.
93-
function boundChildRefFor(parentRef: Reference, path: string): Reference {
94-
const innerRef = childRefFor(parentRef, path);
95-
96-
let cachedParent: object | null = null;
97-
let wrapper: CallableFunction | null = null;
98-
99-
return createComputeRef(
100-
() => {
101-
const value = valueForRef(innerRef);
102-
103-
if (typeof value !== 'function' || hasDefinitionManager(value as object)) {
104-
return value;
105-
}
87+
// Per-parent cache of wrapper functions so that the same (parent, path) pair
88+
// returns a stable callable identity across revalidations.
89+
const BOUND_FN_CACHE: WeakMap<object, Map<string, CallableFunction>> = new WeakMap();
90+
91+
// Transform for childRefFor that wraps function values in a stable callable
92+
// preserving `this`. The wrapper defers the actual property lookup to
93+
// invocation time — `parent[path]` is evaluated when the function is called,
94+
// not during render.
95+
const bindTransform: ChildRefTransform = (parent, path, value) => {
96+
if (typeof value !== 'function' || hasDefinitionManager(value as object)) {
97+
return value;
98+
}
10699

107-
const parent = valueForRef(parentRef);
108-
109-
// Return a stable wrapper per parent identity. The wrapper defers
110-
// the property read to call time — `parent[path]` is evaluated
111-
// only when the function is actually invoked, not during render.
112-
if (parent !== cachedParent) {
113-
cachedParent = parent;
114-
wrapper = function (this: unknown, ...args: unknown[]) {
115-
const fn = (parent as Record<string, CallableFunction>)[path];
116-
return fn?.call(parent, ...args);
117-
};
118-
}
119-
return wrapper;
120-
},
121-
(val) => {
122-
const parent = valueForRef(parentRef);
100+
let cache = BOUND_FN_CACHE.get(parent);
101+
if (cache === undefined) {
102+
cache = new Map();
103+
BOUND_FN_CACHE.set(parent, cache);
104+
}
123105

124-
if (isDict(parent)) {
125-
return setProp(parent, path, val);
126-
}
127-
}
128-
);
129-
}
106+
let wrapper = cache.get(path);
107+
if (wrapper === undefined) {
108+
wrapper = function (this: unknown, ...args: unknown[]) {
109+
const fn = (parent as Record<string, CallableFunction>)[path];
110+
return fn?.call(parent, ...args);
111+
};
112+
cache.set(path, wrapper);
113+
}
114+
return wrapper;
115+
};
130116

131117
APPEND_OPCODES.add(VM_CURRY_OP, (vm, { op1: type, op2: _isStrict }) => {
132118
let stack = vm.stack;
@@ -275,7 +261,7 @@ APPEND_OPCODES.add(VM_GET_PROPERTY_OP, (vm, { op1: _key }) => {
275261
APPEND_OPCODES.add(VM_GET_PROPERTY_BOUND_OP, (vm, { op1: _key }) => {
276262
let key = vm.constants.getValue<string>(_key);
277263
let expr = check(vm.stack.pop(), CheckReference);
278-
vm.stack.push(boundChildRefFor(expr, key));
264+
vm.stack.push(childRefFor(expr, key, bindTransform));
279265
});
280266

281267
APPEND_OPCODES.add(VM_GET_BLOCK_OP, (vm, { op1: _block }) => {

0 commit comments

Comments
 (0)