this should not be added to the scope bag

[NullVoxPopuli]

Since we know how this works:

• we omit this from the scope bag always, as it's a syntax error to do { this }

• we should still error if this shows up in a place where this doesn't exist (outside of classes and functions)

  • I couldn't figure out how to effectively do this that didn't have a ton of footguns, so I left it as an exercise for another time.
    For example, this is an easy use case where we want to forbid this:

    export default class Link extends Helper<Signature> {
      @service declare router: RouterService;
    
      compute(
        [href]: [href: string],
        { includeActiveQueryParams = false }: { includeActiveQueryParams?: boolean | string[] }
      ) {
        assert('href was not passed in', href);
    
        const isExternal = isExternal(href);
        const router = this.router;
        const handleClick = this.handleClick;
    
        // if this is accessed here -- we should error, because the `this` available
        // is not correct
        const Element = <template>
          <a
            data-active={{ (isActive router href includeActiveQueryParams) }}
            href={{if href href "##missing##"}}
            {{on "click" handleClick}}
            ...attributes
          >
            {{yield (hash isExternal=isExternal isActive=isActive)}}
          </a>
        </template>;
    
        return { 
          isExternal, 
          get isActive() {
            return isActive(router, href, includeActiveQueryParams);
          }, 
          Element 
        };
      }
    }

    This could maybe be written more simply in the underlying APIs, getting around the this restriction altogether by forcing folks to alias the variable -- which maybe they could do in the inline version above as well 🤔

    compute(...) {
      return {
        Element: wireUpElement(this, href, includeActiveQueryParams)
         // ...
      }
    }
    // ...
    
    function wireUpElement(context, href, includeActiveQueryParams) {
      return setComponentTemplate(precompileTemplate(`
          <a
            data-active={{ (isActive context.router href includeActiveQueryParams) }}
            href={{if href href "##missing##"}}
            {{on "click" context.handleClick}}
            ...attributes
          >
            {{yield (hash isExternal=(isExternal href) isActive=isActive)}}
          </a>
      `, {
        context, on, href, includeActiveQueryParams
      }), templateOnly());
    }

    But this implies that we need to remove some caching that we have in ember right now -- because we don't want to keep component references around forever -- because wireUpElement won't care about previous component bindings next time it's called, even though the precompileTemplate part will be totally static, we're making a new binding between the scope bag and new templateOnly() instance.

    In any case, I think it warrants discussion about how to make this work ergonomically before restricting this usage arbitrarily

  That's my morning, tangential, mind spew

[patricklx]

Is there another way to reference this. I can see how it would work for component templates.
But what about template in tests. shouldn't it possible to reference this. I guess we should still support it for wire format? And only if it doesn't have a component.
Not sure when hbs format is taking place. Is it happening as a step before wire or is it for addons?

[NullVoxPopuli]

But what about template in tests. shouldn't it possible to reference this. I guess we should still support it for wire format? And only if it doesn't have a component.

in tests, you can define this:

test('...', function () {
  class Foo extends Component {
    hi = 'hi';
    
    <template>{{this.hi}}</template>
  }
}

which is valid.

[patricklx]

What about

test('...', function () {
  this.hi = b;
  tpl =  <template>{{this.hi}}</template>
}

[NullVoxPopuli]

I don't think that can be supported, setComponentTemplate would have to get a reference to that function first, and it's anonymous

[patricklx]

If this is passed with the scope it would work.
Only hbs format doesn't support it

[NullVoxPopuli]

If this is passed with the scope it would work.

well, the transform currently does scope: () => ({ this }) which is illegal, you'd have to

• rewrite all this references to context in the template
• change the this in the scope bag to scope: () => ({ context: this })

[patricklx]

If this is passed with the scope it would work.

well, the transform currently does scope: () => ({ this }) which is illegal, you'd have to

• rewrite all this references to context in the template
• change the this in the scope bag to scope: () => ({ context: this })

that is because its the hbs format.
in wire format it would be something like scope: () => [this, other]. So I think we could just disable adding this in hbs format.

babel-plugin-ember-template-compilation/__tests__/tests.ts

Line 1938 in ab7578e

scope: () => [HelloWorld],

though i'm nore sure what will happen when its in hbs format and then gets converted to wire format.
So my question is: when is it converted to hbs format. I think normally it gets converted directly to wire format?

[ef4]

What about

test('...', function () {
  this.hi = b;
  tpl =  <template>{{this.hi}}</template>
}

In the present design of components, it's intentional that that doesn't work. Because that's a template-only component and template-only components never have a this.

I don't think it's completely unreasonable to propose a change that would reclaim this unused piece of the namespace and make lexically-scoped this work. But I do think that would need some design discussion and at the very least a small amendment to the template-tag RFC. (Which is something we need to do anyway, since we've found enough cases that it's silent on.)

[ef4]

• change the this in the scope bag to scope: () => ({ context: this })

No, if we decided to allow lexically scoped this is would just be scope: () => ({ "this": this }).

But also, there is existing API for how you pass this into the template

babel-plugin-ember-template-compilation/__tests__/tests.ts

Line 1352 in ab7578e

template('<HelloWorld @color={{red}} />', { component: this, scope: () => ({ HelloWorld }) });

Disambiguating between these two and considering how it relates to the component manager API is why I'm saying this would require design work.

[patricklx]

• change the this in the scope bag to scope: () => ({ context: this })

No, if we decided to allow lexically scoped this is would just be scope: () => ({ "this": this }).

But also, there is existing API for how you pass this into the template

babel-plugin-ember-template-compilation/__tests__/tests.ts

Line 1352 in ab7578e

template('<HelloWorld @color={{red}} />', { component: this, scope: () => ({ HelloWorld }) });

Disambiguating between these two and considering how it relates to the component manager API is why I'm saying this would require design work.

That would be good.
Actually, it looks like it's different. There, the this refers to the component prototype, or constructor. Since it's inside static. So it cannot be used directly as this context.

[ef4]

Since it's inside static.

Oh yes, good point. The way that the instance this gets to the template normally is via the component manager's getContext.

So the way I'd summarize things right now is that this inside a template means whatever that component's manager says it means via getContext. In order to decide whether to bind this to the scope bag, we'd need to be able to statically know whether the manager actually wants to own this, which seems unlikely to be possible in general.

[NullVoxPopuli]

@ef4 anything blocking this PR?

we've have some great tangential discussion! 😅