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

Deprecate implicit this property lookup fallback #19371

Merged
merged 1 commit into from
Feb 2, 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.74.0",
"@glimmer/vm-babel-plugins": "0.74.1",
"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.74.0",
"@glimmer/compiler": "0.74.1",
"@glimmer/env": "^0.1.7",
"@glimmer/global-context": "0.74.0",
"@glimmer/interfaces": "0.74.0",
"@glimmer/manager": "0.74.0",
"@glimmer/destroyable": "0.74.0",
"@glimmer/owner": "0.74.0",
"@glimmer/node": "0.74.0",
"@glimmer/opcode-compiler": "0.74.0",
"@glimmer/program": "0.74.0",
"@glimmer/reference": "0.74.0",
"@glimmer/runtime": "0.74.0",
"@glimmer/validator": "0.74.0",
"@glimmer/global-context": "0.74.1",
"@glimmer/interfaces": "0.74.1",
"@glimmer/manager": "0.74.1",
"@glimmer/destroyable": "0.74.1",
"@glimmer/owner": "0.74.1",
"@glimmer/node": "0.74.1",
"@glimmer/opcode-compiler": "0.74.1",
"@glimmer/program": "0.74.1",
"@glimmer/reference": "0.74.1",
"@glimmer/runtime": "0.74.1",
"@glimmer/validator": "0.74.1",
"@simple-dom/document": "^1.4.0",
"@types/qunit": "^2.9.1",
"@types/rsvp": "^4.0.3",
Expand Down
17 changes: 17 additions & 0 deletions packages/@ember/-internals/environment/lib/env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,23 @@ export const ENV = {
*/
_RERENDER_LOOP_LIMIT: 1000,

/**
Allows disabling the implicit this property fallback deprecation. This could be useful
as a way to control the volume of deprecations that are issued by temporarily disabling
the implicit this fallback deprecations, which would allow the other deprecations to be more easily
identified in the console).

NOTE: The fallback behavior **will be removed** in Ember 4.0.0, disabling **_IS NOT_**
a viable strategy for handling this deprecation.

@property _DISABLE_PROPERTY_FALLBACK_DEPRECATION
@for EmberENV
@type boolean
@default false
@private
*/
_DISABLE_PROPERTY_FALLBACK_DEPRECATION: false,

EMBER_LOAD_HOOKS: {} as {
[hook: string]: Function[];
},
Expand Down
20 changes: 18 additions & 2 deletions packages/@ember/-internals/glimmer/lib/environment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,10 @@ setGlobalContext({

if (!override) throw new Error(`deprecation override for ${id} not found`);

deprecate(override.message ?? msg, Boolean(test), override);
// allow deprecations to be disabled in the VM_DEPRECATION_OVERRIDES array below
if (!override.disabled) {
deprecate(override.message ?? msg, Boolean(test), override);
}
}
},
});
Expand All @@ -91,7 +94,10 @@ if (DEBUG) {

// VM Assertion/Deprecation overrides

const VM_DEPRECATION_OVERRIDES: (DeprecationOptions & { message?: string })[] = [
const VM_DEPRECATION_OVERRIDES: (DeprecationOptions & {
disabled?: boolean;
message?: string;
})[] = [
{
id: 'autotracking.mutation-after-consumption',
until: '4.0.0',
Expand All @@ -100,6 +106,16 @@ const VM_DEPRECATION_OVERRIDES: (DeprecationOptions & { message?: string })[] =
enabled: '3.21.0',
},
},
{
id: 'this-property-fallback',
disabled: ENV._DISABLE_PROPERTY_FALLBACK_DEPRECATION,
url: 'https://deprecations.emberjs.com/v3.x#toc_this-property-fallback',
until: '4.0.0',
for: 'ember-source',
rwjblue marked this conversation as resolved.
Show resolved Hide resolved
since: {
enabled: '3.26.0',
},
},
];

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

['@test sharing a template between engine and application has separate refinements']() {
this.assert.expect(1);
this.assert.expect(2);

let sharedTemplate = compile(strip`
<h1>{{this.contextType}}</h1>
Expand All @@ -290,6 +290,10 @@ moduleFor(
{{outlet}}
`);

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: {{[^}]+}}/
);

this.add('template:application', sharedTemplate);
this.add(
'controller:application',
Expand Down Expand Up @@ -335,7 +339,11 @@ moduleFor(
}

['@test sharing a layout between engine and application has separate refinements']() {
this.assert.expect(1);
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: {{[^}]+}}/
);

let sharedLayout = compile(strip`
{{ambiguous-curlies}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,10 @@ 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: {{[^}]+}}/
);

this.add(
'route:application',
Route.extend({
Expand Down Expand Up @@ -138,6 +142,10 @@ 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: {{[^}]+}}/
);

this.router.map(function () {
this.route('color', { path: '/:color' });
});
Expand Down Expand Up @@ -195,6 +203,10 @@ 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: {{[^}]+}}/
);

class Model {
@tracked color;

Expand Down Expand Up @@ -259,6 +271,10 @@ 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: {{[^}]+}}/
);

this.router.map(function () {
this.route('color', { path: '/:color' });
});
Expand Down Expand Up @@ -316,6 +332,10 @@ 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: {{[^}]+}}/
);

this.router.map(function () {
this.route('color', { path: '/:color' });
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ moduleFor(
this.registerComponent('foo-bar', { template: 'hello' });

this.render(
'{{foo-bar classNameBindings="model.someInitiallyTrueProperty model.someInitiallyFalseProperty model.someInitiallyUndefinedProperty :static model.isBig:big model.isOpen:open:closed model.isUp::down model.bar:isTruthy:isFalsy"}}',
'{{foo-bar classNameBindings="this.model.someInitiallyTrueProperty this.model.someInitiallyFalseProperty this.model.someInitiallyUndefinedProperty :static this.model.isBig:big this.model.isOpen:open:closed this.model.isUp::down this.model.bar:isTruthy:isFalsy"}}',
{
model: {
someInitiallyTrueProperty: true,
Expand Down Expand Up @@ -344,7 +344,7 @@ moduleFor(

['@test const bindings can be set as attrs']() {
this.registerComponent('foo-bar', { template: 'hello' });
this.render('{{foo-bar classNameBindings="foo:enabled:disabled"}}', {
this.render('{{foo-bar classNameBindings="this.foo:enabled:disabled"}}', {
foo: true,
});

Expand Down Expand Up @@ -697,7 +697,7 @@ moduleFor(
['@test it should merge classBinding with class']() {
this.registerComponent('foo-bar', { template: 'hello' });

this.render('{{foo-bar classBinding="birdman:respeck" class="myName"}}', {
this.render('{{foo-bar classBinding="this.birdman:respeck" class="myName"}}', {
birdman: true,
});

Expand All @@ -719,7 +719,7 @@ moduleFor(
['@test it should apply classBinding with only truthy condition']() {
this.registerComponent('foo-bar', { template: 'hello' });

this.render('{{foo-bar classBinding="myName:respeck"}}', {
this.render('{{foo-bar classBinding="this.myName:respeck"}}', {
myName: true,
});

Expand All @@ -741,7 +741,7 @@ moduleFor(
['@test it should apply classBinding with only falsy condition']() {
this.registerComponent('foo-bar', { template: 'hello' });

this.render('{{foo-bar classBinding="myName::shade"}}', {
this.render('{{foo-bar classBinding="this.myName::shade"}}', {
myName: false,
});

Expand All @@ -763,7 +763,7 @@ moduleFor(
['@test it should apply nothing when classBinding is falsy but only supplies truthy class']() {
this.registerComponent('foo-bar', { template: 'hello' });

this.render('{{foo-bar classBinding="myName:respeck"}}', {
this.render('{{foo-bar classBinding="this.myName:respeck"}}', {
myName: false,
});

Expand All @@ -785,7 +785,7 @@ moduleFor(
['@test it should apply nothing when classBinding is truthy but only supplies falsy class']() {
this.registerComponent('foo-bar', { template: 'hello' });

this.render('{{foo-bar classBinding="myName::shade"}}', { myName: true });
this.render('{{foo-bar classBinding="this.myName::shade"}}', { myName: true });

this.assertComponentElement(this.firstChild, {
tagName: 'div',
Expand All @@ -805,7 +805,7 @@ moduleFor(
['@test it should apply classBinding with falsy condition']() {
this.registerComponent('foo-bar', { template: 'hello' });

this.render('{{foo-bar classBinding="swag:fresh:scrub"}}', {
this.render('{{foo-bar classBinding="this.swag:fresh:scrub"}}', {
swag: false,
});

Expand All @@ -827,7 +827,7 @@ moduleFor(
['@test it should apply classBinding with truthy condition']() {
this.registerComponent('foo-bar', { template: 'hello' });

this.render('{{foo-bar classBinding="swag:fresh:scrub"}}', {
this.render('{{foo-bar classBinding="this.swag:fresh:scrub"}}', {
swag: true,
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1501,7 +1501,7 @@ applyMixins(
title: 'hash value',
setup() {
this.registerComponent('my-comp', {
template: '{{component component}}',
template: '{{component this.component}}',
});

this.render('{{my-comp component=(component "change-button" val=this.model.val2)}}');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3734,6 +3734,10 @@ 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: {{[^}]+}}/
);

this.registerComponent('foo-bar', {
template: '{{component.foo}}',
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -985,7 +985,7 @@ moduleFor(
'Did you mean `<Input @type="checkbox" @checked={{...}} />`?';

expectWarning(() => {
this.render(`<Input @type="checkbox" @value={{value}} />`, {
this.render(`<Input @type="checkbox" @value={{this.value}} />`, {
value: true,
});
}, message);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -776,7 +776,7 @@ moduleFor(
'Did you mean `<Input @type="checkbox" @checked={{...}} />`?';

expectWarning(() => {
this.render(`{{input type="checkbox" value=value}}`, {
this.render(`{{input type="checkbox" value=this.value}}`, {
value: true,
});
}, message);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ moduleFor(
'index',
`
<h3 class="home">Home</h3>
{{#link-to 'about' id='about-link' classNameBindings='foo:foo-is-true:foo-is-false'}}About{{/link-to}}
{{#link-to 'about' id='about-link' classNameBindings='this.foo:foo-is-true:foo-is-false'}}About{{/link-to}}
`
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ if (ENV._TEMPLATE_ONLY_GLIMMER_COMPONENTS) {
['@test it can render named arguments']() {
this.registerTemplateOnlyComponent('foo-bar', '|{{@foo}}|{{@bar}}|');

this.render('{{foo-bar foo=foo bar=bar}}', {
this.render('{{foo-bar foo=this.foo bar=this.bar}}', {
foo: 'foo',
bar: 'bar',
});
Expand All @@ -197,9 +197,13 @@ 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: {{[^}]+}}/
);

this.registerTemplateOnlyComponent('foo-bar', '|{{foo}}|{{this.bar}}|');

this.render('{{foo-bar foo=foo bar=bar}}', {
this.render('{{foo-bar foo=this.foo bar=this.bar}}', {
foo: 'foo',
bar: 'bar',
});
Expand All @@ -224,7 +228,7 @@ if (ENV._TEMPLATE_ONLY_GLIMMER_COMPONENTS) {
['@test it has curly component features']() {
this.registerTemplateOnlyComponent('foo-bar', 'hello');

this.render('{{foo-bar tagName="p" class=class}}', {
this.render('{{foo-bar tagName="p" class=this.class}}', {
class: 'foo bar',
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ 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: {{[^}]+}}/
);

this.registerHelper('hello.world', () => 'hello world');

this.render('{{hello.world}}', {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -731,6 +731,10 @@ 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: {{[^}]+}}/
);

this.makeList(['Yehuda']);

this.render(`{{name}}-{{#each this.list as |name|}}{{name}}{{/each}}-{{name}}`, {
Expand Down Expand Up @@ -948,6 +952,10 @@ 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: {{[^}]+}}/
);

let first = this.createList(['Limbo']);
let fifth = this.createList(['Wrath']);
let ninth = this.createList(['Treachery']);
Expand Down
Loading