Skip to content

Commit

Permalink
Reduce the strictness of the helper keyword handling
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Windvis committed Mar 6, 2022
1 parent 2a44b73 commit c3b43d4
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 26 deletions.
17 changes: 7 additions & 10 deletions packages/compat/src/resolver-transform.ts
Expand Up @@ -329,15 +329,12 @@ function handleComponentHelper(
resolver.resolveComponentHelper(locator, moduleName, param.loc, impliedBecause);
}

function handleDynamicHelper(param: ASTv1.Node, resolver: Resolver, moduleName: string): void {
switch (param.type) {
case 'StringLiteral':
resolver.resolveDynamicHelper({ type: 'literal', path: param.value }, moduleName, param.loc);
break;
case 'TextNode':
resolver.resolveDynamicHelper({ type: 'literal', path: param.chars }, moduleName, param.loc);
break;
default:
resolver.resolveDynamicHelper({ type: 'other' }, moduleName, param.loc);
function handleDynamicHelper(param: ASTv1.Expression, resolver: Resolver, moduleName: string): void {
// We only need to handle StringLiterals since Ember already throws an error if unsupported values
// are passed to the helper keyword.
// If a helper reference is passed in we don't need to do anything since it's either the result of a previous
// helper keyword invocation, or a helper reference that was imported somewhere.
if (param.type === 'StringLiteral') {
resolver.resolveDynamicHelper({ type: 'literal', path: param.value }, moduleName, param.loc);
}
}
45 changes: 29 additions & 16 deletions packages/compat/tests/resolver.test.ts
Expand Up @@ -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 () {
let findDependencies = configure({
staticHelpers: true,
});
Expand All @@ -546,7 +546,7 @@ describe('compat-resolver', function () {
)
).toEqual([]);
});
test('built-in helpers are ignored when used with the "helper" helper', function () {
test('built-in helpers are ignored when used with the "helper" keyword', function () {
let findDependencies = configure({
staticHelpers: true,
});
Expand Down Expand Up @@ -662,7 +662,7 @@ describe('compat-resolver', function () {
},
]);
});
test('string literal passed to "helper" helper in helper position', function () {
test('string literal passed to "helper" keyword in helper position', function () {
let findDependencies = configure({ staticHelpers: true });
givenFile('helpers/hello-world.js');
expect(
Expand All @@ -681,14 +681,35 @@ describe('compat-resolver', function () {
},
]);
});
test('helper currying using the "helper" keyword', function () {
let findDependencies = configure({ staticHelpers: true });
givenFile('helpers/hello-world.js');
expect(
findDependencies(
'templates/application.hbs',
`
{{#let (helper "hello-world" name="World") as |hello|}}
{{#let (helper hello name="Tomster") as |helloTomster|}}
{{helloTomster name="Zoey"}}
{{/let}}
{{/let}}
`
)
).toEqual([
{
path: '../helpers/hello-world.js',
runtimeName: 'the-app/helpers/hello-world',
},
]);
});
test('string literal passed to component helper fails to resolve', function () {
let findDependencies = configure({ staticComponents: true });
givenFile('components/my-thing.js');
expect(() => {
findDependencies('templates/application.hbs', `{{my-thing header=(component "hello-world") }}`);
}).toThrow(new RegExp(`Missing component: hello-world in templates/application.hbs`));
});
test('string literal passed to "helper" helper fails to resolve', function () {
test('string literal passed to "helper" keyword fails to resolve', function () {
let findDependencies = configure({ staticHelpers: true });
expect(() => {
findDependencies('templates/application.hbs', `{{helper "hello-world"}}`);
Expand All @@ -699,8 +720,9 @@ describe('compat-resolver', function () {
givenFile('components/my-thing.js');
expect(findDependencies('templates/application.hbs', `{{my-thing header=(component "hello-world") }}`)).toEqual([]);
});
test('string literal passed to "helper" helper fails to resolve when staticHelpers is off', function () {
test('string literal passed to "helper" keyword fails to resolve when staticHelpers is off', function () {
let findDependencies = configure({ staticHelpers: false });
givenFile('helpers/hello-world.js');
expect(findDependencies('templates/application.hbs', `{{helper "hello-world"}}`)).toEqual([]);
});
test('dynamic component helper error in content position', function () {
Expand Down Expand Up @@ -1764,18 +1786,9 @@ describe('compat-resolver', function () {
);
});

test('rejects arbitrary expression in "helper" helper', function () {
test('ignores any non-string-literal in "helper" keyword', function () {
let findDependencies = configure({ staticHelpers: true });
expect(() => findDependencies('templates/application.hbs', `{{helper (some-helper this.which) }}`)).toThrow(
`Unsafe dynamic helper: cannot statically analyze this expression`
);
});

test('rejects any non-string-literal in "helper" helper', function () {
let findDependencies = configure({ staticHelpers: true });
expect(() => findDependencies('templates/application.hbs', `{{helper this.which }}`)).toThrow(
`Unsafe dynamic helper: cannot statically analyze this expression`
);
expect(findDependencies('templates/application.hbs', `{{helper this.which}}`)).toEqual([]);
});

test('trusts inline ensure-safe-component helper', function () {
Expand Down

0 comments on commit c3b43d4

Please sign in to comment.