Update argument handling

This commit updates the argument handling:

* To match [RFC PR #620](emberjs/rfcs#620)
  * Returns `null` on `null` or `undefined`
  * Use development mode assertions for other invalid arguments
* To match more closely the likely Ember side implementation
  * Handle argument errors at runtime
  * `owner.hasRegistration('helper:element') === true`

Since the argument errors are moved to runtime, this also removes
the need for separate node tests. Previously, that also caused an
error when `ember-auto-import` is included. Since we removed the
`qunit` dependency we can now include `ember-auto-import` to align
with the default addon blueprint.

See also https://discordapp.com/channels/480462759797063690/485447409296736276/704799818316251216

.github/workflows/build.yml

@@ -51,8 +51,6 @@ jobs:
         run: yarn lint:js
       - name: Build
         run: yarn ember build --environment test
-      - name: Run test (node)
-        run: yarn test:node
       - name: Run test (ember)
         # Due to a bug in ember-cli, running `ember test` with `--path` doesn't set `EMBER_ENV=test`
         # See https://github.com/ember-cli/ember-cli/issues/8922

CONTRIBUTING.md

@@ -17,7 +17,6 @@
 * `yarn test:ember` – Runs the test suite on the current Ember version
 * `yarn test:ember --server` – Runs the test suite in "watch mode"
 * `yarn test:all` – Runs the test suite against multiple Ember versions
-* `yarn test:node` – Runs the additional test suite for syntax errors
 
 ## Running the dummy application
 

addon/helpers/-element.js

@@ -1,36 +1,48 @@
 import Helper from '@ember/component/helper';
+import { assert, runInDebug } from '@ember/debug';
 
 function UNINITIALIZED() {}
 
 export default Helper.extend({
   init() {
     this._super(...arguments);
     this.tagName = UNINITIALIZED;
-    this.componentName = UNINITIALIZED;
+    this.componentName = null;
   },
 
-  compute([tagName]) {
-    if (tagName === this.tagName) {
-      return this.componentName;
-    } else if (typeof tagName !== 'string') {
-      let message = 'the argument passed to the `element` helper must be a string';
+  compute(params, hash) {
+    assert('The `element` helper takes a single positional argument', params.length === 1);
+    assert('The `element` helper does not take any named arguments', Object.keys(hash).length === 0);
 
-      try {
-        message += ` (you passed \`${tagName}\`)`;
-      } catch (e) {
-        // ignore
-      }
+    let tagName = params[0];
 
-      throw new Error(message);
-    } else {
+    if (tagName !== this.tagName) {
       this.tagName = tagName;
 
-      // return a different component name to force a teardown
-      if (this.componentName === '-dynamic-element') {
-        return this.componentName = '-dynamic-element-alt';
+      if (typeof tagName === 'string') {
+        // return a different component name to force a teardown
+        if (this.componentName === '-dynamic-element') {
+          this.componentName = '-dynamic-element-alt';
+        } else {
+          this.componentName = '-dynamic-element';
+        }
       } else {
-        return this.componentName = '-dynamic-element';
+        this.componentName = null;
+
+        runInDebug(() => {
+          let message = 'The argument passed to the `element` helper must be a string';
+
+          try {
+            message += ` (you passed \`${tagName}\`)`;
+          } catch (e) {
+            // ignore
+          }
+
+          assert(message, tagName === undefined || tagName === null);
+        });
       }
     }
+
+    return this.componentName;
   }
 });

addon/helpers/element.js

@@ -0,0 +1,17 @@
+import { helper } from '@ember/component/helper';
+import { assert } from '@ember/debug';
+
+export default helper(function() {
+  // This helper (`element`, as opposed to `-element`) mostly exists to satisfy
+  // things like `owner.hasRegistration('helper:element')`. The AST transform
+  // replaces all usages of `(element ...)` into `(component (-element ...))`
+  // so if this helper is invoked directly, something is wrong.
+
+  assert(
+    'The `element` helper polyfill encounted an unexpected error. ' +
+    'Please report the issue at http://github.com/tildeio/ember-element-helper ' +
+    'with the usage and conditions that caused this error.'
+  );
+
+  return null;
+});

app/helpers/element.js

@@ -0,0 +1 @@
+export { default } from 'ember-element-helper/helpers/element';

lib/element-helper-syntax-plugin.js

@@ -6,11 +6,6 @@ module.exports = class ElementHelperSyntaxPlugin {
     this.syntax.traverse(ast, {
       BlockStatement: {
         enter(node) {
-          // check this before we push the new locals
-          if (isElementHelper(node, locals)) {
-            throw error('does not take a block', node);
-          }
-
           locals.push(node.program.blockParams);
         },
 
@@ -49,8 +44,6 @@ module.exports = class ElementHelperSyntaxPlugin {
 }
 
 function transformParts(node, b) {
-  validateElementHelperArgs(node);
-
   return {
     // path
     path: b.path('component', node.path.loc),
@@ -66,9 +59,11 @@ function transformParts(node, b) {
       node.loc
     )],
     // hash
-    hash: b.hash([
-      b.pair('tagName', node.params[0], node.params[0].loc)
-    ], node.params[0].loc)
+    hash: node.params.length > 0 ?
+      b.hash([
+        b.pair('tagName', node.params[0], node.params[0].loc)
+      ], node.params[0].loc) :
+      b.hash(),
   };
 }
 
@@ -81,29 +76,3 @@ function isElementHelper(node, locals) {
     node.path.original === 'element' &&
     !hasLocalVariable('element', locals);
 }
-
-function validateElementHelperArgs(node) {
-  if (node.params.length !== 1) {
-    throw error('requires exactly one positional argument', node);
-  }
-
-  if (node.hash.pairs.length !== 0) {
-    throw error('does not accept any named arguments', node);
-  }
-}
-
-function error(reason, node) {
-  let message = `the \`element\` helper ${reason}`;
-
-  if (node.loc && node.loc.source !== '(synthetic)' && node.loc.start) {
-    let { line, column } = node.loc.start;
-
-    if (line !== undefined && column !== undefined) {
-      message += ` (on line ${line} column ${column})`;
-    } else if (line !== undefined) {
-      message += ` (on line ${line})`;
-    }
-  }
-
-  return new Error(message);
-}

package.json

@@ -18,7 +18,6 @@
     "lint:js": "eslint .",
     "start": "ember serve",
     "test": "yarn test:node && yarn test:ember",
-    "test:node": "qunit node-tests",
     "test:ember": "ember test",
     "test:all": "ember try:each"
   },
@@ -32,6 +31,7 @@
     "@glimmer/tracking": "^1.0.0",
     "babel-eslint": "^10.0.3",
     "broccoli-asset-rev": "^3.0.0",
+    "ember-auto-import": "^1.5.3",
     "ember-cli": "~3.16.1",
     "ember-cli-dependency-checker": "^3.2.0",
     "ember-cli-eslint": "^5.1.0",
@@ -51,7 +51,6 @@
     "eslint-plugin-ember": "^7.7.2",
     "eslint-plugin-node": "^11.0.0",
     "loader.js": "^4.7.0",
-    "qunit": "*",
     "qunit-dom": "^1.0.0"
   },
   "engines": {