Skip to content
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { RenderingTestCase, moduleFor } from 'internal-test-helpers';

moduleFor(
'Path expression this-binding for class methods',
class extends RenderingTestCase {
['@test foo.foo maintains this binding when invoked from a template'](assert) {
let instance;
let seenThis;

class Foo {
constructor() {
instance = this;
}

foo() {
seenThis = this;
}
}

let foo = new Foo();

let TestComponent = <template>{{ (foo.foo) }}</template>;
this.render(`<this.TestComponent />`, { TestComponent });

assert.strictEqual(seenThis, instance, 'this binding is maintained');
}
}
);
72 changes: 70 additions & 2 deletions packages/@glimmer/reference/lib/reference.ts
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,72 @@ export function updateRef(_ref: Reference, value: unknown) {
update(value);
}

// When a path read like `this.foo` resolves to a prototype method on the
// parent, we want the consumer (e.g. `{{on "click" this.foo}}`) to see a
// `this`-bound version, but we must NOT break subsequent property reads
// (`this.func.aProp` where `func` is a function carrying its own properties).
//
// A `Function.prototype.bind` call creates a brand new function object that
// loses the original's own properties, which breaks the latter case. So
// instead we return a callable Proxy: invoking it binds `this` to the parent,
// while property access is forwarded transparently to the original function.
//
// The result is cached on the parent (weakly) so that identity is stable
// across revalidations — required by modifiers that diff callback identity
// (e.g. `{{on}}` only re-installs the DOM listener when the callback ref
// changes).
type AnyFn = (...args: unknown[]) => unknown;

const BOUND_METHODS: WeakMap<object, Map<string, AnyFn>> = new WeakMap();

// True when `path` resolves on the parent's prototype chain to a function
// declared as a class method. Class-body method shorthand (`foo() {}`) is
// non-enumerable on the prototype, while functions assigned via object
// literals (e.g. `Component.extend({ foo() {} })` or a plain context like
// `{ foo() {} }`) end up enumerable. We only auto-bind the former so that
// existing object-literal contracts (no implicit `this`) are preserved.
function isClassMethod(parent: object, path: string): boolean {
let proto: object | null = Object.getPrototypeOf(parent) as object | null;
while (proto !== null && proto !== Object.prototype && proto !== Function.prototype) {
const desc = Object.getOwnPropertyDescriptor(proto, path);
if (desc !== undefined) {
return desc.enumerable === false && typeof desc.value === 'function';
}
proto = Object.getPrototypeOf(proto) as object | null;
}
return false;
}

function maybeBindPrototypeMethod(parent: object, path: string, value: unknown): unknown {
if (
typeof value !== 'function' ||
parent === null ||
(typeof parent !== 'object' && typeof parent !== 'function') ||
Object.prototype.hasOwnProperty.call(parent, path) ||
!isClassMethod(parent, path)
) {
return value;
}

let cache = BOUND_METHODS.get(parent);
if (cache === undefined) {
cache = new Map();
BOUND_METHODS.set(parent, cache);
}

let bound = cache.get(path);
if (bound === undefined) {
const target = value as AnyFn;
bound = new Proxy(target, {
apply(fn, _thisArg, args) {
return Reflect.apply(fn, parent, args);
},
});
cache.set(path, bound);
}
return bound;
}

export function childRefFor(_parentRef: Reference, path: string): Reference {
Comment thread
NullVoxPopuli marked this conversation as resolved.
const parentRef = _parentRef as ReferenceImpl;

Expand All @@ -213,8 +279,9 @@ export function childRefFor(_parentRef: Reference, path: string): Reference {
const parent = valueForRef(parentRef);

if (isDict(parent)) {
const raw = (parent as Record<string, unknown>)[path];
child = createUnboundRef(
(parent as Record<string, unknown>)[path],
maybeBindPrototypeMethod(parent, path, raw),
DEBUG && `${parentRef.debugLabel}.${path}`
);
} else {
Expand All @@ -226,7 +293,8 @@ export function childRefFor(_parentRef: Reference, path: string): Reference {
const parent = valueForRef(parentRef);

if (isDict(parent)) {
return getProp(parent, path);
const value = getProp(parent, path);
return maybeBindPrototypeMethod(parent, path, value);
}
},
(val) => {
Expand Down
Loading