Make the helper keyword handling less strict #1150
Conversation
| @@ -519,7 +519,7 @@ describe('compat-resolver', function () { | |||
| }, | |||
| ]); | |||
| }); | |||
| test('string literal passed to `helper` helper in content position', function () { | |||
| test('string literal passed to "helper" keyword in content position', function () { | |||
There was a problem hiding this comment.
I started renaming "helper" to "keyword" since that's the word they use in the error in ember-source:
Uncaught Error: Assertion Failed: Passing a dynamic string to the
(helper)keyword is disallowed. (You specified(helper (concat "hel" "lo"))and(concat "hel" "lo")evaluated into "hello".) This ensures we can statically analyze the template and determine which helpers are used. If the helper name is always the same, use a string literal instead, i.e.(helper "hello"). Otherwise, import the helpers into JavaScript and pass them to the helper keyword. See https://github.com/emberjs/rfcs/blob/master/text/0496-handlebars-strict-mode.md#4-no-dynamic-resolution for details. ('contextual-built-ins/templates/application.hbs' @ L39:C0)
I assumed "keyword" was an implementation detail and not an end-user facing term, but it seems it's used in error messages so I think we can be consistent with that?
There was a problem hiding this comment.
Yes, I agree, it seems clearer to say "keyword"
| findDependencies( | ||
| 'templates/application.hbs', | ||
| ` | ||
| {{#let (helper "hello-world" name="World") as |hello|}} |
There was a problem hiding this comment.
This is based on the code snippet in the RFC.
| break; | ||
| default: | ||
| resolver.resolveDynamicHelper({ type: 'other' }, moduleName, param.loc); | ||
| function handleDynamicHelper(param: ASTv1.Expression, resolver: Resolver, moduleName: string): void { |
There was a problem hiding this comment.
I'm new to TypeScript but I think Expression is the correct type here since node.params seems to be typed as Expression[]? Expression doesn't include the TextNode type, so that doesn't need to be covered anymore.
(I think that's also the case for the component helper version).
The current implementation throws an error if you try to curry helpers using the helper keyword. This reduces the strictness of the resolver since Ember itself catches the cases were users provide unsupported dynamic values to the helper keyword.
Windvis
force-pushed
the
fix/helper-keyword-currying
branch
from
4ca8705 to
c3b43d4
Compare
The lazy css doesn't get a deterministic ordering
The current implementation throws an error if you try to curry helpers using the helper keyword. This reduces the strictness of the resolver since Ember itself catches the cases were users provide unsupported dynamic values to the helper keyword.
This PR is a continuation of the discussion in the previous PR: #1120 (comment)