Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUGFIX beta] [DEPRECATION] Deprecate @foo={{helper}} #19499

Merged
merged 1 commit into from
Apr 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 13 additions & 13 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
"@babel/plugin-transform-block-scoping": "^7.8.3",
"@babel/plugin-transform-object-assign": "^7.8.3",
"@ember/edition-utils": "^1.2.0",
"@glimmer/vm-babel-plugins": "0.77.5",
"@glimmer/vm-babel-plugins": "0.78.2",
"babel-plugin-debug-macros": "^0.3.3",
"babel-plugin-filter-imports": "^4.0.0",
"broccoli-concat": "^4.2.4",
Expand All @@ -75,19 +75,19 @@
},
"devDependencies": {
"@babel/preset-env": "^7.9.5",
"@glimmer/compiler": "0.77.5",
"@glimmer/compiler": "0.78.2",
"@glimmer/env": "^0.1.7",
"@glimmer/global-context": "0.77.5",
"@glimmer/interfaces": "0.77.5",
"@glimmer/manager": "0.77.5",
"@glimmer/destroyable": "0.77.5",
"@glimmer/owner": "0.77.5",
"@glimmer/node": "0.77.5",
"@glimmer/opcode-compiler": "0.77.5",
"@glimmer/program": "0.77.5",
"@glimmer/reference": "0.77.5",
"@glimmer/runtime": "0.77.5",
"@glimmer/validator": "0.77.5",
"@glimmer/global-context": "0.78.2",
"@glimmer/interfaces": "0.78.2",
"@glimmer/manager": "0.78.2",
"@glimmer/destroyable": "0.78.2",
"@glimmer/owner": "0.78.2",
"@glimmer/node": "0.78.2",
"@glimmer/opcode-compiler": "0.78.2",
"@glimmer/program": "0.78.2",
"@glimmer/reference": "0.78.2",
"@glimmer/runtime": "0.78.2",
"@glimmer/validator": "0.78.2",
"@simple-dom/document": "^1.4.0",
"@types/qunit": "^2.9.1",
"@types/rsvp": "^4.0.3",
Expand Down
8 changes: 8 additions & 0 deletions packages/@ember/-internals/glimmer/lib/environment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,14 @@ const VM_DEPRECATION_OVERRIDES: (DeprecationOptions & {
enabled: '3.26.0',
},
},
{
id: 'argument-less-helper-paren-less-invocation',
until: '4.0.0',
for: 'ember-source',
since: {
enabled: '3.27.0',
},
},
];

const VM_ASSERTION_OVERRIDES: { id: string; message: string }[] = [];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ moduleFor(
`);

expectDeprecation(
/The `[^`]+` property(?: path)? was used in a template for the `[^`]+` component without using `this`. This fallback behavior has been deprecated, all properties must be looked up on `this` when used in the template: {{[^}]+}}/
/The `[^`]+` property(?: path)? was used in the `[^`]+` template without using `this`. This fallback behavior has been deprecated, all properties must be looked up on `this` when used in the template: {{[^}]+}}/
);

this.add('template:application', sharedTemplate);
Expand Down Expand Up @@ -345,7 +345,7 @@ moduleFor(
this.assert.expect(2);

expectDeprecation(
/The `[^`]+` property(?: path)? was used in a template for the `[^`]+` component without using `this`. This fallback behavior has been deprecated, all properties must be looked up on `this` when used in the template: {{[^}]+}}/
/The `[^`]+` property(?: path)? was used in the `[^`]+` template without using `this`. This fallback behavior has been deprecated, all properties must be looked up on `this` when used in the template: {{[^}]+}}/
);

let sharedLayout = compile(strip`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ moduleFor(

['@test it can access the model provided by the route via implicit this fallback']() {
expectDeprecation(
/The `[^`]+` property(?: path)? was used in a template for the `[^`]+` component without using `this`. This fallback behavior has been deprecated, all properties must be looked up on `this` when used in the template: {{[^}]+}}/
/The `[^`]+` property(?: path)? was used in the `[^`]+` template without using `this`. This fallback behavior has been deprecated, all properties must be looked up on `this` when used in the template: {{[^}]+}}/
);

this.add(
Expand Down Expand Up @@ -143,7 +143,7 @@ moduleFor(

async ['@test interior mutations on the model with set'](assert) {
expectDeprecation(
/The `[^`]+` property(?: path)? was used in a template for the `[^`]+` component without using `this`. This fallback behavior has been deprecated, all properties must be looked up on `this` when used in the template: {{[^}]+}}/
/The `[^`]+` property(?: path)? was used in the `[^`]+` template without using `this`. This fallback behavior has been deprecated, all properties must be looked up on `this` when used in the template: {{[^}]+}}/
);

this.router.map(function () {
Expand Down Expand Up @@ -204,7 +204,7 @@ moduleFor(

async ['@test interior mutations on the model with tracked properties'](assert) {
expectDeprecation(
/The `[^`]+` property(?: path)? was used in a template for the `[^`]+` component without using `this`. This fallback behavior has been deprecated, all properties must be looked up on `this` when used in the template: {{[^}]+}}/
/The `[^`]+` property(?: path)? was used in the `[^`]+` template without using `this`. This fallback behavior has been deprecated, all properties must be looked up on `this` when used in the template: {{[^}]+}}/
);

class Model {
Expand Down Expand Up @@ -272,7 +272,7 @@ moduleFor(

async ['@test exterior mutations on the model with set'](assert) {
expectDeprecation(
/The `[^`]+` property(?: path)? was used in a template for the `[^`]+` component without using `this`. This fallback behavior has been deprecated, all properties must be looked up on `this` when used in the template: {{[^}]+}}/
/The `[^`]+` property(?: path)? was used in the `[^`]+` template without using `this`. This fallback behavior has been deprecated, all properties must be looked up on `this` when used in the template: {{[^}]+}}/
);

this.router.map(function () {
Expand Down Expand Up @@ -333,7 +333,7 @@ moduleFor(

async ['@test exterior mutations on the model with tracked properties'](assert) {
expectDeprecation(
/The `[^`]+` property(?: path)? was used in a template for the `[^`]+` component without using `this`. This fallback behavior has been deprecated, all properties must be looked up on `this` when used in the template: {{[^}]+}}/
/The `[^`]+` property(?: path)? was used in the `[^`]+` template without using `this`. This fallback behavior has been deprecated, all properties must be looked up on `this` when used in the template: {{[^}]+}}/
);

this.router.map(function () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3883,7 +3883,7 @@ moduleFor(

['@test can use `{{component.foo}}` in a template GH#19313']() {
expectDeprecation(
/The `[^`]+` property(?: path)? was used in a template for the `[^`]+` component without using `this`. This fallback behavior has been deprecated, all properties must be looked up on `this` when used in the template: {{[^}]+}}/
/The `[^`]+` property(?: path)? was used in the `[^`]+` template without using `this`. This fallback behavior has been deprecated, all properties must be looked up on `this` when used in the template: {{[^}]+}}/
);

this.registerComponent('foo-bar', {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ moduleFor(
) {
this.addTemplate(
'index',
`<LinkTo id='the-link' @route='index' @query={{hash}}>Index</LinkTo>`
`<LinkTo id='the-link' @route='index' @query={{(hash)}}>Index</LinkTo>`
);

await this.visit('/');
Expand All @@ -163,7 +163,7 @@ moduleFor(
async [`@test it doesn't update controller QP properties on current route when invoked (empty query-params obj, inferred route)`](
assert
) {
this.addTemplate('index', `<LinkTo id='the-link' @query={{hash}}>Index</LinkTo>`);
this.addTemplate('index', `<LinkTo id='the-link' @query={{(hash)}}>Index</LinkTo>`);

await this.visit('/');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2085,7 +2085,7 @@ moduleFor(

this.addTemplate(
'index',
`<LinkTo id='the-link' @route='index' @query={{hash}}>Index</LinkTo>`
`<LinkTo id='the-link' @route='index' @query={{(hash)}}>Index</LinkTo>`
);

await this.visit('/');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ if (ENV._TEMPLATE_ONLY_GLIMMER_COMPONENTS) {

['@test it renders named arguments as reflected properties']() {
expectDeprecation(
/The `[^`]+` property(?: path)? was used in a template for the `[^`]+` component without using `this`. This fallback behavior has been deprecated, all properties must be looked up on `this` when used in the template: {{[^}]+}}/
/The `[^`]+` property(?: path)? was used in the `[^`]+` template without using `this`. This fallback behavior has been deprecated, all properties must be looked up on `this` when used in the template: {{[^}]+}}/
);

this.registerTemplateOnlyComponent('foo-bar', '|{{foo}}|{{this.bar}}|');
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
import { DEBUG } from '@glimmer/env';
import { moduleFor, RenderingTestCase, runTask, defineSimpleModifier } from 'internal-test-helpers';
import {
moduleFor,
RenderingTestCase,
runTask,
defineSimpleHelper,
defineSimpleModifier,
} from 'internal-test-helpers';

import { Component } from '@ember/-internals/glimmer';
import { setModifierManager, modifierCapabilities } from '@glimmer/manager';
Expand Down Expand Up @@ -644,6 +650,20 @@ moduleFor(
});
}, /Cannot use the \(modifier\) keyword yet, as it has not been implemented/);
}

'@feature(EMBER_DYNAMIC_HELPERS_AND_MODIFIERS) Can use a dynamic modifier with a nested dynamic helper'() {
let foo = defineSimpleHelper(() => 'Hello, world!');
let bar = defineSimpleModifier((element, [value]) => (element.innerHTML = value));

this.registerComponent('baz', {
template: '<div {{this.bar (this.foo)}}></div>',
ComponentClass: Component.extend({ tagName: '', foo, bar }),
});

this.render('<Baz/>');
this.assertHTML('<div>Hello, world!</div>');
this.assertStableRerender();
}
}
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ moduleFor(

['@test it does not resolve helpers with a `.` (period)']() {
expectDeprecation(
/The `[^`]+` property(?: path)? was used in a template for the `[^`]+` component without using `this`. This fallback behavior has been deprecated, all properties must be looked up on `this` when used in the template: {{[^}]+}}/
/The `[^`]+` property(?: path)? was used in the `[^`]+` template without using `this`. This fallback behavior has been deprecated, all properties must be looked up on `this` when used in the template: {{[^}]+}}/
);

this.registerHelper('hello.world', () => 'hello world');
Expand Down Expand Up @@ -826,6 +826,20 @@ moduleFor(
}, /Cannot use the \(helper\) keyword yet, as it has not been implemented/);
}

'@feature(EMBER_DYNAMIC_HELPERS_AND_MODIFIERS) Can use a dynamic helper with nested helpers'() {
let foo = defineSimpleHelper(() => 'world!');
let bar = defineSimpleHelper((value) => 'Hello, ' + value);

this.registerComponent('baz', {
template: '{{this.bar (this.foo)}}',
ComponentClass: Component.extend({ foo, bar }),
});

this.render('<Baz/>');
this.assertText('Hello, world!');
this.assertStableRerender();
}

['@test helpers are not computed eagerly when used with if expressions'](assert) {
this.registerHelper('is-ok', () => 'hello');
this.registerHelper('throws-error', () => assert.ok(false, 'helper was computed eagerly'));
Expand Down Expand Up @@ -909,4 +923,95 @@ if (DEBUG) {
}
}
);

moduleFor(
'Helpers test: argument-less helper invocation in named arguments position',
class extends RenderingTestCase {
constructor() {
super(...arguments);

this.registerComponent('bar', {
template: '[{{is-string @content}}][{{@content}}]',
});

this.registerHelper('is-string', ([value]) => typeof value === 'string');
}

['@test invoking an argument-less helper without parens in named argument position is deprecated']() {
this.registerHelper('foo', () => 'Hello, world!');

expectDeprecation(
() => this.render('<Bar @content={{foo}} />', { foo: 'Not it!' }),
new RegExp(
/The `foo` helper was used in the `-top-level` template as /.source +
/`@content={{foo}}`\. This is ambigious between wanting the `@content` /.source +
/argument to be the `foo` helper itself, or the result of invoking the /.source +
/`foo` helper \(current behavior\)\. This implicit invocation behavior /.source +
/has been deprecated\./.source
)
);

this.assertText('[true][Hello, world!]');
this.assertStableRerender();
}

['@test invoking an argument-less helper with parens in named argument position is not deprecated']() {
this.registerHelper('foo', () => 'Hello, world!');

expectNoDeprecation(() => this.render('<Bar @content={{(foo)}} />', { foo: 'Not it!' }));

this.assertText('[true][Hello, world!]');
this.assertStableRerender();
}

['@test invoking an argument-less helper with quotes in named argument position is not deprecated']() {
this.registerHelper('foo', () => 'Hello, world!');

expectNoDeprecation(() => this.render('<Bar @content="{{foo}}" />', { foo: 'Not it!' }));

this.assertText('[true][Hello, world!]');
this.assertStableRerender();
}

['@test passing a local helper in named argument position is not deprecated']() {
let foo = defineSimpleHelper(() => 'Hello, world!');

expectNoDeprecation(() =>
this.render(`{{#let this.foo as |foo|}}<Bar @content={{foo}} />{{/let}}`, { foo })
);

this.assertText('[false][Hello, world!]');
this.assertStableRerender();
}

// TODO: this one really should work, and there is a passing test in glimmer-vm,
// but somehow it doesn't work here. This is almost certainly a VM bug as something
// is trying to call `block.compile()` but `block` is the reference for `this.foo`.
// So the execution stack is probably off-by-one or something.
chancancode marked this conversation as resolved.
Show resolved Hide resolved

['@test invoking a local helper with parens in named argument position is not deprecated']() {
let foo = defineSimpleHelper(() => 'Hello, world!');

expectNoDeprecation(() =>
this.render(`{{#let this.foo as |foo|}}<Bar @content={{(foo)}} />{{/let}}`, { foo })
);

this.assertText('[true][Hello, world!]');
this.assertStableRerender();
}

// TODO: this one doesn't work yet, and there is a failing test in glimmer-vm

['@skip invoking a helper with quotes in named argument position is not deprecated']() {
let foo = defineSimpleHelper(() => 'Hello, world!');

expectNoDeprecation(() =>
this.render(`{{#let this.foo as |foo|}}<Bar @content="{{foo}}" />{{/let}}`, { foo })
);

this.assertText('[true][Hello, world!]');
this.assertStableRerender();
}
}
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,28 @@ moduleFor(
this.assertText('[In layout:] [In block:] Seattle');
}

['@feature(EMBER_NAMED_BLOCKS) <:else> and <:inverse> named blocks']() {
this.registerComponent('yielder', {
template:
'[:else][{{has-block "else"}}][{{yield to="else"}}]' +
'[:inverse][{{has-block "inverse"}}][{{yield to="inverse"}}]',
});

this.render(
'[<Yielder />]' +
'[<Yielder><:else>Hello</:else></Yielder>]' +
'[<Yielder><:inverse>Goodbye</:inverse></Yielder>]'
);

this.assertText(
'[[:else][false][][:inverse][false][]]' +
'[[:else][true][Hello][:inverse][true][Hello]]' +
'[[:else][true][Goodbye][:inverse][true][Goodbye]]'
);

this.assertStableRerender();
}

['@test templates should yield to block inside a nested component']() {
this.registerComponent('outer-comp', {
template: '<div>[In layout:] {{yield}}</div>',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -732,7 +732,7 @@ class EachTest extends AbstractEachTest {

['@test the scoped variable is not available outside the {{#each}} block.']() {
expectDeprecation(
/The `[^`]+` property(?: path)? was used in a template for the `[^`]+` component without using `this`. This fallback behavior has been deprecated, all properties must be looked up on `this` when used in the template: {{[^}]+}}/
/The `[^`]+` property(?: path)? was used in the `[^`]+` template without using `this`. This fallback behavior has been deprecated, all properties must be looked up on `this` when used in the template: {{[^}]+}}/
);

this.makeList(['Yehuda']);
Expand Down Expand Up @@ -953,7 +953,7 @@ class EachTest extends AbstractEachTest {

['@test the scoped variable is not available outside the {{#each}} block']() {
expectDeprecation(
/The `[^`]+` property(?: path)? was used in a template for the `[^`]+` component without using `this`. This fallback behavior has been deprecated, all properties must be looked up on `this` when used in the template: {{[^}]+}}/
/The `[^`]+` property(?: path)? was used in the `[^`]+` template without using `this`. This fallback behavior has been deprecated, all properties must be looked up on `this` when used in the template: {{[^}]+}}/
);

let first = this.createList(['Limbo']);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ moduleFor(

['@test the scoped variable is not available outside the {{#let}} block.']() {
expectDeprecation(
/The `[^`]+` property(?: path)? was used in a template for the `[^`]+` component without using `this`. This fallback behavior has been deprecated, all properties must be looked up on `this` when used in the template: {{[^}]+}}/
/The `[^`]+` property(?: path)? was used in the `[^`]+` template without using `this`. This fallback behavior has been deprecated, all properties must be looked up on `this` when used in the template: {{[^}]+}}/
);

this.render(`{{name}}-{{#let this.other as |name|}}{{name}}{{/let}}-{{name}}`, {
Expand Down Expand Up @@ -247,7 +247,7 @@ moduleFor(

['@test the scoped variable is not available outside the {{#let}} block']() {
expectDeprecation(
/The `[^`]+` property(?: path)? was used in a template for the `[^`]+` component without using `this`. This fallback behavior has been deprecated, all properties must be looked up on `this` when used in the template: {{[^}]+}}/
/The `[^`]+` property(?: path)? was used in the `[^`]+` template without using `this`. This fallback behavior has been deprecated, all properties must be looked up on `this` when used in the template: {{[^}]+}}/
);

this.render(
Expand Down Expand Up @@ -332,7 +332,7 @@ moduleFor(

['@test nested {{#let}} blocks should have access to root context']() {
expectDeprecation(
/The `[^`]+` property(?: path)? was used in a template for the `[^`]+` component without using `this`. This fallback behavior has been deprecated, all properties must be looked up on `this` when used in the template: {{[^}]+}}/
/The `[^`]+` property(?: path)? was used in the `[^`]+` template without using `this`. This fallback behavior has been deprecated, all properties must be looked up on `this` when used in the template: {{[^}]+}}/
);

this.render(
Expand Down
Loading