From 66260f41cd1203420b09d13a46eecc6e48a904b8 Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Mon, 11 Dec 2017 22:37:38 -0800 Subject: [PATCH 1/2] [CLEANUP] Remove allow-backtracking build Leftover from https://github.com/emberjs/ember.js/pull/15890 --- .travis.yml | 1 - broccoli/features.js | 9 +-------- features.json | 1 - packages/ember-metal/lib/transaction.js | 1 - 4 files changed, 1 insertion(+), 11 deletions(-) diff --git a/.travis.yml b/.travis.yml index 57c6eff7288..77dcb7553ba 100644 --- a/.travis.yml +++ b/.travis.yml @@ -65,5 +65,4 @@ env: - TEST_SUITE=blueprints - TEST_SUITE=travis-browsers DISABLE_JSCS=true DISABLE_JSHINT=true - TEST_SUITE=sauce - - TEST_SUITE=allow-backtracking-rerender ALLOW_BACKTRACKING=true - TEST_SUITE=each-package-tests BUILD_TYPE=alpha PUBLISH=true diff --git a/broccoli/features.js b/broccoli/features.js index 93a66cacb11..538cc2f77af 100644 --- a/broccoli/features.js +++ b/broccoli/features.js @@ -22,13 +22,6 @@ function getFeatures(isDebug) { } } - features['ember-glimmer-allow-backtracking-rerender'] = false; - - if (process.env.ALLOW_BACKTRACKING) { - features['ember-glimmer-allow-backtracking-rerender'] = true; - features['ember-glimmer-detect-backtracking-rerender'] = false; - } - features['mandatory-setter'] = isDebug; features['ember-glimmer-detect-backtracking-rerender'] = isDebug; @@ -46,4 +39,4 @@ function toConst(features) { module.exports.toConst = toConst; module.exports.RELEASE = getFeatures(false); -module.exports.DEBUG = getFeatures(true); \ No newline at end of file +module.exports.DEBUG = getFeatures(true); diff --git a/features.json b/features.json index 673c2b921ae..c05ba9d18cf 100644 --- a/features.json +++ b/features.json @@ -3,7 +3,6 @@ "features-stripped-test": null, "ember-libraries-isregistered": null, "ember-improved-instrumentation": null, - "ember-glimmer-allow-backtracking-rerender": null, "ember-routing-router-service": true, "ember-engines-mount-params": true, "ember-module-unification": null, diff --git a/packages/ember-metal/lib/transaction.js b/packages/ember-metal/lib/transaction.js index 5fa00ae4cd5..f1cbe081ad3 100644 --- a/packages/ember-metal/lib/transaction.js +++ b/packages/ember-metal/lib/transaction.js @@ -8,7 +8,6 @@ import { let runInTransaction, didRender, assertNotRendered; // detect-backtracking-rerender by default is debug build only -// detect-glimmer-allow-backtracking-rerender can be enabled in custom builds if (EMBER_GLIMMER_DETECT_BACKTRACKING_RERENDER) { // there are 2 states From 22fda1136a5239ad5fae595fb8820cabae68fda4 Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Tue, 12 Dec 2017 00:03:52 -0800 Subject: [PATCH 2/2] Implement named arguments feature Also added a feature to the test harness for testing optional features --- FEATURES.md | 5 + features.json | 1 + .../components/curly-components-test.js | 150 +++++++++++++++++- .../assert-reserved-named-arguments.js | 22 ++- .../assert-reserved-named-arguments-test.js | 98 ++++++++++-- .../internal-test-helpers/lib/module-for.js | 12 ++ 6 files changed, 261 insertions(+), 27 deletions(-) diff --git a/FEATURES.md b/FEATURES.md index 4233f6c3a20..06a523068d1 100644 --- 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 index c05ba9d18cf..18a2a5fc5a8 100644 --- 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 index f49730026be..48f46553b30 100644 --- 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 index 09ae0ebcbde..706815f0818 100644 --- 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,6 +1,11 @@ import { assert } from 'ember-debug'; +import { EMBER_GLIMMER_NAMED_ARGUMENTS } from 'ember/features'; import calculateLocationDisplay from '../system/calculate-location-display'; +const RESERVED = ['@arguments', '@args']; + +let isReserved, assertMessage; + export default function assertReservedNamedArguments(env) { let { moduleName } = env.meta; @@ -8,18 +13,19 @@ 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); - - 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 index 97c5d1b0bf9..a44684c1c98 100644 --- 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,91 @@ +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 + '@Arguments', '@Args', + '@A', '@FOO', '@Foo', + '@.', '@_', '@-', '@$' + ]; - 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 index f99c358d1a0..02f2a40197c 100644 --- 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)); + } } } }