Merge pull request #17779 from emberjs/angle-link-to

Transform positional `{{link-to}}` invocations into named arguments.

packages/@ember/-internals/glimmer/lib/components/link-to.ts

@@ -557,13 +557,14 @@ if (EMBER_GLIMMER_ANGLE_BRACKET_BUILT_INS) {
       let { model, models } = this;
 
       assert(
-        'You cannot provide both the `@model` and `@models` arguments to the <LinkTo /> component',
+        'You cannot provide both the `@model` and `@models` arguments to the <LinkTo /> component.',
         model === UNDEFINED || models === UNDEFINED
       );
 
       if (model !== UNDEFINED) {
         return [model];
       } else if (models !== UNDEFINED) {
+        assert('The `@models` argument must be an array.', Array.isArray(models));
         return models;
       } else {
         return [];
@@ -921,6 +922,16 @@ if (EMBER_GLIMMER_ANGLE_BRACKET_BUILT_INS) {
           )
         );
 
+        if (DEBUG && this.query === UNDEFINED) {
+          let { _models: models } = this;
+          let lastModel = models.length > 0 && models[models.length - 1];
+
+          assert(
+            'The `(query-params)` helper can only be used when invoking the `{{link-to}}` component.',
+            !(lastModel && lastModel.isQueryParams)
+          );
+        }
+
         return;
       }
 
@@ -933,9 +944,9 @@ if (EMBER_GLIMMER_ANGLE_BRACKET_BUILT_INS) {
       }
 
       // 2. The last argument is possibly the `query` object.
-      let lastParam = params[params.length - 1];
+      let queryParams = params[params.length - 1];
 
-      if (lastParam && lastParam.isQueryParams) {
+      if (queryParams && queryParams.isQueryParams) {
         this.set('query', params.pop().values);
       } else {
         this.set('query', UNDEFINED);
@@ -1795,8 +1806,8 @@ if (EMBER_GLIMMER_ANGLE_BRACKET_BUILT_INS) {
       }
 
       assert(
-        'You must provide one or more parameters to the link-to component.',
-        params && params.length
+        'You must provide one or more parameters to the `{{link-to}}` component.',
+        params && params.length > 0
       );
 
       let disabledWhen = get(this, 'disabledWhen');

packages/@ember/-internals/glimmer/tests/integration/components/link-to/query-params-curly-test.js

@@ -50,6 +50,18 @@ moduleFor(
         });
       });
     }
+
+    ['@feature(ember-glimmer-angle-bracket-built-ins) `(query-params)` must be used in conjunction with `{{link-to}}']() {
+      this.addTemplate(
+        'index',
+        `{{#let (query-params foo='456' bar='NAW') as |qp|}}{{link-to 'Index' 'index' qp}}{{/let}}`
+      );
+
+      return expectAssertion(
+        () => this.visit('/'),
+        /The `\(query-params\)` helper can only be used when invoking the `{{link-to}}` component\./
+      );
+    }
   }
 );
 

packages/@ember/-internals/glimmer/tests/integration/components/link-to/rendering-curly-test.js

@@ -12,11 +12,9 @@ moduleFor(
     ) {
       assert.expect(1);
 
-      this.addTemplate('application', `{{#link-to id='the-link'}}Index{{/link-to}}`);
-
       expectAssertion(() => {
-        this.visit('/');
-      }, /You must provide at least one of the `@route`, `@model`, `@models` or `@query` argument to `<LinkTo \/>`/);
+        this.addTemplate('application', `{{#link-to id='the-link'}}Index{{/link-to}}`);
+      }, /You must provide one or more parameters to the `{{link-to}}` component\. \('my-app\/templates\/application\.hbs' @ L1:C0\)/);
     }
 
     [`@feature(!ember-glimmer-angle-bracket-built-ins) throws a useful error if you invoke it wrong`](

packages/ember-template-compiler/lib/plugins/transform-link-to.ts

@@ -1,6 +1,166 @@
+import { EMBER_GLIMMER_ANGLE_BRACKET_BUILT_INS } from '@ember/canary-features';
+import { assert } from '@ember/debug';
 import { AST, ASTPlugin, ASTPluginEnvironment } from '@glimmer/syntax';
+import calculateLocationDisplay from '../system/calculate-location-display';
 import { Builders } from '../types';
 
+function isInlineLinkTo(node: AST.MustacheStatement): boolean {
+  return node.path.original === 'link-to';
+}
+
+function isBlockLinkTo(node: AST.BlockStatement): boolean {
+  return node.path.original === 'link-to';
+}
+
+function isSubExpression(node: AST.Expression): node is AST.SubExpression {
+  return node.type === 'SubExpression';
+}
+
+function isQueryParams(node: AST.Expression): node is AST.SubExpression {
+  return isSubExpression(node) && node.path.original === 'query-params';
+}
+
+function transformInlineLinkToIntoBlockForm(
+  env: ASTPluginEnvironment,
+  node: AST.MustacheStatement
+): AST.BlockStatement {
+  let { builders: b } = env.syntax;
+
+  return b.block(
+    'link-to',
+    node.params.slice(1),
+    node.hash,
+    buildProgram(b, node.params[0], node.escaped, node.loc),
+    null,
+    node.loc
+  );
+}
+
+function transformPositionalLinkToIntoNamedArguments(
+  env: ASTPluginEnvironment,
+  node: AST.BlockStatement
+): AST.BlockStatement {
+  let { builders: b } = env.syntax;
+  let { moduleName } = env.meta;
+  let {
+    params,
+    hash: { pairs },
+  } = node;
+
+  let keys = pairs.map(pair => pair.key);
+
+  if (params.length === 0) {
+    assert(
+      `You must provide one or more parameters to the \`{{link-to}}\` component. ${calculateLocationDisplay(
+        moduleName,
+        node.loc
+      )}`,
+      keys.indexOf('params') !== -1 ||
+        keys.indexOf('route') !== -1 ||
+        keys.indexOf('model') !== -1 ||
+        keys.indexOf('models') !== -1 ||
+        keys.indexOf('query') !== -1
+    );
+
+    return node;
+  } else {
+    assert(
+      `You cannot pass positional parameters and the \`params\` argument to the \`{{link-to}}\` component at the same time. ${calculateLocationDisplay(
+        moduleName,
+        node.loc
+      )}`,
+      keys.indexOf('params') === -1
+    );
+
+    assert(
+      `You cannot pass positional parameters and the \`route\` argument to the \`{{link-to}}\` component at the same time. ${calculateLocationDisplay(
+        moduleName,
+        node.loc
+      )}`,
+      keys.indexOf('route') === -1
+    );
+
+    assert(
+      `You cannot pass positional parameters and the \`model\` argument to the \`{{link-to}}\` component at the same time. ${calculateLocationDisplay(
+        moduleName,
+        node.loc
+      )}`,
+      keys.indexOf('model') === -1
+    );
+
+    assert(
+      `You cannot pass positional parameters and the \`models\` argument to the \`{{link-to}}\` component at the same time. ${calculateLocationDisplay(
+        moduleName,
+        node.loc
+      )}`,
+      keys.indexOf('models') === -1
+    );
+
+    assert(
+      `You cannot pass positional parameters and the \`query\` argument to the \`{{link-to}}\` component at the same time. ${calculateLocationDisplay(
+        moduleName,
+        node.loc
+      )}`,
+      keys.indexOf('query') === -1
+    );
+  }
+
+  assert(
+    `You must provide one or more parameters to the \`{{link-to}}\` component. ${calculateLocationDisplay(
+      moduleName,
+      node.loc
+    )}`,
+    params.length > 0
+  );
+
+  // 1. The last argument is possibly the `query` object.
+
+  let query = params[params.length - 1];
+
+  if (query && isQueryParams(query)) {
+    params.pop();
+
+    assert(
+      `The \`(query-params ...)\` helper does not take positional arguments. ${calculateLocationDisplay(
+        moduleName,
+        query.loc
+      )}`,
+      query.params.length === 0
+    );
+
+    pairs.push(
+      b.pair('query', b.sexpr(b.path('hash', query.path.loc), [], query.hash, query.loc), query.loc)
+    );
+  }
+
+  // 2. If there is a `route`, it is now at index 0.
+
+  let route = params.shift();
+
+  if (route) {
+    pairs.push(b.pair('route', route, route.loc));
+  }
+
+  // 3. Any remaining indices (if any) are `models`.
+
+  if (params.length === 1) {
+    pairs.push(b.pair('model', params[0], params[0].loc));
+  } else if (params.length > 1) {
+    pairs.push(
+      b.pair('models', b.sexpr(b.path('array', node.loc), params, undefined, node.loc), node.loc)
+    );
+  }
+
+  return b.block(
+    node.path,
+    null,
+    b.hash(pairs, node.hash.loc),
+    node.program,
+    node.inverse,
+    node.loc
+  );
+}
+
 function buildProgram(b: Builders, content: AST.Node, escaped: boolean, loc: AST.SourceLocation) {
   return b.program([buildStatement(b, content, escaped, loc)], undefined, loc);
 }
@@ -20,22 +180,25 @@ function buildStatement(b: Builders, content: AST.Node, escaped: boolean, loc: A
 }
 
 export default function transformLinkTo(env: ASTPluginEnvironment): ASTPlugin {
-  let { builders: b } = env.syntax;
-
   return {
     name: 'transform-link-to',
 
     visitor: {
       MustacheStatement(node: AST.MustacheStatement): AST.Node | void {
-        if (node.path.original === 'link-to') {
-          return b.block(
-            'link-to',
-            node.params.slice(1),
-            node.hash,
-            buildProgram(b, node.params[0], node.escaped, node.loc),
-            null,
-            node.loc
-          );
+        if (isInlineLinkTo(node)) {
+          let block = transformInlineLinkToIntoBlockForm(env, node);
+
+          if (EMBER_GLIMMER_ANGLE_BRACKET_BUILT_INS) {
+            block = transformPositionalLinkToIntoNamedArguments(env, block);
+          }
+
+          return block;
+        }
+      },
+
+      BlockStatement(node: AST.BlockStatement): AST.Node | void {
+        if (EMBER_GLIMMER_ANGLE_BRACKET_BUILT_INS && isBlockLinkTo(node)) {
+          return transformPositionalLinkToIntoNamedArguments(env, node);
         }
       },
     },