Skip to content

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

Open
NullVoxPopuli wants to merge 4 commits into
mainfrom
nvp/fix-this-binding-for-real
Open

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

Conversation

@NullVoxPopuli

@NullVoxPopuli NullVoxPopuli commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

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

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@github-actions

github-actions Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

📊 Size report

Tarball size1.2 MB1.2 MB

dist/dev   0.1%↑

File Before (Size / Brotli) After (Size / Brotli)
./packages/shared-chunks/reference-{hash}.js 4.9 kB / 1.3 kB 15%↑5.6 kB / 20%↑1.5 kB
./packages/shared-chunks/render-{hash}.js 55.5 kB / 12 kB 2%↑56.8 kB / 3%↑12.4 kB
./packages/shared-chunks/template-{hash}.js 491 B / 203 B 118%↑1.1 kB / 98%↑401 B
Total (Includes all files) 2.1 MB / 492.6 kB 0.1%↑2.1 MB / 0.2%↑493.4 kB

dist/prod   0.1%↑

File Before (Size / Brotli) After (Size / Brotli)
./packages/shared-chunks/reference-{hash}.js 4.3 kB / 1.2 kB 16%↑5 kB / 22%↑1.4 kB
./packages/shared-chunks/render-{hash}.js 51.8 kB / 11.2 kB 2%↑53.1 kB / 4%↑11.6 kB
Total (Includes all files) 1.9 MB / 450.7 kB 0.1%↑1.9 MB / 0.2%↑451.4 kB

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

File Before (Size / Brotli) After (Size / Brotli)
./assets/main-{hash}.js 312.7 kB / 83.7 kB 0.09%↑312.9 kB / 0.1%↑83.8 kB
Total (Includes all files) 348.1 kB / 94.6 kB 0.08%↑348.3 kB / 0.1%↑94.8 kB

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

File Before (Size / Brotli) After (Size / Brotli)
./assets/main-{hash}.js 159.1 kB / 43.8 kB 0.2%↑159.3 kB / 0.1%↑43.9 kB
Total (Includes all files) 159.4 kB / 44 kB 0.2%↑159.6 kB / 0.1%↑44 kB

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

@NullVoxPopuli NullVoxPopuli changed the title fix plain method calling [BUGFIX] enable plain method calling from templates Jun 12, 2026
@NullVoxPopuli 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 NullVoxPopuli marked this pull request as ready for review June 12, 2026 20:35

// 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);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the actual fix is here

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

more reference linking 🙈

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

Copy link
Copy Markdown
Contributor Author

from review times -- this PR needs to be re-worked to only solve {{ (ctx.called) }}

@NullVoxPopuli NullVoxPopuli marked this pull request as draft June 16, 2026 18:24
@NullVoxPopuli NullVoxPopuli force-pushed the nvp/fix-this-binding-for-real branch from f25e012 to 889278b Compare June 16, 2026 20:53
@NullVoxPopuli 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 NullVoxPopuli force-pushed the nvp/fix-this-binding-for-real branch from 889278b to 4b573d6 Compare June 16, 2026 21:00
@NullVoxPopuli NullVoxPopuli force-pushed the nvp/fix-this-binding-for-real branch from 4b573d6 to 9c29b51 Compare June 16, 2026 21:44
@NullVoxPopuli NullVoxPopuli marked this pull request as ready for review June 16, 2026 22:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants