Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement this-fallback-plugin #1

Merged
merged 4 commits into from May 5, 2023
Merged

Implement this-fallback-plugin #1

merged 4 commits into from May 5, 2023

Conversation

gitKrystan
Copy link
Collaborator

@gitKrystan gitKrystan commented Apr 10, 2023

How It Works

The addon registers an ember-cli-htmlbars plugin that traverses the nodes in your Ember templates and transforms them using the following logic:

For each PathExpression with a VarHead that is NOT in the local template scope:

  • If it is within node.params or a node.hash value for a CallNode (MustacheStatement | BlockStatement | ElementModifierStatement | SubExpression):

    Prefix the head with this, making it a ThisHead ("expression fallback").

    For example:

    {{! before }}
    {{global-helper property}}
    
    {{! after }}
    {{global-helper this.property}}
  • If is the path for a MustacheStatement with NO params or hash:

    • If there is a tail:

      Prefix the head with this, making it a ThisHead ("expression fallback").

      {{! before }}
      {{property.value}}
      
      {{! after }}
      {{this.property.value}}
    • If there is NO tail:

      • If the MustacheStatement is the child of an AttrNode with a name NOT starting with @:

        Wrap the invocation with the tryLookupHelper helper to determine if it is a helper at runtime and fall back to the this property if not ("ambiguous attribute fallback").

        {{! before }}
        <Parent id={{property}} />
        
        {{! after }}
        {{#let (hash property=(tryLookupHelper "property")) as |maybeHelpers|}}
          <Parent
            id={{(if
              maybeHelpers.property (maybeHelpers.property) this.property
            )}}
          />
        {{/let}}
      • Otherwise:

        Wrap the invocation with the isComponent helper to determine if it is a component at runtime and invoke it as a component if so. If not, wrap the invocation with the tryLookupHelper helper to determine if it is a helper ad runtime and fall back to the this property if not ("ambiguous statement fallback").

        {{! before }}
        {{property}}
        
        {{! after }}
        {{#if (isComponent "property")}}
          <Property />
        {{else}}
          {{#let (hash property=(tryLookupHelper "property")) as |maybeHelpers|}}
            {{(if maybeHelpers.property (maybeHelpers.property) this.property)}}
          {{/let}}
        {{/if}}

Caveats

Runtime implications

The isComponent and tryLookupHelper helpers have runtime implications that may have performance impacts. Thus, we recommend relying on this addon only temporarily to unblock 4.0+ upgrades while continuing to migrate away from reliance on the Property Fallback Lookup feature.

Embroider compatibility

In the "ambiguous attribute fallback" and "ambiguous statement fallback" cases shown above, we fall back to dynamic resolution at runtime to determine if the contents of the mustache statement point to a helper, a component, or a property on this. This technique is fundamentally incompatible with Embroider "optimized" mode, specifically the staticHelpers and staticComponents configs (and thus, splitAtRoutes), which requires that helpers are resolvable at build-time.

Thus, in these cases, we log a warning to ember-this-fallback-plugin.log. If you wish to use Embroider's "optimized" mode, we recommend manually updating the code in these cases to either:

{{! For a known property on `this`. }}
{{this.property}}

or

{{! For a known global helper. }}
{{(property)}}

or

{{! For a known global component. }}
<Property />

In the future, we could resolve this incompatibility if we had access to Embroider's static resolution.

@gitKrystan gitKrystan force-pushed the this-fallback-plugin branch 24 times, most recently from 820fa6a to 5d45656 Compare April 13, 2023 21:47
gitKrystan added a commit that referenced this pull request Apr 14, 2023
gitKrystan added a commit that referenced this pull request Apr 14, 2023
@gitKrystan gitKrystan force-pushed the this-fallback-plugin branch 2 times, most recently from 1a167b7 to 18e04c1 Compare April 17, 2023 20:06
@@ -78,10 +83,12 @@
"eslint-plugin-n": "^15.7.0",
"eslint-plugin-qunit": "^7.3.4",
"eslint-plugin-unicorn": "^46.0.0",
"jest": "^29.5.0",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I tried vitest first but the types in the current version are whack and I got impatient waiting for them to fix it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting! You found the types in jest to be better than vitest?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not really. The latest vitest version had a bunch of type bugs in it (I found related issues but don't remember what it was now) and rather than wait for them to fix and ship, I just switched to jest since the API is basically the same. Might be fixed now. I just put this comment here bc I knew you were going to suggest trying vitest. :-P

@gitKrystan gitKrystan force-pushed the this-fallback-plugin branch 2 times, most recently from a976f74 to 14ffb36 Compare April 19, 2023 18:43
@gitKrystan gitKrystan changed the title WIP Implement this-fallback-plugin Implement this-fallback-plugin Apr 19, 2023
@gitKrystan gitKrystan force-pushed the this-fallback-plugin branch 4 times, most recently from 8846032 to d3afa64 Compare April 20, 2023 18:33
@gitKrystan gitKrystan force-pushed the this-fallback-plugin branch 2 times, most recently from 3887d0c to bf9bc75 Compare April 21, 2023 22:45
app/is-helper.js Outdated
@@ -0,0 +1 @@
export { default } from 'ember-this-fallback/is-helper';
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not sure if these re-exports are:

  1. used, or
  2. correct

await render(
hbs`{{if (this-fallback/is-helper "global-helper") "true" "false"}}`
);
await render(<template>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

IDK how I would have tested this w/o ember-template-imports...fun stuff

"test:ember": "yarn build:ts && ember test && yarn clean",
"test:ember-compatibility": "yarn build:ts && ember try:each",
"prepack": "yarn build:ts && ember ts:precompile",
"postpack": "yarn clean && ember ts:clean"
},
"dependencies": {
"babel-plugin-ember-template-compilation": "^2.0.2",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I only use this for types. devDependency?

@@ -70,6 +77,7 @@
"ember-resolver": "^10.0.0",
"ember-source": "~4.11.0",
"ember-source-channel-url": "^3.0.0",
"ember-template-imports": "^3.4.2",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

only used in tests

@gitKrystan gitKrystan added the enhancement New feature or request label Apr 25, 2023
@gitKrystan gitKrystan merged commit 3a9bcec into main May 5, 2023
10 checks passed
@gitKrystan gitKrystan deleted the this-fallback-plugin branch May 5, 2023 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants