[BUGFIX] plain method calling from templates (no explicit binding required -- follows JS semantics) - `on` still requires binding by NullVoxPopuli · Pull Request #21469 · emberjs/ember.js

NullVoxPopuli · 2026-06-12T19:58:02Z

supersedes

this PR enables/unblocks the change in recommendation RFC:

Recommend regular methods instead of the @action decorator rfcs#1045

Notes

we can't tie this-fixes to invokeHelper, because a helper manager's return value is from getValue, which only receives the args and definition from createHelper, so like.. 😬

as in, the helper manager pattern is what is screwing us over here, because we (correctly?) abstracted references away from public API, but the reference is what is needed to invoke functions.

So, now the path forward I see is to... not use a helper a manager for the "default helper manager" case, and wire it all up using only private apis, no manager

I think the best I con do is bind the definition before createHelper (only for the default case)

Main test cases I care about:

calling methods from classes (such as in a class component)
calling methods on chained property paths (could be class or plain object)

Things we didn't fix (because these are problems with JavaScript)

function references passed to on (addEventListener)
function references passed anywhere else

REPL PR: NullVoxPopuli/limber#2170
Deployed: https://test-ember-source-nvp-fix-th.limber-glimdown.pages.dev/
Demo: https://test-ember-source-nvp-fix-th.limber-glimdown.pages.dev/edit?c=JYWwDg9gTgLgBAYQuCA7Apq%2BAzKy4DkAAgOYA2oI6UA9AMbKQZYEDcAUKJLHAN5wwoAQzoBrdABM4AXzi58xcpWo1BI0cFQk27dugAe3eBPTYhAVzLw6ZIQGc7cABLoyZCAHVoZKQZiYJRyQUZnhedjg4IjUxSTgGcyw4AF44AAYOCLgaGjgodABHc2B8qRgIOAAjdDghKDwAdzg7GGA3LM06fKok1IAKAEoUgD4%2BLMiYAAtgOwA6BKSAalSARg5I2V1IiQhzSrJJQbHIyPyYcyhUAWm5hfgAKjgAJnWZLbgAHn9wW39h8c%2BYGGAE1dnBJkIAG41GzAWJlSY1SrmGDlK68XhTGbzXZYaSyVpUOYfGhAgEfIFCVBSHZ7A4SABcfExN1mtP2knxJLJ5ORqLQAgAnmB0MkAER8tFi5kCsWwsTSrFzTrdTAwfHDBAUMQkyVof6REnfMC-dD-aTsIA&format=gjs

NullVoxPopuli · 2026-06-12T19:59:30Z

      assert.equal(this.stashedFn(), 'arg1: foo, arg2: bar');
    }

-    '@test there is no `this` context within the callback'(assert) {


a change in behavior, but for the better -- I dare say: a bugfix

github-actions · 2026-06-12T20:00:59Z

📊 Size report

Tarball size — 1.2 MB → 1.2 MB

dist/dev `0.1%↑`

File	Before (Size / Brotli)	After (Size / Brotli)
`./packages/shared-chunks/args-{hash}.js`	`5.2 kB` / `1.3 kB`	^5%↑`5.5 kB` / ^6%↑`1.4 kB`
`./packages/shared-chunks/index-{hash}.js`	`62.6 kB` / `12.5 kB`	^2%↑`63.7 kB` / ^3%↑`12.9 kB`
`./packages/shared-chunks/program-{hash}.js`	`5 kB` / `1.4 kB`	^3%↑`5.2 kB` / ^5%↑`1.5 kB`
`./packages/shared-chunks/render-{hash}.js`	`55.5 kB` / `12 kB`	^0.5%↑`55.8 kB` / ^1%↑`12.1 kB`
Total _{(Includes all files)}	`2.1 MB` / `495.7 kB`	^0.1%↑`2.1 MB` / ^0.1%↑`496.3 kB`

dist/prod `0.1%↑`

File	Before (Size / Brotli)	After (Size / Brotli)
`./packages/shared-chunks/api-{hash}.js`	`25.4 kB` / `5.6 kB`	^-67.2%↓`8.3 kB` / ^-68.8%↓`1.8 kB`
`./packages/shared-chunks/args-{hash}.js`	`4 kB` / `971 B`	^6%↑`4.3 kB` / ^9%↑`1.1 kB`
`./packages/shared-chunks/index-{hash}.js`	`59.8 kB` / `12.1 kB`	^2%↑`61 kB` / ^3%↑`12.4 kB`
`./packages/shared-chunks/program-{hash}.js`	`5 kB` / `1.4 kB`	^3%↑`5.2 kB` / ^5%↑`1.5 kB`
`./packages/shared-chunks/render-{hash}.js`	`51.8 kB` / `11.2 kB`	^0.6%↑`52.1 kB` / ^0.9%↑`11.3 kB`
Total _{(Includes all files)}	`1.9 MB` / `453 kB`	^0.1%↑`1.9 MB` / ^0.2%↑`453.7 kB`

smoke-tests/v2-app-template/dist `0.1%↑`

File	Before (Size / Brotli)	After (Size / Brotli)
`./assets/main-{hash}.js`	`313.1 kB` / `83.9 kB`	^0.1%↑`313.4 kB` / ^0.03%↑`83.9 kB`
Total _{(Includes all files)}	`348.7 kB` / `94.9 kB`	^0.1%↑`349 kB` / ^0.02%↑`94.9 kB`

smoke-tests/v2-app-hello-world-template/dist `0.2%↑`

File	Before (Size / Brotli)	After (Size / Brotli)
`./assets/main-{hash}.js`	`152.5 kB` / `42.2 kB`	^0.2%↑`152.9 kB` / ^0.3%↑`42.3 kB`
Total _{(Includes all files)}	`152.9 kB` / `42.3 kB`	^0.2%↑`153.2 kB` / ^0.3%↑`42.5 kB`

_{🤖 This report was automatically generated by wyvox/pkg-size}

NullVoxPopuli · 2026-06-12T20:36:39Z

+
          // eslint-disable-next-line @typescript-eslint/no-unsafe-return -- @fixme
-          return (fn as AnyFn).call(context, ...args, ...invocationArgs);
+          return (fn as AnyFn).call(self, ...args, ...invocationArgs);


the actual fix is here

NullVoxPopuli · 2026-06-12T20:38:50Z

+   * The reference this one was created from via a property read (`parent.path`),
+   * if any. See {@linkcode parentRefFor}.
+   */
+  public parent: Nullable<Reference> = null;


Why we considered touching the opcode/VM layer (and why we're backing it out)

We explored gating this-binding with a new boolean operand on VM_DYNAMIC_HELPER_OP plus member-binding variants of the append stdlib. We're reverting that, but it's worth recording why it came up, because the constraint is real and will resurface for anyone who wants the stricter semantics later.

The irreducible problem: at runtime, {{(this.obj.method)}} and {{(@cb)}} (where @cb={{this.foo}}) arrive at the invocation with identical shapes. In both cases the callee is a reference whose .parent resolves to an object — because a glimmer reference remembers its parent, and @cb literally is this.foo's child-reference passed through. So parentRefFor(callee) returns a bindable object in both cases.

The only thing that differs is source syntax: this.obj.method has a path tail; @cb does not. That's compile-time-only information. To bind the first but not the second (matching JS, where obj.method() binds but const f = obj.method; f() does not), something has to carry one bit from the compiler to the runtime. A bool operand on the existing opcode is the cheapest carrier — a second opcode or smuggling it through args/refs is strictly more complex.

Why it ballooned: the one-bit operand itself is ~5 small edits. The bulk of the complexity was making bare {{item.greet}} (no parens) bind. Bare appends route through the shared, generic cautious-append stdlib routine, which is compiled once and can't carry a per-call-site bit — so binding them required forking that routine (StdLib class, operand unions, interface unions, two new stdlib handles).

The decision: require parens for invocation-with-binding ({{(item.greet)}}), and treat {{item.greet}} (append of a member path) as not-this-bound. That lets us delete the stdlib variants entirely.

⚠️ Known limitation of the reverted approach: without the compile-time bit, {{(@cb)}} (a parenthesized call of a passed-in function reference) will bind this to wherever the reference was originally read from, rather than calling it unbound. This is the passing function references loses the "this" case. Fully matching JS there is what the opcode bit bought; we're consciously trading that exactness for a dramatically smaller change.

@cb

…fFor) `(this.obj.method)` invokes `method` with `this.obj` as `this` (JS member-call semantics), while `(@cb)` — a function passed as an argument — stays unbound, like `const f = obj.m; f()`. Whether a call has a receiver is a *syntactic* question (does the callee have a path tail?), independent of how the callee reference was derived. So the compiler computes the receiver (`receiverExpressionFor`: the callee minus its last path segment) and pushes it at the call site; `VM_DYNAMIC_HELPER_OP` reads it into `args.context`. `parentRefFor` is no longer consulted at the call site. Bare member-path appends (`{{item.greet}}`) have no syntactic receiver and are unbound — use `{{(item.greet)}}` to bind. Full suite: 9395 tests, 63898 assertions, green (the one red `(fn)` test stringifies an untouchable `this` and fails regardless of this change). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

NullVoxPopuli · 2026-06-30T23:34:21Z

🤖 Note: this comment was written by Claude (Opus 4.8), driving via Claude Code, while pairing with @NullVoxPopuli. Posting for the historical record.

Update: landed on a compiler-provided syntactic receiver (no opcode operand, no `parentRefFor`)

Since the last comment we explored three more shapes for the member-call vs passed-reference distinction and converged on the one that matches the @ef4 framing in Discord: the receiver is a syntactic property of the call site, not a function of how the callee reference was derived.

What we tried, in order:

Revert all the opcode/stdlib work → unconditional args.context = parentRefFor(ref). Simplest possible, but it over-binds (@cb) (a passed function ref still reports its original parent) and can't express "() required." Confirmed empirically: passing function references loses the "this" fails.
Detach the parent when a value is passed as an argument (drop .parent at capture() / NamedArgumentsImpl.get). This passed the full suite (9395 tests green), but it's the wrong concept: it manipulates ref derivation to fake a syntactic property. As ef4 put it — "how a ref gets derived is its own business… that is a separate concept from: is there a syntactic this argument in my invocation?" So we dropped it.
Compiler-provided syntactic receiver (landed, this push). The compiler answers the syntactic question directly: a call has a receiver iff the callee has a path tail. receiverExpressionFor(callee) returns the callee minus its last segment (this.obj.method → this.obj, @cb → none). CallDynamic pushes that receiver at the call site; VM_DYNAMIC_HELPER_OP pops it into args.context. parentRefFor is no longer consulted — derivation is irrelevant.

Resulting semantics (JS member-call faithful)

Template	binds `this`?
`{{(this.obj.method)}}`	✅ to `this.obj`
`{{(this.foo)}}`	✅ to `this`
`<Child @cb={{this.foo}} />` → `{{(@cb)}}`	❌ (passed ref, like `const f = obj.m; f()`)
`{{(@cb.method)}}`	✅ to `@cb`
`{{(item.greet)}}` (block param)	✅ to `item`
`{{item.greet}}` (bare append, no parens)	❌ — use `{{(item.greet)}}` to bind

() is now the marker for invoke-and-bind. Bare member-path appends flow through the shared cautious-append stdlib, which has no per-call-site receiver, so they invoke unbound. The this-binding-test.gjs iterated-item case was updated to {{(item.greet)}} accordingly.

Footprint

+56/−15 across 6 files, entirely in the compiler + the one opcode — no new opcode operand, no stdlib append variants, no reference-graph hacks. Full suite: 9395 tests / 63898 assertions, green. The single red test (a plain method passed through (fn) is not this-bound) fails independently of this work — it stringifies (fn)'s buildUntouchableThis (Symbol.toPrimitive), which throws in DEBUG; it needs a capture-and-compare rewrite rather than ${this}.

Matches the `receiver` local in `VM_DYNAMIC_HELPER_OP` and reads more clearly: `Arguments.receiver` / `args.receiver` is the object a member-call's function is invoked with as `this`. Pure rename — no behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

NullVoxPopuli commented Jun 12, 2026

View reviewed changes

NullVoxPopuli mentioned this pull request Jun 12, 2026

Enable plain methods to work on classes when called from templates #21341

Closed

NullVoxPopuli changed the title ~~fix plain method calling~~ [BUGFIX] enable plain method calling from templates Jun 12, 2026

NullVoxPopuli changed the title ~~[BUGFIX] enable plain method calling from templates~~ [BUGFIX] enable plain method calling from templates (no explicit binding required -- follows JS semantics) Jun 12, 2026

NullVoxPopuli mentioned this pull request Jun 12, 2026

Test out: https://github.com/emberjs/ember.js/pull/21469 NullVoxPopuli/limber#2170

Open

NullVoxPopuli marked this pull request as ready for review June 12, 2026 20:35

NullVoxPopuli commented Jun 12, 2026

View reviewed changes

ef4 reviewed Jun 16, 2026

View reviewed changes

Comment thread packages/@ember/-internals/glimmer/tests/integration/this-binding-test.gjs Outdated

NullVoxPopuli marked this pull request as draft June 16, 2026 18:24

NullVoxPopuli force-pushed the nvp/fix-this-binding-for-real branch from f25e012 to 889278b Compare June 16, 2026 20:53

NullVoxPopuli changed the title ~~[BUGFIX] enable plain method calling from templates (no explicit binding required -- follows JS semantics)~~ [BUGFIX] plain method calling from templates (no explicit binding required -- follows JS semantics) - on still requires binding Jun 16, 2026

NullVoxPopuli force-pushed the nvp/fix-this-binding-for-real branch 2 times, most recently from 4b573d6 to 9c29b51 Compare June 16, 2026 21:44

NullVoxPopuli marked this pull request as ready for review June 16, 2026 22:11

NullVoxPopuli requested a review from ef4 June 26, 2026 04:24

NullVoxPopuli commented Jun 30, 2026

View reviewed changes

Make default function invocation behave like JavaScript (re: this)

31130d8

NullVoxPopuli force-pushed the nvp/fix-this-binding-for-real branch from 9c29b51 to 31130d8 Compare June 30, 2026 19:16

NullVoxPopuli marked this pull request as draft June 30, 2026 19:17

Is this crazy

8c75fa2

NullVoxPopuli and others added 2 commits June 30, 2026 18:55

Fix typo

d113eb2

NullVoxPopuli and others added 3 commits June 30, 2026 20:50

Fix test

f1096e5

Clean

7c17ec2

                     associateDestroyableChild(helperInstanceRef, helperRef);
                   } else if (isIndexable(definition)) {
-                    let helper = resolveHelper(definition, ref);
+                    let helper = resolveHelper(bindFunctionToParent(definition, ref), ref);

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

[BUGFIX] plain method calling from templates (no explicit binding required -- follows JS semantics) - `on` still requires binding #21469

[BUGFIX] plain method calling from templates (no explicit binding required -- follows JS semantics) - `on` still requires binding #21469
NullVoxPopuli wants to merge 7 commits into
mainfrom
nvp/fix-this-binding-for-real

NullVoxPopuli commented Jun 12, 2026 •

edited

Loading

Uh oh!

NullVoxPopuli Jun 12, 2026

Uh oh!

github-actions Bot commented Jun 12, 2026 •

edited

Loading

Uh oh!

NullVoxPopuli Jun 12, 2026

Uh oh!

NullVoxPopuli Jun 12, 2026

Uh oh!

Uh oh!

NullVoxPopuli commented Jun 16, 2026

Uh oh!

NullVoxPopuli Jun 30, 2026

Uh oh!

NullVoxPopuli commented Jun 30, 2026

Uh oh!

NullVoxPopuli commented Jun 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

NullVoxPopuli commented Jun 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Notes

Uh oh!

NullVoxPopuli Jun 12, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jun 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📊 Size report

dist/dev 0.1%↑

dist/prod 0.1%↑

smoke-tests/v2-app-template/dist 0.1%↑

smoke-tests/v2-app-hello-world-template/dist 0.2%↑

Uh oh!

NullVoxPopuli Jun 12, 2026

Choose a reason for hiding this comment

Uh oh!

NullVoxPopuli Jun 12, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

NullVoxPopuli commented Jun 16, 2026

Uh oh!

NullVoxPopuli Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

NullVoxPopuli commented Jun 30, 2026

Why we considered touching the opcode/VM layer (and why we're backing it out)

Uh oh!

NullVoxPopuli commented Jun 30, 2026

Update: landed on a compiler-provided syntactic receiver (no opcode operand, no parentRefFor)

Resulting semantics (JS member-call faithful)

Footprint

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

NullVoxPopuli commented Jun 12, 2026 •

edited

Loading

github-actions Bot commented Jun 12, 2026 •

edited

Loading

dist/dev `0.1%↑`

dist/prod `0.1%↑`

smoke-tests/v2-app-template/dist `0.1%↑`

smoke-tests/v2-app-hello-world-template/dist `0.2%↑`

Update: landed on a compiler-provided syntactic receiver (no opcode operand, no `parentRefFor`)