Merge pull request #515 from lolmaus/opt-in-for-unambiguous-helpers

mansona · web-flow · commit a715623d8e6d · 2024-02-14T17:20:26.000Z
Move helper disambiguation to a flag that is off by default
diff --git 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
@@ -191,6 +191,22 @@ If you would like to only convert certain component invocations to use the angle
 }
 ```
 
+### Making helper invocations unambiguous
+
+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.
+
+In your **config/anglebrackets-codemod-config.json**, add this:
+
+```js
+{
+  "unambiguousHelpers": true
+}
+```
+
+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.
+
 ## Debugging Workflow
 
 Oftentimes, you want to debug the codemod or the transform to identify issues with the code or to understand
diff --git 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
diff --git a/test/fixtures/with-telemetry/input/app/templates/application.hbs b/test/fixtures/with-telemetry/input/app/templates/application.hbs
@@ -14,27 +14,8 @@
     {{/bs-nav}}
   </div>
   <div class="col-sm-8 col-sm-pull-4 col-md-9 col-md-pull-3">
-    {{utils/bee-bop}}
     {{-wat-wat}}
-    {{utils/-wat-wat}}
-    {{#if this.isDetailPage}}
-      <h1>
-        {{currentComponent.title}}
-      </h1>
-      <p class="lead">
-        {{currentComponent.description}}
-      </p>
-      {{api-reference component=this.currentComponent}}
-    {{/if}}
-    {{outlet}}
-
-    {{#bs-button id="openModal" onClick=(action this.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}}
   </div>
 </div>
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
@@ -1,2 +1 @@
 <div>this template has no js </div>
-{{#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
@@ -14,27 +14,8 @@
     </BsNav>
   </div>
   <div class="col-sm-8 col-sm-pull-4 col-md-9 col-md-pull-3">
-    {{(utils/bee-bop)}}
-    {{(-wat-wat)}}
-    {{(utils/-wat-wat)}}
-    {{#if this.isDetailPage}}
-      <h1>
-        {{currentComponent.title}}
-      </h1>
-      <p class="lead">
-        {{currentComponent.description}}
-      </p>
-      <ApiReference @component={{this.currentComponent}} />
-    {{/if}}
-    {{outlet}}
-
-    <BsButton @id="openModal" @onClick={{action this.addModal}}>Open</BsButton>
-
-    {{#if hasModal}}
-      <BsModalSimple @open={{modal}} @onHidden={{action "removeModal"}} @title="Dynamic Dialog">
-        Hi there
-      </BsModalSimple>
-    {{/if}}
+    {{-wat-wat}}
+    {{outlet}}    
     <FileLess @foo={{true}} />
   </div>
 </div>
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
@@ -1,2 +1 @@
 <div>this template has no js </div>
-<BsButton @type="primary">Primary</BsButton>
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
@@ -1,4 +1,4 @@
 <button>
-  {{(titleize "string helpers for ember")}}
+  {{titleize "string helpers for ember"}}
 </button>
-{{(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
@@ -1,9 +1,9 @@
-{{site-header user=this.user class=(if this.user.isAdmin "admin")}}
+<SiteHeader @user={{this.user}} @class={{if this.user.isAdmin "admin"}} />
 
-{{#super-select selected=this.user.country as |s|}}
+<SuperSelect @selected={{this.user.country}} as |s|>
   {{#each this.availableCountries as |country|}}
-    {{#s.option value=country}}{{country.name}}{{/s.option}}
+    <s.option @value={{country}}>{{country.name}}</s.option>
   {{/each}}
-{{/super-select}}
+</SuperSelect>
 
-{{ui/button text="Click me"}}
+<Ui::Button @text="Click me" />
diff --git 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);
diff --git 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/telemetry/invokable.js b/transforms/angle-brackets/telemetry/invokable.js
@@ -4,7 +4,18 @@ 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/';
+  }
+
   return name.substring(name.lastIndexOf(invokePath) + invokePath.length, name.length);
 }
 
diff --git a/transforms/angle-brackets/transform.js b/transforms/angle-brackets/transform.js
@@ -341,18 +341,26 @@ 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 strName = `${name}`; // coerce boolean and number to string
-    return isHelper && !strName.includes('.');
+      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);
+      let isComponent = [...(components || []), ...BUILT_IN_COMPONENTS].includes(name);
+      let strName = `${name}`; // coerce boolean and number to string
+      return (isHelper || !isComponent) && !strName.includes('.');
+    }
   } else {
     return KNOWN_HELPERS.includes(name) || config.helpers.includes(name);
   }
@@ -489,6 +497,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

Original file line number	Diff line number	Diff line change
`@@ -1,2 +1 @@`
`1`	`1`	`<div>this template has no js </div>`
`2`		`-{{#bs-button type="primary"}}Primary{{/bs-button}}`
Original file line number	Diff line number	Diff line change
`@@ -1,2 +1 @@`
`1`	`1`	`<div>this template has no js </div>`
`2`		`-<BsButton @type="primary">Primary</BsButton>`