From 882c956ccdf922033733906fa75f1a379f438603 Mon Sep 17 00:00:00 2001 From: "Andrey Mikhaylov (lolmaus)" Date: Tue, 6 Feb 2024 14:33:28 +0300 Subject: [PATCH 1/6] Make unambiguous helpers opt-in, fixes #514, fixes #504 --- README.md | 18 +++++ .../input/app/templates/application.hbs | 16 ++--- .../app/templates/components/file-less.hbs | 2 +- .../output/app/templates/application.hbs | 8 +-- .../app/templates/components/foo-bar.hbs | 4 +- .../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 ++++++++++++------- 9 files changed, 105 insertions(+), 58 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..c0dba4462 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}} @@ -28,13 +28,13 @@ {{/if}} {{outlet}} - {{#bs-button id="openModal" onClick=(action this.addModal)}}Open{{/bs-button}} + {{#bs-button id="openModal" onClick=(action "addModal")}}Open{{/bs-button}} {{#if hasModal}} {{#bs-modal-simple open=modal onHidden=(action "removeModal") title="Dynamic Dialog"}} Hi there {{/bs-modal-simple}} {{/if}} - {{file-less foo=true}} +
diff --git a/test/fixtures/with-telemetry/input/app/templates/components/file-less.hbs b/test/fixtures/with-telemetry/input/app/templates/components/file-less.hbs index b9c03c0c8..4c50d5d9c 100644 --- a/test/fixtures/with-telemetry/input/app/templates/components/file-less.hbs +++ b/test/fixtures/with-telemetry/input/app/templates/components/file-less.hbs @@ -1,2 +1,2 @@
this template has no js
-{{#bs-button type="primary"}}Primary{{/bs-button}} +Primary diff --git a/test/fixtures/with-telemetry/output/app/templates/application.hbs b/test/fixtures/with-telemetry/output/app/templates/application.hbs index 077434de3..552feedef 100644 --- a/test/fixtures/with-telemetry/output/app/templates/application.hbs +++ b/test/fixtures/with-telemetry/output/app/templates/application.hbs @@ -14,9 +14,9 @@
- {{(utils/bee-bop)}} - {{(-wat-wat)}} - {{(utils/-wat-wat)}} + + {{-wat-wat}} + {{#if this.isDetailPage}}

{{currentComponent.title}} @@ -28,7 +28,7 @@ {{/if}} {{outlet}} - Open + Open {{#if hasModal}} diff --git a/test/fixtures/with-telemetry/output/app/templates/components/foo-bar.hbs b/test/fixtures/with-telemetry/output/app/templates/components/foo-bar.hbs index 44dd7b86b..47fb0ecdd 100644 --- a/test/fixtures/with-telemetry/output/app/templates/components/foo-bar.hbs +++ b/test/fixtures/with-telemetry/output/app/templates/components/foo-bar.hbs @@ -1,4 +1,4 @@ -{{(biz-baz canConvert="no" why="helper" where="local")}} +{{biz-baz canConvert="no" why="helper" where="local"}} 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)}} " `); }); From 15a1365ac4f7fef5a5a2c70c0e3971079e5f594e Mon Sep 17 00:00:00 2001 From: "Andrey Mikhaylov (lolmaus)" Date: Wed, 7 Feb 2024 16:23:01 +0300 Subject: [PATCH 2/6] Unambiguous helpers: simplify logic --- transforms/angle-brackets/transform.js | 18 +++++++----------- transforms/angle-brackets/transform.test.js | 2 +- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/transforms/angle-brackets/transform.js b/transforms/angle-brackets/transform.js index 054105b59..04eb5698a 100755 --- a/transforms/angle-brackets/transform.js +++ b/transforms/angle-brackets/transform.js @@ -45,16 +45,12 @@ function isBuiltInComponent(key) { return BUILT_IN_COMPONENTS.includes(key); } -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 isNestedComponentTagName(tagName) { + return ( + tagName && + tagName.includes && + (tagName.includes('/') || (tagName.includes('-') && tagName.includes('.'))) + ); } function isWallStreet(tagName) { @@ -491,7 +487,7 @@ function transformToAngleBracket(fileInfo, config, invokableData) { ? node.loc.source !== '(synthetic)' && !isTagKnownHelper : node.loc.source !== '(synthetic)' && !isKnownHelper(tagName, config, invokableData); - const isNestedComponent = isNestedComponentTagName(tagName, config.unambiguousHelpers); + const isNestedComponent = isNestedComponentTagName(tagName); if ( isValidMustacheComponent && diff --git a/transforms/angle-brackets/transform.test.js b/transforms/angle-brackets/transform.test.js index f6379eb62..af03ac45c 100644 --- a/transforms/angle-brackets/transform.test.js +++ b/transforms/angle-brackets/transform.test.js @@ -878,7 +878,7 @@ test('tilde', () => { expect(runTest('tilde.hbs', input)).toMatchInlineSnapshot(` " {{#if foo~}} - + {{some-component}} {{/if}} " `); From e4b480ac0179d0d2141e498d17334d33320ccc61 Mon Sep 17 00:00:00 2001 From: "Andrey Mikhaylov (lolmaus)" Date: Wed, 7 Feb 2024 16:23:21 +0300 Subject: [PATCH 3/6] Fix a test input template --- .../with-telemetry/input/app/templates/components/file-less.hbs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/fixtures/with-telemetry/input/app/templates/components/file-less.hbs b/test/fixtures/with-telemetry/input/app/templates/components/file-less.hbs index 4c50d5d9c..b9c03c0c8 100644 --- a/test/fixtures/with-telemetry/input/app/templates/components/file-less.hbs +++ b/test/fixtures/with-telemetry/input/app/templates/components/file-less.hbs @@ -1,2 +1,2 @@
this template has no js
-Primary +{{#bs-button type="primary"}}Primary{{/bs-button}} From 37c3bce1c7a5179a87838ac7285e92d4c09ec0c9 Mon Sep 17 00:00:00 2001 From: "Andrey Mikhaylov (lolmaus)" Date: Wed, 7 Feb 2024 16:23:40 +0300 Subject: [PATCH 4/6] Show diff of failing integration tests --- package.json | 1 + pnpm-lock.yaml | 41 ++++++++++++++++++++++++++++++++++++++--- test/run-test.js | 3 ++- 3 files changed, 41 insertions(+), 4 deletions(-) diff --git a/package.json b/package.json index 989a091ec..ce66b4937 100644 --- a/package.json +++ b/package.json @@ -42,6 +42,7 @@ "yargs": "^17.7.2" }, "devDependencies": { + "compare-fixture": "^1.1.0", "coveralls": "^3.1.1", "eslint": "^8.22.0", "eslint-config-prettier": "^8.5.0", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index b335425d3..535f838f5 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -25,6 +25,9 @@ dependencies: version: 17.7.2 devDependencies: + compare-fixture: + specifier: ^1.1.0 + version: 1.1.0 coveralls: specifier: ^3.1.1 version: 3.1.1 @@ -2083,7 +2086,6 @@ packages: /@types/minimatch@3.0.5: resolution: {integrity: sha512-Klz949h02Gz2uZCMGwDUSDS1YBlTdDDgbWHi+81l29tQALUtvz4rAYi5uoVhE5Lagoq6DeqAUlbrHvW/mXDgdQ==} - dev: false /@types/node@20.10.3: resolution: {integrity: sha512-XJavIpZqiXID5Yxnxv3RUDKTN5b81ddNC3ecsA0SoFXz/QU8OGBwZGMomiq0zw+uuqbL/krztv/DINAQ/EV4gg==} @@ -2913,6 +2915,14 @@ packages: resolution: {integrity: sha512-W9pAhw0ja1Edb5GVdIF1mjZw/ASI0AlShXM83UUGe2DVr5TdAPEA1OA8m/g8zWp9x6On7gqufY+FatDbC3MDQg==} dev: false + /compare-fixture@1.1.0: + resolution: {integrity: sha512-xAfzI5+Xin4fTixi3A4LzA5bI/zU0aT6+Kf1BX3fdkOAaUvzpVJxRbACR05VH+CfQfKZPamqV87PZWBQhCa8NQ==} + hasBin: true + dependencies: + mocha-diff: 1.0.2 + walk-sync: 3.0.0 + dev: true + /component-emitter@1.3.1: resolution: {integrity: sha512-T0+barUSQRTUQASh8bx02dl+DhF54GtIDY13Y3m9oWTklKbb3Wv974meRpeZ3lp1JpLVECWWNHC4vaG2XHXouQ==} dev: false @@ -3166,6 +3176,11 @@ packages: engines: {node: ^12.13.0 || ^14.15.0 || ^16.10.0 || >=17.0.0} dev: true + /diff@5.1.0: + resolution: {integrity: sha512-D+mk+qE8VC/PAUrlAU34N+VfXev0ghe5ywmpqrawphmVZc1bEfn56uo9qpyGp1p4xpzOHkSW4ztBd6L7Xx4ACw==} + engines: {node: '>=0.3.1'} + dev: true + /dir-glob@3.0.1: resolution: {integrity: sha512-WkrWp9GR4KXfKGYzOLmTuGVi1UWFfws377n9cc55/tb6DuqyF6pcQ5AbiHEshaDpY9v6oaSr2XCDidGmMwdzIA==} engines: {node: '>=8'} @@ -3290,7 +3305,6 @@ packages: /ensure-posix-path@1.1.1: resolution: {integrity: sha512-VWU0/zXzVbeJNXvME/5EmLuEj2TauvoaTz6aFYK1Z92JCBlDlZ3Gu0tuGR42kpW1754ywTs+QB0g5TP0oj9Zaw==} - dev: false /err-code@1.1.2: resolution: {integrity: sha512-CJAN+O0/yA1CKfRn9SXOGctSpEM7DCon/r/5r2eXFMY2zCCJBasFhcM5I+1kh3Ap11FsQCX+vGHceNPvpWKhoA==} @@ -5413,7 +5427,6 @@ packages: dependencies: '@types/minimatch': 3.0.5 minimatch: 3.1.2 - dev: false /mem@4.3.0: resolution: {integrity: sha512-qX2bG48pTqYRVmDB37rn/6PT7LcR8T7oAX3bf99u1Tt1nzxYfxkgqDwUwolPlXweM0XzBOBFzSx4kfp7KP1s/w==} @@ -5533,6 +5546,13 @@ packages: dependencies: minimist: 1.2.8 + /mocha-diff@1.0.2: + resolution: {integrity: sha512-LJXN9eSTwVTPzo4Ja6Z8CuxcjK9HYE17J+3+0KxCwHmJeOPBb6v+YtXQRg53NCHO/lrCvRgYhAMw2ubkZTueYA==} + dependencies: + diff: 5.1.0 + supports-color: 9.4.0 + dev: true + /move-concurrently@1.0.1: resolution: {integrity: sha512-hdrFxZOycD/g6A6SoI2bB5NA/5NEqD0569+S47WZhPvm46sD50ZHdYaFmnua5lndde9rCHGjmfK7Z8BuCt/PcQ==} dependencies: @@ -6956,6 +6976,11 @@ packages: has-flag: 4.0.0 dev: true + /supports-color@9.4.0: + resolution: {integrity: sha512-VL+lNrEoIXww1coLPOmiEmK/0sGigko5COxI09KzHc2VJXJsQ37UaQ+8quuxjDeA7+KnLGTWRyOXSLLR2Wb4jw==} + engines: {node: '>=12'} + dev: true + /supports-hyperlinks@2.3.0: resolution: {integrity: sha512-RpsAZlpWcDwOPQA22aCH4J0t7L8JmAvsCxfOSEwm7cQs3LshN36QaTkwd70DnBOXDWGssw2eUoc8CaRWT0XunA==} engines: {node: '>=8'} @@ -7372,6 +7397,16 @@ packages: minimatch: 3.1.2 dev: false + /walk-sync@3.0.0: + resolution: {integrity: sha512-41TvKmDGVpm2iuH7o+DAOt06yyu/cSHpX3uzAwetzASvlNtVddgIjXIb2DfB/Wa20B1Jo86+1Dv1CraSU7hWdw==} + engines: {node: 10.* || >= 12.*} + dependencies: + '@types/minimatch': 3.0.5 + ensure-posix-path: 1.1.1 + matcher-collection: 2.0.1 + minimatch: 3.1.2 + dev: true + /walker@1.0.8: resolution: {integrity: sha512-ts/8E8l5b7kY0vlWLewOkDXMmPdLcVV4GmOQLyxuSswIJsweeFZtAsMF7k1Nszz+TYBQrlYRmzOnr398y1JemQ==} dependencies: diff --git a/test/run-test.js b/test/run-test.js index 1db6f569c..c69839ce8 100644 --- a/test/run-test.js +++ b/test/run-test.js @@ -45,7 +45,8 @@ const execOpts = { cwd: inputDir, stderr: 'inherit' }; console.log('comparing results'); try { - await execa('diff', ['-rq', './app', '../output/app'], execOpts); + const compareFixture = await import('compare-fixture'); + compareFixture.default(path.join(inputDir, 'app'), path.join(inputDir, '../output/app')); } catch (e) { console.error('codemod did not run successfully'); console.log(e); From 208d697c0822c8eb9ddee20bf11985ec464fc947 Mon Sep 17 00:00:00 2001 From: "Andrey Mikhaylov (lolmaus)" Date: Thu, 8 Feb 2024 16:25:00 +0300 Subject: [PATCH 5/6] Fix telemetry consumption: new components from @ember namespace --- .../angle-brackets/telemetry/invokable.js | 18 +++++++++++++++++- transforms/angle-brackets/transform.js | 17 +++++++---------- 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/transforms/angle-brackets/telemetry/invokable.js b/transforms/angle-brackets/telemetry/invokable.js index f80ceed48..663abd021 100644 --- a/transforms/angle-brackets/telemetry/invokable.js +++ b/transforms/angle-brackets/telemetry/invokable.js @@ -4,7 +4,23 @@ const HELPER = 'Helper'; const COMPONENT = 'Component'; function invokableName(name, type) { - let invokePath = type === HELPER ? '/helpers/' : '/components/'; + let invokePath; + + if (name.startsWith('@ember/component/')) { + invokePath = '@ember/component/'; + } else if (name.startsWith('@ember/routing/')) { + invokePath = '@ember/routing/'; + } else if (type === HELPER) { + invokePath = '/helpers/'; + } else { + invokePath = '/components/'; + } + + console.log({ + name, + invokePath, + result: name.substring(name.lastIndexOf(invokePath) + invokePath.length, name.length), + }); return name.substring(name.lastIndexOf(invokePath) + invokePath.length, name.length); } diff --git a/transforms/angle-brackets/transform.js b/transforms/angle-brackets/transform.js index 04eb5698a..18fbca425 100755 --- a/transforms/angle-brackets/transform.js +++ b/transforms/angle-brackets/transform.js @@ -349,13 +349,13 @@ function isKnownHelper(fullName, config, invokableData) { if (isComponent) { return false; } + } else { + 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 || !isComponent) && !strName.includes('.'); } - - 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 || (!config.unambiguousHelpers && !isComponent)) && !strName.includes('.'); } else { return KNOWN_HELPERS.includes(name) || config.helpers.includes(name); } @@ -483,10 +483,7 @@ function transformToAngleBracket(fileInfo, config, invokableData) { const isTagKnownHelper = isKnownHelper(tagName, config, invokableData); // Don't change attribute statements - const isValidMustacheComponent = config.unambiguousHelpers - ? node.loc.source !== '(synthetic)' && !isTagKnownHelper - : node.loc.source !== '(synthetic)' && !isKnownHelper(tagName, config, invokableData); - + const isValidMustacheComponent = node.loc.source !== '(synthetic)' && !isTagKnownHelper; const isNestedComponent = isNestedComponentTagName(tagName); if ( From ab2a93cad3975bd23f170907f73ca851624a9cea Mon Sep 17 00:00:00 2001 From: "Andrey Mikhaylov (lolmaus)" Date: Wed, 14 Feb 2024 18:14:48 +0300 Subject: [PATCH 6/6] Remove glimmer components from tests as telemetry helpers do not report glimmer components --- .eslintrc.js | 2 +- README.md | 10 +++--- .../input/app/templates/application.hbs | 35 +++++-------------- .../app/templates/components/file-less.hbs | 1 - .../output/app/templates/application.hbs | 21 +---------- .../app/templates/components/file-less.hbs | 1 - .../angle-brackets/telemetry/invokable.js | 5 --- transforms/angle-brackets/transform.js | 5 +++ transforms/angle-brackets/transform.test.js | 12 +++---- 9 files changed, 24 insertions(+), 68 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index 3bc604e7d..c9a0cdd47 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -1,7 +1,7 @@ module.exports = { root: true, parserOptions: { - ecmaVersion: 2017, + ecmaVersion: 'latest', }, env: { node: true, diff --git a/README.md b/README.md index 508417c47..b1f42e7fb 100644 --- a/README.md +++ b/README.md @@ -193,9 +193,9 @@ 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: +You may want to convert invocations like `{{concat "foo" "bar"}}` into `{{(concat "foo" "bar")}}`, which may be useful as a temporary step when upgrading to strict-mode Embroider. -**config/anglebrackets-codemod-config.json** +In your **config/anglebrackets-codemod-config.json**, add this: ```js { @@ -203,11 +203,9 @@ In order to make helper invocations unambiguous, use this: } ``` -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 unambiguous helpers do 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. +Note that ambiguous invocations that cannot be statically distinguished between a helper, a property and a component — will not be modified. ## Debugging Workflow diff --git a/test/fixtures/with-telemetry/input/app/templates/application.hbs b/test/fixtures/with-telemetry/input/app/templates/application.hbs index c0dba4462..332dd51e4 100644 --- a/test/fixtures/with-telemetry/input/app/templates/application.hbs +++ b/test/fixtures/with-telemetry/input/app/templates/application.hbs @@ -3,38 +3,19 @@

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}} {{-wat-wat}} - {{utils/-wat-wat}} - {{#if this.isDetailPage}} -

- {{currentComponent.title}} -

-

- {{currentComponent.description}} -

- {{api-reference component=this.currentComponent}} - {{/if}} - {{outlet}} - - {{#bs-button id="openModal" onClick=(action "addModal")}}Open{{/bs-button}} - - {{#if hasModal}} - {{#bs-modal-simple open=modal onHidden=(action "removeModal") title="Dynamic Dialog"}} - Hi there - {{/bs-modal-simple}} - {{/if}} - + {{outlet}} + {{file-less foo=true}}
diff --git a/test/fixtures/with-telemetry/input/app/templates/components/file-less.hbs b/test/fixtures/with-telemetry/input/app/templates/components/file-less.hbs index b9c03c0c8..6c801e1d9 100644 --- a/test/fixtures/with-telemetry/input/app/templates/components/file-less.hbs +++ b/test/fixtures/with-telemetry/input/app/templates/components/file-less.hbs @@ -1,2 +1 @@
this template has no js
-{{#bs-button type="primary"}}Primary{{/bs-button}} diff --git a/test/fixtures/with-telemetry/output/app/templates/application.hbs b/test/fixtures/with-telemetry/output/app/templates/application.hbs index 552feedef..87bcba4c9 100644 --- a/test/fixtures/with-telemetry/output/app/templates/application.hbs +++ b/test/fixtures/with-telemetry/output/app/templates/application.hbs @@ -14,27 +14,8 @@
- {{-wat-wat}} - - {{#if this.isDetailPage}} -

- {{currentComponent.title}} -

-

- {{currentComponent.description}} -

- - {{/if}} - {{outlet}} - - Open - - {{#if hasModal}} - - Hi there - - {{/if}} + {{outlet}}
diff --git a/test/fixtures/with-telemetry/output/app/templates/components/file-less.hbs b/test/fixtures/with-telemetry/output/app/templates/components/file-less.hbs index 4c50d5d9c..6c801e1d9 100644 --- a/test/fixtures/with-telemetry/output/app/templates/components/file-less.hbs +++ b/test/fixtures/with-telemetry/output/app/templates/components/file-less.hbs @@ -1,2 +1 @@
this template has no js
-Primary diff --git a/transforms/angle-brackets/telemetry/invokable.js b/transforms/angle-brackets/telemetry/invokable.js index 663abd021..62a3f8545 100644 --- a/transforms/angle-brackets/telemetry/invokable.js +++ b/transforms/angle-brackets/telemetry/invokable.js @@ -16,11 +16,6 @@ function invokableName(name, type) { invokePath = '/components/'; } - console.log({ - name, - invokePath, - result: name.substring(name.lastIndexOf(invokePath) + invokePath.length, name.length), - }); return name.substring(name.lastIndexOf(invokePath) + invokePath.length, name.length); } diff --git a/transforms/angle-brackets/transform.js b/transforms/angle-brackets/transform.js index 18fbca425..8d94df81f 100755 --- a/transforms/angle-brackets/transform.js +++ b/transforms/angle-brackets/transform.js @@ -349,6 +349,11 @@ function isKnownHelper(fullName, config, invokableData) { if (isComponent) { return false; } + + let mergedHelpers = [...KNOWN_HELPERS, ...(helpers || [])]; + let isHelper = mergedHelpers.includes(name) || config.helpers.includes(name); + let strName = `${name}`; // coerce boolean and number to string + return isHelper && !strName.includes('.'); } else { let mergedHelpers = [...KNOWN_HELPERS, ...(helpers || [])]; let isHelper = mergedHelpers.includes(name) || config.helpers.includes(name); diff --git a/transforms/angle-brackets/transform.test.js b/transforms/angle-brackets/transform.test.js index af03ac45c..eec96619c 100644 --- a/transforms/angle-brackets/transform.test.js +++ b/transforms/angle-brackets/transform.test.js @@ -1382,24 +1382,22 @@ test('unknown helper with args', () => { " {{api-reference someArg}} - + {{api-reference}} " `); }); test('unambiguousHelpers: true', () => { let input = ` - {{concat}} - {{unknown}} - {{t "some.string" param="string" another=1}} + {{helper-1}} + {{nested/helper "some.string" param="string" another=1}} `; expect(runTest('unambiguousHelpers: true', input, { unambiguousHelpers: true })) .toMatchInlineSnapshot(` " - {{(concat)}} - {{unknown}} - {{(t \\"some.string\\" param=\\"string\\" another=1)}} + {{(helper-1)}} + {{(nested/helper \\"some.string\\" param=\\"string\\" another=1)}} " `); });