Implement named arguments feature

Also added a feature to the test harness for testing optional features
emberjs · Dec 12, 2017 · 416f56f · 416f56f
1 parent 2af8ccf
commit 416f56f
Show file tree

Hide file tree

Showing 6 changed files with 256 additions and 26 deletions.
diff --git a/FEATURES.md b/FEATURES.md
@@ -27,6 +27,11 @@ for a detailed explanation.
   Adds an ability to for developers to integrate their own custom component managers
   into Ember Applications per [RFC](https://github.com/emberjs/rfcs/blob/custom-components/text/0000-custom-components.md).
 
+* `ember-glimmer-named-arguments`
+
+  Add `{{@foo}}` syntax to access named arguments in component templates per
+  [RFC](https://github.com/emberjs/rfcs/pull/276).
+
 * `ember-module-unification`
 
   Introduces support for Module Unification

diff --git a/features.json b/features.json
@@ -3,6 +3,7 @@
     "features-stripped-test": null,
     "ember-libraries-isregistered": null,
     "ember-improved-instrumentation": null,
+    "ember-glimmer-named-arguments": null,
     "ember-routing-router-service": true,
     "ember-engines-mount-params": true,
     "ember-module-unification": null,

diff --git a/packages/ember-glimmer/tests/integration/components/curly-components-test.js b/packages/ember-glimmer/tests/integration/components/curly-components-test.js
@@ -534,7 +534,33 @@ moduleFor('Components test: curly components', class extends RenderingTest {
     assert.deepEqual(fooBarInstance.childViews, [fooBarBazInstance]);
   }
 
-  ['@test it renders passed named arguments']() {
+  ['@feature(ember-glimmer-named-arguments) it renders passed named arguments']() {
+    this.registerComponent('foo-bar', {
+      template: '{{@foo}}'
+    });
+
+    this.render('{{foo-bar foo=model.bar}}', {
+      model: {
+        bar: 'Hola'
+      }
+    });
+
+    this.assertText('Hola');
+
+    this.runTask(() => this.rerender());
+
+    this.assertText('Hola');
+
+    this.runTask(() => this.context.set('model.bar', 'Hello'));
+
+    this.assertText('Hello');
+
+    this.runTask(() => this.context.set('model', { bar: 'Hola' }));
+
+    this.assertText('Hola');
+  }
+
+  ['@test it reflects named arguments as properties']() {
     this.registerComponent('foo-bar', {
       template: '{{foo}}'
     });
@@ -1063,6 +1089,30 @@ moduleFor('Components test: curly components', class extends RenderingTest {
     this.assertText('In layout - someProp: something here');
   }
 
+  ['@feature(ember-glimmer-named-arguments) non-block with named argument']() {
+    this.registerComponent('non-block', {
+      template: 'In layout - someProp: {{@someProp}}'
+    });
+
+    this.render('{{non-block someProp=prop}}', {
+      prop: 'something here'
+    });
+
+    this.assertText('In layout - someProp: something here');
+
+    this.runTask(() => this.rerender());
+
+    this.assertText('In layout - someProp: something here');
+
+    this.runTask(() => this.context.set('prop', 'other thing there'));
+
+    this.assertText('In layout - someProp: other thing there');
+
+    this.runTask(() => this.context.set('prop', 'something here'));
+
+    this.assertText('In layout - someProp: something here');
+  }
+
   ['@test non-block with properties overridden in init']() {
     let instance;
     this.registerComponent('non-block', {
@@ -1171,7 +1221,7 @@ moduleFor('Components test: curly components', class extends RenderingTest {
     this.assertText('In layout - someProp: wycats');
   }
 
-  ['@test this.attrs.foo === attrs.foo === foo']() {
+  ['@feature(!ember-glimmer-named-arguments) this.attrs.foo === attrs.foo === foo']() {
     this.registerComponent('foo-bar', {
       template: strip`
         Args: {{this.attrs.value}} | {{attrs.value}} | {{value}}
@@ -1208,6 +1258,46 @@ moduleFor('Components test: curly components', class extends RenderingTest {
     this.assertText('Args: wat | wat | wat123123123');
   }
 
+  ['@feature(ember-glimmer-named-arguments) this.attrs.foo === attrs.foo === @foo === foo']() {
+    this.registerComponent('foo-bar', {
+      template: strip`
+        Args: {{this.attrs.value}} | {{attrs.value}} | {{@value}} | {{value}}
+        {{#each this.attrs.items as |item|}}
+          {{item}}
+        {{/each}}
+        {{#each attrs.items as |item|}}
+          {{item}}
+        {{/each}}
+        {{#each @items as |item|}}
+          {{item}}
+        {{/each}}
+        {{#each items as |item|}}
+          {{item}}
+        {{/each}}
+      `
+    });
+
+    this.render('{{foo-bar value=model.value items=model.items}}', {
+      model: {
+        value: 'wat',
+        items: [1, 2, 3]
+      }
+    });
+
+    this.assertStableRerender();
+
+    this.runTask(() => {
+      this.context.set('model.value', 'lul');
+      this.context.set('model.items', [1]);
+    });
+
+    this.assertText(strip`Args: lul | lul | lul | lul1111`);
+
+    this.runTask(() => this.context.set('model', { value: 'wat', items: [1, 2, 3] }));
+
+    this.assertText('Args: wat | wat | wat | wat123123123123');
+  }
+
   ['@test non-block with properties on self']() {
     this.registerComponent('non-block', {
       template: 'In layout - someProp: {{someProp}}'
@@ -1288,6 +1378,34 @@ moduleFor('Components test: curly components', class extends RenderingTest {
     this.assertText('In layout - someProp: something here - In template');
   }
 
+  ['@feature(ember-glimmer-named-arguments) block with named argument']() {
+    this.registerComponent('with-block', {
+      template: 'In layout - someProp: {{@someProp}} - {{yield}}'
+    });
+
+    this.render(strip`
+      {{#with-block someProp=prop}}
+        In template
+      {{/with-block}}`, {
+        prop: 'something here'
+      }
+    );
+
+    this.assertText('In layout - someProp: something here - In template');
+
+    this.runTask(() => this.rerender());
+
+    this.assertText('In layout - someProp: something here - In template');
+
+    this.runTask(() => this.context.set('prop', 'something else'));
+
+    this.assertText('In layout - someProp: something else - In template');
+
+    this.runTask(() => this.context.set('prop', 'something here'));
+
+    this.assertText('In layout - someProp: something here - In template');
+  }
+
   ['@test static arbitrary number of positional parameters'](assert) {
     this.registerComponent('sample-component', {
       ComponentClass: Component.extend().reopenClass({
@@ -2966,6 +3084,21 @@ moduleFor('Components test: curly components', class extends RenderingTest {
     this.assertText('MyVar1: 1 1 MyVar2: 2 2');
   }
 
+  ['@feature(ember-glimmer-named-arguments) using named arguments for positional params'](assert) {
+    let MyComponent = Component.extend();
+
+    this.registerComponent('foo-bar', {
+      ComponentClass: MyComponent.reopenClass({
+        positionalParams: ['myVar']
+      }),
+      template: 'MyVar1: {{@myVar}} {{myVar}} MyVar2: {{myVar2}} {{@myVar2}}'
+    });
+
+    this.render('{{foo-bar 1 myVar2=2}}');
+
+    this.assertText('MyVar1: 1 1 MyVar2: 2 2');
+  }
+
   ['@test can use `{{this}}` to emit the component\'s toString value [GH#14581]'](assert) {
     this.registerComponent('foo-bar', {
       ComponentClass: Component.extend({
@@ -3035,7 +3168,7 @@ moduleFor('Components test: curly components', class extends RenderingTest {
     this.assertText('Hi!');
   }
 
-  ['@test can access properties off of rest style positionalParams array'](assert) {
+  ['@feature(!ember-glimmer-named-arguments) can access properties off of rest style positionalParams array'](assert) {
     this.registerComponent('foo-bar', {
       ComponentClass: Component.extend().reopenClass({ positionalParams: 'things' }),
       // using `attrs` here to simulate `@things.length`
@@ -3046,4 +3179,15 @@ moduleFor('Components test: curly components', class extends RenderingTest {
 
     this.assertText('3');
   }
+
+  ['@feature(ember-glimmer-named-arguments) can access properties off of rest style positionalParams array'](assert) {
+    this.registerComponent('foo-bar', {
+      ComponentClass: Component.extend().reopenClass({ positionalParams: 'things' }),
+      template: `{{@things.length}}`
+    });
+
+    this.render('{{foo-bar "foo" "bar" "baz"}}');
+
+    this.assertText('3');
+  }
 });
diff --git a/packages/ember-template-compiler/lib/plugins/assert-reserved-named-arguments.js b/packages/ember-template-compiler/lib/plugins/assert-reserved-named-arguments.js
@@ -1,4 +1,5 @@
 import { assert } from 'ember-debug';
+import { EMBER_GLIMMER_NAMED_ARGUMENTS } from 'ember/features';
 import calculateLocationDisplay from '../system/calculate-location-display';
 
 export default function assertReservedNamedArguments(env) {
@@ -8,18 +9,22 @@ export default function assertReservedNamedArguments(env) {
     name: 'assert-reserved-named-arguments',
 
     visitors: {
-      PathExpression(node) {
-        if (node.original[0] === '@') {
-          assert(assertMessage(moduleName, node));
+      PathExpression({ original, loc }) {
+        if (isReserved(original)) {
+          assert(`${assertMessage(original)} ${calculateLocationDisplay(moduleName, loc)}`);
         }
       }
     }
   };
 }
 
-function assertMessage(moduleName, node) {
-  let path = node.original;
-  let source = calculateLocationDisplay(moduleName, node.loc);
+const RESERVED = ['@arguments', '@args'];
+let isReserved, assertMessage;
 
-  return `'${path}' is not a valid path. ${source}`;
+if (EMBER_GLIMMER_NAMED_ARGUMENTS) {
+  isReserved = name => RESERVED.indexOf(name) !== -1 || name.match(/^@[^a-z]/);
+  assertMessage = name => `'${name}' is reserved.`;
+} else {
+  isReserved = name => name[0] === '@';
+  assertMessage = name => `'${name}' is not a valid path.`;
 }
diff --git a/packages/ember-template-compiler/tests/plugins/assert-reserved-named-arguments-test.js b/packages/ember-template-compiler/tests/plugins/assert-reserved-named-arguments-test.js
@@ -1,25 +1,88 @@
+import { EMBER_GLIMMER_NAMED_ARGUMENTS } from 'ember/features';
 import { compile } from '../../index';
 
 QUnit.module('ember-template-compiler: assert-reserved-named-arguments');
 
-QUnit.test('Paths beginning with @ are not valid', function() {
-  expect(3);
+if (EMBER_GLIMMER_NAMED_ARGUMENTS) {
+  let RESERVED = [
+    '@arguments',
+    '@args',
+    // anything else that doesn't start with a lower case letter
+    '@.', '@_', '@-', '@$'
+  ];
 
-  expectAssertion(() => {
-    compile('{{@foo}}', {
-      moduleName: 'baz/foo-bar'
-    });
-  }, `'@foo' is not a valid path. ('baz/foo-bar' @ L1:C2) `);
+  RESERVED.forEach(name => {
+    QUnit.test(`'${name}' is reserved`, () => {
+      expect(3);
+
+      expectAssertion(() => {
+        compile(`{{${name}}}`, {
+          moduleName: 'baz/foo-bar'
+        });
+      }, `'${name}' is reserved. ('baz/foo-bar' @ L1:C2) `);
+
+      expectAssertion(() => {
+        compile(`{{#if ${name}}}Yup{{/if}}`, {
+          moduleName: 'baz/foo-bar'
+        });
+      }, `'${name}' is reserved. ('baz/foo-bar' @ L1:C6) `);
 
-  expectAssertion(() => {
-    compile('{{#if @foo}}Yup{{/if}}', {
-      moduleName: 'baz/foo-bar'
+      expectAssertion(() => {
+        compile(`{{input type=(if ${name} "bar" "baz")}}`, {
+          moduleName: 'baz/foo-bar'
+        });
+      }, `'${name}' is reserved. ('baz/foo-bar' @ L1:C17) `);
     });
-  }, `'@foo' is not a valid path. ('baz/foo-bar' @ L1:C6) `);
+  });
+
+  let DE_FACTO_RESERVED = [
+    '@0', '@1', '@2',
+    '@@', '@!', '@='
+  ];
+
+  DE_FACTO_RESERVED.forEach(name => {
+    QUnit.test(`'${name}' is de facto reserved (parse error)`, assert => {
+      expect(3);
 
-  expectAssertion(() => {
-    compile('{{input type=(if @foo "bar" "baz")}}', {
-      moduleName: 'baz/foo-bar'
+      assert.throws(() => {
+        compile(`{{${name}}}`, {
+          moduleName: 'baz/foo-bar'
+        });
+      }, /Expecting 'ID'/);
+
+      assert.throws(() => {
+        compile(`{{#if ${name}}}Yup{{/if}}`, {
+          moduleName: 'baz/foo-bar'
+        });
+      }, /Expecting 'ID'/);
+
+      assert.throws(() => {
+        compile(`{{input type=(if ${name} "bar" "baz")}}`, {
+          moduleName: 'baz/foo-bar'
+        });
+      }, /Expecting 'ID'/);
     });
-  }, `'@foo' is not a valid path. ('baz/foo-bar' @ L1:C17) `);
-});
+  });
+} else {
+  QUnit.test('Paths beginning with @ are not valid', () => {
+    expect(3);
+
+    expectAssertion(() => {
+      compile('{{@foo}}', {
+        moduleName: 'baz/foo-bar'
+      });
+    }, `'@foo' is not a valid path. ('baz/foo-bar' @ L1:C2) `);
+
+    expectAssertion(() => {
+      compile('{{#if @foo}}Yup{{/if}}', {
+        moduleName: 'baz/foo-bar'
+      });
+    }, `'@foo' is not a valid path. ('baz/foo-bar' @ L1:C6) `);
+
+    expectAssertion(() => {
+      compile('{{input type=(if @foo "bar" "baz")}}', {
+        moduleName: 'baz/foo-bar'
+      });
+    }, `'@foo' is not a valid path. ('baz/foo-bar' @ L1:C17) `);
+  });
+}
diff --git a/packages/internal-test-helpers/lib/module-for.js b/packages/internal-test-helpers/lib/module-for.js
@@ -1,3 +1,4 @@
+import { isFeatureEnabled } from 'ember-debug';
 import { RSVP } from 'ember-runtime';
 import applyMixins from './apply-mixins';
 
@@ -31,6 +32,17 @@ export default function moduleFor(description, TestClass, ...mixins) {
       QUnit.test(name.slice(5), assert => context[name](assert));
     } else if (name.indexOf('@skip ') === 0) {
       QUnit.skip(name.slice(5), assert => context[name](assert));
+    } else {
+      let match = /^@feature\((!?)([a-z-]+)\) /.exec(name);
+      let shouldTest = match && isFeatureEnabled(match[2]);
+
+      if (match && match[1] === '!') {
+        shouldTest = !shouldTest;
+      }
+
+      if (shouldTest) {
+        QUnit.test(name.slice(match[0].length), assert => context[name](assert));
+      }
     }
   }
 }