From e4271757d4fab99551fd4143f3d691eff2a54ff3 Mon Sep 17 00:00:00 2001 From: "Andrey Mikhaylov (lolmaus)" Date: Tue, 6 Feb 2024 14:33:28 +0300 Subject: [PATCH] Make unambiguous helpers opt-in, fixes #514, fixes #504 --- README.md | 18 +++++ .../input/app/templates/application.hbs | 12 ++-- .../input/app/templates/application.hbs | 10 +-- transforms/angle-brackets/index.js | 1 + transforms/angle-brackets/transform.js | 39 +++++++---- transforms/angle-brackets/transform.test.js | 65 ++++++++++++------- 6 files changed, 96 insertions(+), 49 deletions(-) diff --git a/README.md b/README.md index eda22814f..508417c47 100644 --- a/README.md +++ b/README.md @@ -191,6 +191,24 @@ If you would like to only convert certain component invocations to use the angle } ``` +### Making helper invocations unambiguous + +In order to make helper invocations unambiguous, use this: + +**config/anglebrackets-codemod-config.json** + +```js +{ + "unambiguousHelpers": true +} +``` + +This will result in invocations like `{{concat "foo" "bar"}}` to be converted into `{{(concat "foo" "bar")}}`, which may be useful in strict-mode Embroider. + +Note that it does not work in non-Embroider Ember, as of January 2024. + +Note that ambiguous invocations, that cannot be statically distinguished between a helper, a property and a component, will not be modified. + ## Debugging Workflow Oftentimes, you want to debug the codemod or the transform to identify issues with the code or to understand diff --git a/test/fixtures/with-telemetry/input/app/templates/application.hbs b/test/fixtures/with-telemetry/input/app/templates/application.hbs index f665128ad..42c5e969a 100644 --- a/test/fixtures/with-telemetry/input/app/templates/application.hbs +++ b/test/fixtures/with-telemetry/input/app/templates/application.hbs @@ -3,15 +3,15 @@

Components

- {{#bs-nav type="pills" stacked=true as |nav|}} + {{#each this.model as |comp|}} - {{#nav.item}} - {{#nav.link-to route=comp.demoRoute}} + + {{comp.title}} - {{/nav.link-to}} - {{/nav.item}} + + {{/each}} - {{/bs-nav}} +
{{utils/bee-bop}} diff --git a/test/fixtures/without-telemetry/input/app/templates/application.hbs b/test/fixtures/without-telemetry/input/app/templates/application.hbs index 648a45edb..1647f3b36 100644 --- a/test/fixtures/without-telemetry/input/app/templates/application.hbs +++ b/test/fixtures/without-telemetry/input/app/templates/application.hbs @@ -1,9 +1,9 @@ -{{site-header user=this.user class=(if this.user.isAdmin "admin")}} + -{{#super-select selected=this.user.country as |s|}} + {{#each this.availableCountries as |country|}} - {{#s.option value=country}}{{country.name}}{{/s.option}} + {{country.name}} {{/each}} -{{/super-select}} + -{{ui/button text="Click me"}} + diff --git a/transforms/angle-brackets/index.js b/transforms/angle-brackets/index.js index 293a8653d..ea5a69568 100755 --- a/transforms/angle-brackets/index.js +++ b/transforms/angle-brackets/index.js @@ -31,6 +31,7 @@ function getOptions() { options.includeValuelessDataTestAttributes = !!config.includeValuelessDataTestAttributes; options.skipBuiltInComponents = !!config.skipBuiltInComponents; + options.unambiguousHelpers = !!config.unambiguousHelpers; } if (cliOptions.telemetry) { diff --git a/transforms/angle-brackets/transform.js b/transforms/angle-brackets/transform.js index 9d40f8d2d..054105b59 100755 --- a/transforms/angle-brackets/transform.js +++ b/transforms/angle-brackets/transform.js @@ -45,12 +45,16 @@ function isBuiltInComponent(key) { return BUILT_IN_COMPONENTS.includes(key); } -function isNestedComponentTagName(tagName) { - return ( - tagName && - tagName.includes && - (tagName.includes('/') || (tagName.includes('-') && tagName.includes('.'))) - ); +function isNestedComponentTagName(tagName, unambiguousHelpers = false) { + if (unambiguousHelpers) { + return ( + tagName && + tagName.includes && + (tagName.includes('/') || (tagName.includes('-') && tagName.includes('.'))) + ); + } + + return tagName && tagName.includes && (tagName.includes('/') || tagName.includes('-')); } function isWallStreet(tagName) { @@ -341,18 +345,21 @@ function isKnownHelper(fullName, config, invokableData) { } if (isTelemetryData) { - let isComponent = - !config.helpers.includes(name) && - [...(components || []), ...BUILT_IN_COMPONENTS].includes(name); + if (config.unambiguousHelpers) { + let isComponent = + !config.helpers.includes(name) && + [...(components || []), ...BUILT_IN_COMPONENTS].includes(name); - if (isComponent) { - return false; + if (isComponent) { + return false; + } } let mergedHelpers = [...KNOWN_HELPERS, ...(helpers || [])]; let isHelper = mergedHelpers.includes(name) || config.helpers.includes(name); + let isComponent = [...(components || []), ...BUILT_IN_COMPONENTS].includes(name); let strName = `${name}`; // coerce boolean and number to string - return isHelper && !strName.includes('.'); + return (isHelper || (!config.unambiguousHelpers && !isComponent)) && !strName.includes('.'); } else { return KNOWN_HELPERS.includes(name) || config.helpers.includes(name); } @@ -480,8 +487,11 @@ function transformToAngleBracket(fileInfo, config, invokableData) { const isTagKnownHelper = isKnownHelper(tagName, config, invokableData); // Don't change attribute statements - const isValidMustacheComponent = node.loc.source !== '(synthetic)' && !isTagKnownHelper; - const isNestedComponent = isNestedComponentTagName(tagName); + const isValidMustacheComponent = config.unambiguousHelpers + ? node.loc.source !== '(synthetic)' && !isTagKnownHelper + : node.loc.source !== '(synthetic)' && !isKnownHelper(tagName, config, invokableData); + + const isNestedComponent = isNestedComponentTagName(tagName, config.unambiguousHelpers); if ( isValidMustacheComponent && @@ -489,6 +499,7 @@ function transformToAngleBracket(fileInfo, config, invokableData) { ) { return transformComponentNode(node, fileInfo, config); } else if ( + config.unambiguousHelpers && isTagKnownHelper && node.path.type !== 'SubExpression' && walkerPath.parent.node.type !== 'AttrNode' && diff --git a/transforms/angle-brackets/transform.test.js b/transforms/angle-brackets/transform.test.js index 15daa3f85..f6379eb62 100644 --- a/transforms/angle-brackets/transform.test.js +++ b/transforms/angle-brackets/transform.test.js @@ -476,7 +476,7 @@ test('let', () => { {{#let (capitalize this.person.firstName) (capitalize this.person.lastName) as |firstName lastName| }} - Welcome back {{(concat firstName ' ' lastName)}} + Welcome back {{concat firstName ' ' lastName}} Account Details: First Name: {{firstName}} @@ -554,8 +554,8 @@ test('link-to-inline', () => { Segments Segments {{segment.name}} - {{(t \\"show\\")}} - {{(t \\"show\\")}} + {{t \\"show\\"}} + {{t \\"show\\"}} Show Show Show @@ -839,7 +839,7 @@ test('t-helper', () => { expect(runTest('t-helper.hbs', input)).toMatchInlineSnapshot(` " - {{(t \\"some.string\\" param=\\"string\\" another=1)}} + {{t \\"some.string\\" param=\\"string\\" another=1}} " `); }); @@ -878,7 +878,7 @@ test('tilde', () => { expect(runTest('tilde.hbs', input)).toMatchInlineSnapshot(` " {{#if foo~}} - {{some-component}} + {{/if}} " `); @@ -977,7 +977,7 @@ test('skip-default-helpers', () => { expect(runTest('skip-default-helpers.hbs', input, options)).toMatchInlineSnapshot(` "
- {{(liquid-outlet)}} + {{liquid-outlet}}
@@ -997,11 +997,11 @@ test('skip-default-helpers', () => { Two
{{/liquid-if}} - {{(moment '12-25-1995' 'MM-DD-YYYY')}} - {{(moment-from '1995-12-25' '2995-12-25' hideAffix=true)}} + {{moment '12-25-1995' 'MM-DD-YYYY'}} + {{moment-from '1995-12-25' '2995-12-25' hideAffix=true}} - {{(some-helper1 foo=true)}} - {{(some-helper2 foo=true)}} + {{some-helper1 foo=true}} + {{some-helper2 foo=true}} " `); }); @@ -1039,7 +1039,7 @@ test('skip-default-helpers (no-config)', () => { expect(runTest('skip-default-helpers.hbs', input)).toMatchInlineSnapshot(` "
- {{(liquid-outlet)}} + {{liquid-outlet}}
@@ -1059,8 +1059,8 @@ test('skip-default-helpers (no-config)', () => { Two {{/liquid-if}} - {{(moment '12-25-1995' 'MM-DD-YYYY')}} - {{(moment-from '1995-12-25' '2995-12-25' hideAffix=true)}} + {{moment '12-25-1995' 'MM-DD-YYYY'}} + {{moment-from '1995-12-25' '2995-12-25' hideAffix=true}} @@ -1086,8 +1086,8 @@ test('custom-options', () => { expect(runTest('custom-options.hbs', input, options)).toMatchInlineSnapshot(` " - {{(some-helper1 foo=true)}} - {{(some-helper2 foo=true)}} + {{some-helper1 foo=true}} + {{some-helper2 foo=true}} {{link-to \\"Title\\" \\"some.route\\"}} {{textarea value=this.model.body}} {{input type=\\"checkbox\\" name=\\"email-opt-in\\" checked=this.model.emailPreference}} @@ -1180,7 +1180,7 @@ test('preserve arguments', () => { " {{foo-bar data-baz class=\\"baz\\"}} - {{(t \\"show\\")}} + {{t \\"show\\"}} " `); }); @@ -1284,9 +1284,9 @@ test('wallstreet-telemetry', () => { expect(runTest('wallstreet-telemetry.hbs', input)).toMatchInlineSnapshot(` " {{nested$helper}} - {{(nested::helper)}} - {{(nested::helper param=\\"yeah!\\")}} - {{(helper-1)}} + {{nested::helper}} + {{nested::helper param=\\"yeah!\\"}} + {{helper-1}} " `); }); @@ -1294,16 +1294,16 @@ test('wallstreet-telemetry', () => { test('wrapping-helpers-with-parens', () => { let input = ` {{fooknownhelper}} - {{(fooknownhelper)}} + {{fooknownhelper}} {{fooknownhelper data-test-foo foo="bar"}} {{foounknownhelper}} `; expect(runTest('wrapping-helpers-with-parens.hbs', input)).toMatchInlineSnapshot(` " - {{(fooknownhelper)}} - {{(fooknownhelper)}} - {{(fooknownhelper data-test-foo foo=\\"bar\\")}} + {{fooknownhelper}} + {{fooknownhelper}} + {{fooknownhelper data-test-foo foo=\\"bar\\"}} {{foounknownhelper}} " `); @@ -1382,7 +1382,24 @@ test('unknown helper with args', () => { " {{api-reference someArg}} - {{api-reference}} + + " + `); +}); + +test('unambiguousHelpers: true', () => { + let input = ` + {{concat}} + {{unknown}} + {{t "some.string" param="string" another=1}} + `; + + expect(runTest('unambiguousHelpers: true', input, { unambiguousHelpers: true })) + .toMatchInlineSnapshot(` + " + {{(concat)}} + {{unknown}} + {{(t \\"some.string\\" param=\\"string\\" another=1)}} " `); });