From 4ca87051b726912cd84d77ce6f854a5eff4863ac Mon Sep 17 00:00:00 2001 From: Sam Van Campenhout Date: Sat, 5 Mar 2022 20:19:01 +0100 Subject: [PATCH] Make the helper keyword handling less strict 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. --- packages/compat/src/resolver-transform.ts | 17 ++++----- packages/compat/tests/resolver.test.ts | 44 ++++++++++++++--------- 2 files changed, 35 insertions(+), 26 deletions(-) diff --git a/packages/compat/src/resolver-transform.ts b/packages/compat/src/resolver-transform.ts index 229cf2aa29..97d6bdf621 100644 --- a/packages/compat/src/resolver-transform.ts +++ b/packages/compat/src/resolver-transform.ts @@ -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); } } diff --git a/packages/compat/tests/resolver.test.ts b/packages/compat/tests/resolver.test.ts index 8e435a4c90..0880603024 100644 --- a/packages/compat/tests/resolver.test.ts +++ b/packages/compat/tests/resolver.test.ts @@ -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, }); @@ -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, }); @@ -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( @@ -681,6 +681,27 @@ 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'); @@ -688,7 +709,7 @@ describe('compat-resolver', function () { 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"}}`); @@ -699,7 +720,7 @@ 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 }); expect(findDependencies('templates/application.hbs', `{{helper "hello-world"}}`)).toEqual([]); }); @@ -1764,18 +1785,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 () {