Skip to content

Commit 5465e5a

Browse files
NullVoxPopuliclaude
andcommitted
Defer property evaluation to invocation time
Instead of eagerly calling getProp in the bound ref's compute function (which created parallel tracking and autotracking issues), the bound ref now returns a stable wrapper function that defers the property read to call time: parent[path].call(parent, ...args). This means the property is only tracked through childRefFor's existing tracking (for typeof check), and the actual method lookup happens when the function is invoked, not during render. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
1 parent e733f57 commit 5465e5a

1 file changed

Lines changed: 26 additions & 55 deletions

File tree

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

Lines changed: 26 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ import {
3939
} from '@glimmer/debug';
4040
import { debugToString, localAssert } from '@glimmer/debug-util';
4141
import { _hasDestroyableChildren, associateDestroyableChild, destroy } from '@glimmer/destroyable';
42-
import { debugAssert, getProp, setProp, toBool } from '@glimmer/global-context';
42+
import { debugAssert, setProp, toBool } from '@glimmer/global-context';
4343
import {
4444
getInternalComponentManager,
4545
getInternalHelperManager,
@@ -84,64 +84,39 @@ function hasDefinitionManager(value: object): boolean {
8484
);
8585
}
8686

87-
interface BoundEntry {
88-
original: CallableFunction;
89-
bound: CallableFunction;
90-
}
91-
92-
const BOUND_FN_CACHE: WeakMap<object, Map<string, BoundEntry>> = new WeakMap();
93-
94-
// A unique key prefix for bound child refs, to avoid collisions with
95-
// regular childRefFor's cache on the same parent reference.
96-
const BOUND_KEY_PREFIX = '\0bound:';
97-
98-
// Like childRefFor, but auto-binds the resolved value to its parent via
99-
// `.bind(parent)` when the value is a plain function (not a component or
100-
// modifier definition). This preserves `this` for class methods passed as
101-
// callbacks (e.g. `{{on "click" this.foo}}`).
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.
10290
//
103-
// The result is cached on the parent reference (same as childRefFor) so that
104-
// multiple reads of the same path return the same Reference identity — required
105-
// for modifier update diffing and autotracking stability.
91+
// Uses childRefFor internally for tracking — the property is only tracked
92+
// the same way it would be without binding.
10693
function boundChildRefFor(parentRef: Reference, path: string): Reference {
107-
let parentImpl = parentRef as { children: null | Map<string, Reference> };
94+
const innerRef = childRefFor(parentRef, path);
10895

109-
let cacheKey = BOUND_KEY_PREFIX + path;
96+
let cachedParent: object | null = null;
97+
let wrapper: CallableFunction | null = null;
11098

111-
if (parentImpl.children !== null) {
112-
let cached = parentImpl.children.get(cacheKey);
113-
if (cached) return cached;
114-
} else {
115-
parentImpl.children = new Map();
116-
}
117-
118-
const ref = createComputeRef(
99+
return createComputeRef(
119100
() => {
120-
const parent = valueForRef(parentRef);
121-
122-
if (isDict(parent)) {
123-
const value = getProp(parent, path);
124-
125-
if (typeof value === 'function' && !hasDefinitionManager(value as object)) {
126-
let cache = BOUND_FN_CACHE.get(parent);
127-
if (cache === undefined) {
128-
cache = new Map();
129-
BOUND_FN_CACHE.set(parent, cache);
130-
}
131-
132-
let entry = cache.get(path);
133-
if (entry === undefined || entry.original !== value) {
134-
entry = {
135-
original: value as CallableFunction,
136-
bound: (value as CallableFunction).bind(parent),
137-
};
138-
cache.set(path, entry);
139-
}
140-
return entry.bound;
141-
}
101+
const value = valueForRef(innerRef);
142102

103+
if (typeof value !== 'function' || hasDefinitionManager(value as object)) {
143104
return value;
144105
}
106+
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;
145120
},
146121
(val) => {
147122
const parent = valueForRef(parentRef);
@@ -151,10 +126,6 @@ function boundChildRefFor(parentRef: Reference, path: string): Reference {
151126
}
152127
}
153128
);
154-
155-
parentImpl.children.set(cacheKey, ref);
156-
157-
return ref;
158129
}
159130

160131
APPEND_OPCODES.add(VM_CURRY_OP, (vm, { op1: type, op2: _isStrict }) => {

0 commit comments

Comments
 (0)