[BUGFIX beta] bind all methods on the router service for use with fn …

[NullVoxPopuli]

…and default helper manager

This PR is rebased off of #20018 so review that first

This resolves an issue where this code:

{{#if (this.router.isActive 'some.route')}}

Would lose the this binding.
Discovered here

This is only an issue since the:
emberjs/rfcs#756
(and https://github.com/NullVoxPopuli/ember-functions-as-helper-polyfill/)

Explanation here

but, I'm still upset that bound methods on classes aren't default 😅
anywho, I used @action here, idk if that's the right call since action does a bunch of other stuff, too.

I also noticed that the refresh method was still marked as canary.
refresh was declared as released in 4.1: https://blog.emberjs.com/ember-4-1-released/
so I removed the feature flag.

[NullVoxPopuli]

updated

[chriskrycho]

@NullVoxPopuli mind rebasing now that #20018 is merged? That will help one of us review/land it!

[NullVoxPopuli]

@chriskrycho done! 🎉

[chriskrycho]

@NullVoxPopuli we talked about this at a Framework team meeting a few weeks ago but responding fell off of folks' radars because of the focus on EmberConf. Net, we want to solve this not by binding these but by making direct method invocation work correctly—in which case, the invocation you have here will “just work”, and importantly folks will be able to use any methods directly. An example I’ve used a few times lately that falls out of that is using methods like the relatively new Array.prototype.at:

interface Signature {
  Args: {
    names: string[];
    selectedIdx: number;
  };
}

<template type:signature={Signature}>
  <p>{{(names.at selectedIdx)}}</p>
</template>

(N.B. that specific syntax doesn't yet work; but something like it should soon.)

[NullVoxPopuli]

So does that mean the approach needed is in the VM? ie, change it to do something like names.at.call(names, selectedIdx) ?

but then what happens when you only have a reference to at? with no way of knowing what the this is?

[chriskrycho]

I mentioned this in Discord but copying it here for folks following the thread: the idea is to align it with JS. If you have the array, it works, otherwise you need to bind it—no different than in JS.

[chriskrycho]

I put this on the agenda for today’s spec group meeting (spec group == breakout session from the Framework Team meeting, where we deep dive on technical details of things, which other relevant folks are invited to as makes sense!) so we can hopefully make some progress on it.

[chriskrycho]

The second chunk of the notes from today's spec meeting notes covers the discussion on this!

[kategengler]

Is this still relevant?

[NullVoxPopuli]

Yup, here's a repro: https://limber.glimdown.com/edit?c=JIZwBAFgLlAOIC4D0SDmBLKECuAjAdAMYD2AtkgKam4UBOAViJdXfo0rNgDZdIBMABgEBGAOxgQUdDzC0KXCgDcAhgDsoAfgBQWgAb7UjMF3SKKYWHMXoKAdy3pSsYrShgAwmWeqK6sADNaMjAAcgABVBNSUjokEidiH3UQgG4HBNcwAG8wKFplQgBrCgATMABfAKDSUIiomNokPILC9FVUVPTnTJyQOmtCc0rA4PCqGka%2B2gGKTsdutxzEiqrRsPHY0mIS9H8bWk6tCgAPBbASin9lbjdCLmUQcAAJeS5iAHUXLjKTqF8S8CeBJJRZaMBgMJTGayYjYP60NLgsFgAA8fyc9z%2BAD5keCslksOgQPggnDWESAIKEKRmUKlTAhcrlZEoppUWCYig45n6XRaIA&format=glimdown

[NullVoxPopuli]

I think we're going to solve this in the VM though, and have functions auto-bound to their host object.

e.g.:

{{this.router.isActive "foo"}}
is roughly the same as

let isActive = this.router.isActive.bind(this.router);

isActive('foo');

which will be a better solve across all sorts of libraries