PrecompileTemplate with scope that has properties with different key and value

[candunaj]

Background

This PR solves a bug in precompileTemplate scope. precompileTemplate expects that the scope is a function that returns an object with a few properties where keys and values are equal, as in the following example.

import RootSelect from '../different/select.js';
import Select from '../select.js';

precompileTemplate(`
    <RootSelect/>
    <Select/>
    <button {{on "click" this.toogleClicked}}>
      {{#if this.isClicked}}
        Clicked
      {{else}}
        Not clicked
      {{/if}}
    </button>
  `, {
  strictMode: true,
  scope: () => ({
    RootSelect
    Select,
    on
  });

But when the project is built with the rollup, imports can be renamed. For example, Select can be renamed to Select$1, as shown in the following example.

import Select from '../different/select.js';
import Select$1 from '../select.js';

precompileTemplate(`
    <RootSelect/>
    <Select/>
    <button {{on "click" this.toogleClicked}}>
      {{#if this.isClicked}}
        Clicked
      {{else}}
        Not clicked
      {{/if}}
    </button>
  `, {
  strictMode: true,
  scope: () => ({
    RootSelect: Select
    Select: Select$1,
    on
  });

From a javascript point of view, everything is correct, and both examples are equivalent. But the second example will fail with an error.

I have created a simple example where you can see how an import is renamed by rollup and then there is a problem

Solution

When a GJS component is built in V2 addon, there is precompileTemplate function in the file in the dist dir. Imported components used in scope can be renamed by Rollup and then properties have different keys and values as shown below:

import Select from '../different/select.js';
import Select$1 from '../select.js';

precompileTemplate(`
    <RootSelect/>
    <Select/>
    <button {{on "click" this.toogleClicked}}>
      {{#if this.isClicked}}
        Clicked
      {{else}}
        Not clicked
      {{/if}}
    </button>
  `, {
  strictMode: true,
  scope: () => ({
    RootSelect: Select,
    Select: Select$1,
    on
  })

When the application is built, the ember-auto-import loads V2 addons with babel-plugin-ember-template-compilation and replaces the precompileTemplate with _createTemplateFactory as shown below:

import Select from '../different/select.js';
import Select$1 from '../select.js';

_createTemplateFactory({
      "id":"4QKqJqnE",
      "block":"[[[1,\\"\\\\n    \\"],[8,[32,0],null,null,null],[1,\\"\\\\n    \\"],[8,[32,1],null,null,null],[1,\\"\\\\n    \\"],[11,\\"button\\"],[4,    [32,2],[\\"click\\",[30,0,[\\"toogleClicked\\"]]],null],[12],[1,\\"\\\\n\\"],[41,[30,0,[\\"isClicked\\"]],[[[1,\\"        Clicked\\\\n\\"]],[]],[[[1,\\"        Not clicked\\\\n\\"]],[]]],[1,\\"    \\"],[13],[1,\\"\\\\n  \\"]],[],false,[\\"if\\"]]",
      "moduleName":"(unknown template module)",
      "scope":()=>[RootSelect,Select,on],
      "isStrictMode":true
})

The problem is that the scope arrow function returns the original names of the properties. It can be solved by wrapping glimmer wire format in a closure function shown below:

import Select from '../different/select.js';
import Select$1 from '../select.js';

_createTemplateFactory(
    ((RootSelect, Select, on)=>
    ({
      "id":"4QKqJqnE",
      "block":"[...]",
      "moduleName":"(unknown template module)",
      "scope":()=>[RootSelect,Select,on],
      "isStrictMode":true
    }))(Select, Select$1, on)
)

[ef4]

I agree we need to make this case legal. I don't think this is the right implementation.

We should be able to handle the remapping entirely at build time with no runtime closure.

Since we already parse the ember template compiler's output expression into an AST, we could precisely modify that AST to replace the inner identifiers with the outer identifiers.

[ef4]

The intent of this type was to reflect the real params that the ember template compiler accepts. We can't just add new stuff here.

It's fine to introduce a new type instead for stuff that we handle entirely in this project and never pass into ember-template-compiler.

[candunaj]

Thank you, I am going to address this issue.

[candunaj]

I agree we need to make this case legal. I don't think this is the right implementation.

We should be able to handle the remapping entirely at build time with no runtime closure.

Since we already parse the ember template compiler's output expression into an AST, we could precisely modify that AST to replace the inner identifiers with the outer identifiers.

Thank you for the suggestion. I am going to modify that AST.

[ef4]

Thanks. I made three changes on top of yours.

1. I made a small simplification that might make us more robust against future ember changes to the wire format: we visit the Identifier nodes directly and remap them wherever they might appear in the expression.

2. In source-to-source mode, we needed to update the code that re-emits the scope object to account for possible renaming. See added test to see why.

3. There's a tricky thing that happens in our current implementation, which is that when people use the JSUtils API to add new things to the scope, we need to mutate the original locals array that we already passed to the ember-template-compiler. I don't want that locals array to be out-of-sync with our own localsWithNames, because that seems likely to lead to future bugs when someone else modifiers this code again. So I introduced an explicit ScopeLocals type that can present consistent state to both ember-template-compiler's locals and our own scope name mapping info.

[ef4]

Released in 2.0.1