From c3b43d48b67a1b33b6d54cc5eb25a9cc7e182bb2 Mon Sep 17 00:00:00 2001 From: Sam Van Campenhout Date: Sat, 5 Mar 2022 20:19:01 +0100 Subject: [PATCH] Reduce the strictness of the helper keyword handling 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 | 45 +++++++++++++++-------- 2 files changed, 36 insertions(+), 26 deletions(-) diff --git a/packages/compat/src/resolver-transform.ts b/packages/compat/src/resolver-transform.ts index 229cf2aa2..97d6bdf62 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 8e435a4c9..53c42b464 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,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 () { @@ -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 () {