Skip to content

Commit ed22236

Browse files
rwjbluerwjblue
authored
Merge pull request #1331 from snewcomer/sn/this-fallback
2 parents 847dec7 + 89adeba commit ed22236

File tree

8 files changed

+20
-158
lines changed

8 files changed

+20
-158
lines changed

‎benchmark/benchmarks/krausest/lib/components/Row.hbs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
<tr class={{if @item.selected "danger"}}>
22
<td class="col-md-1">{{@item.id}}</td>
3-
<td class="col-md-4"><a {{on "click" onSelect}}>{{@item.label}}</a></td>
3+
<td class="col-md-4"><a {{on "click" this.onSelect}}>{{@item.label}}</a></td>
44
<td class="col-md-1"><a><span class="glyphicon glyphicon-remove" aria-hidden="true"></span></a></td>
55
<td class="col-md-6"></td>
66
</tr>

‎packages/@glimmer/integration-tests/test/ember-component-test.ts

+2-33
Original file line numberDiff line numberDiff line change
@@ -542,7 +542,7 @@ class CurlyScopeTest extends CurlyTest {
542542
}
543543

544544
@test
545-
'correct scope - self lookup inside #each'(assert: Assert) {
545+
'correct scope - self lookup inside #each'() {
546546
this.registerComponent('TemplateOnly', 'SubItem', `<p>{{@name}}</p>`);
547547

548548
let subitems = [{ id: 0 }, { id: 1 }, { id: 42 }];
@@ -552,8 +552,7 @@ class CurlyScopeTest extends CurlyTest {
552552
<div>
553553
{{#each this.items key="id" as |item|}}
554554
<SubItem @name={{this.id}} />
555-
{{! Intentional property fallback to test self lookup }}
556-
<SubItem @name={{id}} />
555+
<SubItem @name={{this.id}} />
557556
<SubItem @name={{item.id}} />
558557
{{/each}}
559558
</div>`,
@@ -569,10 +568,6 @@ class CurlyScopeTest extends CurlyTest {
569568
</div>
570569
`
571570
);
572-
573-
assert.validateDeprecations(
574-
/The `id` property was used in the `.*` template without using `this`/
575-
);
576571
}
577572

578573
@test
@@ -1633,32 +1628,6 @@ class CurlyGlimmerComponentTest extends CurlyTest {
16331628
this.assertHTML('Foo');
16341629
this.assertStableNodes();
16351630
}
1636-
1637-
@test
1638-
'Can use implicit this fallback for `component.name` emberjs/ember.js#19313'(assert: Assert) {
1639-
this.registerComponent(
1640-
'Glimmer',
1641-
'Outer',
1642-
'{{component.name}}',
1643-
class extends GlimmerishComponent {
1644-
get component() {
1645-
return { name: 'Foo' };
1646-
}
1647-
}
1648-
);
1649-
1650-
this.render('<Outer />');
1651-
this.assertHTML('Foo');
1652-
1653-
this.rerender();
1654-
1655-
this.assertHTML('Foo');
1656-
this.assertStableNodes();
1657-
1658-
assert.validateDeprecations(
1659-
/The `component\.name` property path was used in the `.*` template without using `this`/
1660-
);
1661-
}
16621631
}
16631632

16641633
class CurlyTeardownTest extends CurlyTest {

‎packages/@glimmer/integration-tests/test/helpers/dynamic-helpers-test.ts

-23
Original file line numberDiff line numberDiff line change
@@ -20,29 +20,6 @@ class DynamicHelpersResolutionModeTest extends RenderTest {
2020
this.assertStableRerender();
2121
}
2222

23-
@test
24-
'Can invoke a helper definition based on this fallback lookup in resolution mode'(
25-
assert: Assert
26-
) {
27-
const foo = defineSimpleHelper(() => 'Hello, world!');
28-
this.registerComponent(
29-
'Glimmer',
30-
'Bar',
31-
'{{x.foo}}',
32-
class extends GlimmerishComponent {
33-
x = { foo };
34-
}
35-
);
36-
37-
this.render('<Bar/>');
38-
this.assertHTML('Hello, world!');
39-
this.assertStableRerender();
40-
41-
assert.validateDeprecations(
42-
/The `x\.foo` property path was used in the `.*` template without using `this`/
43-
);
44-
}
45-
4623
@test
4724
'Can use a dynamic helper with nested helpers'() {
4825
const foo = defineSimpleHelper(() => 'world!');

‎packages/@glimmer/integration-tests/test/updating-test.ts

+13-28
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ class UpdatingTest extends RenderTest {
101101
this.render(
102102
stripTight`
103103
<div>
104-
[{{this.[]}}]
104+
[{{this.['']}}]
105105
[{{this.[1]}}]
106106
[{{this.[undefined]}}]
107107
[{{this.[null]}}]
@@ -110,7 +110,7 @@ class UpdatingTest extends RenderTest {
110110
[{{this.[this]}}]
111111
[{{this.[foo.bar]}}]
112112
113-
[{{this.nested.[]}}]
113+
[{{this.nested.['']}}]
114114
[{{this.nested.[1]}}]
115115
[{{this.nested.[undefined]}}]
116116
[{{this.nested.[null]}}]
@@ -125,7 +125,7 @@ class UpdatingTest extends RenderTest {
125125

126126
this.assertHTML(stripTight`
127127
<div>
128-
[empty string]
128+
[]
129129
[1]
130130
[undefined]
131131
[null]
@@ -134,7 +134,7 @@ class UpdatingTest extends RenderTest {
134134
[this]
135135
[foo.bar]
136136
137-
[empty string]
137+
[]
138138
[1]
139139
[undefined]
140140
[null]
@@ -159,7 +159,7 @@ class UpdatingTest extends RenderTest {
159159

160160
this.assertHTML(stripTight`
161161
<div>
162-
[EMPTY STRING]
162+
[]
163163
[ONE]
164164
[UNDEFINED]
165165
[NULL]
@@ -168,7 +168,7 @@ class UpdatingTest extends RenderTest {
168168
[THIS]
169169
[FOO.BAR]
170170
171-
[EMPTY STRING]
171+
[]
172172
[ONE]
173173
[UNDEFINED]
174174
[NULL]
@@ -196,7 +196,7 @@ class UpdatingTest extends RenderTest {
196196

197197
this.assertHTML(stripTight`
198198
<div>
199-
[empty string]
199+
[]
200200
[1]
201201
[undefined]
202202
[null]
@@ -205,7 +205,7 @@ class UpdatingTest extends RenderTest {
205205
[this]
206206
[foo.bar]
207207
208-
[empty string]
208+
[]
209209
[1]
210210
[undefined]
211211
[null]
@@ -215,10 +215,6 @@ class UpdatingTest extends RenderTest {
215215
[foo.bar]
216216
</div>
217217
`);
218-
219-
assert.validateDeprecations(
220-
/The `` property was used in the `.*` template without using `this`/
221-
);
222218
}
223219

224220
@test
@@ -935,8 +931,8 @@ class UpdatingTest extends RenderTest {
935931
foo: "{{foo}}";
936932
bar: "{{bar}}";
937933
value: "{{this.value}}";
938-
echo foo: "{{echo foo}}";
939-
echo bar: "{{echo bar}}";
934+
echo foo: "{{echo this.foo}}";
935+
echo bar: "{{echo this.bar}}";
940936
echo value: "{{echo this.value}}";
941937
942938
-----
@@ -946,7 +942,7 @@ class UpdatingTest extends RenderTest {
946942
bar: "{{bar}}";
947943
value: "{{this.value}}";
948944
echo foo: "{{echo foo}}";
949-
echo bar: "{{echo bar}}";
945+
echo bar: "{{echo this.bar}}";
950946
echo value: "{{echo this.value}}";
951947
952948
-----
@@ -967,7 +963,7 @@ class UpdatingTest extends RenderTest {
967963
foo: "{{foo}}";
968964
bar: "{{bar}}";
969965
value: "{{this.value}}";
970-
echo foo: "{{echo foo}}";
966+
echo foo: "{{echo this.foo}}";
971967
echo bar: "{{echo bar}}";
972968
echo value: "{{echo this.value}}";
973969
{{/with}}
@@ -1101,19 +1097,12 @@ class UpdatingTest extends RenderTest {
11011097
</div>`,
11021098
'After reset'
11031099
);
1104-
1105-
assert.validateDeprecations(
1106-
/The `foo` property path was used in the `.*` template without using `this`/,
1107-
/The `bar` property path was used in the `.*` template without using `this`/,
1108-
/The `bar` property path was used in the `.*` template without using `this`/,
1109-
/The `foo` property path was used in the `.*` template without using `this`/
1110-
);
11111100
}
11121101

11131102
@test
11141103
'block arguments (ensure balanced push/pop)'() {
11151104
let person = { name: { first: 'Godfrey', last: 'Chan' } };
1116-
this.render('<div>{{#with this.person.name.first as |f|}}{{f}}{{/with}}{{f}}</div>', {
1105+
this.render('<div>{{#with this.person.name.first as |f|}}{{f}}{{/with}}{{this.f}}</div>', {
11171106
person,
11181107
f: 'Outer',
11191108
});
@@ -1124,10 +1113,6 @@ class UpdatingTest extends RenderTest {
11241113
this.rerender({ person });
11251114

11261115
this.assertHTML('<div>GodfreakOuter</div>', 'After updating');
1127-
1128-
assert.validateDeprecations(
1129-
/The `f` property was used in the `.*` template without using `this`/
1130-
);
11311116
}
11321117

11331118
@test

‎packages/@glimmer/interfaces/lib/compile/encoder.d.ts

-2
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,6 @@ export type ResolveOptionalHelperOp = [
8888
op1: WireFormat.Expressions.Expression,
8989
op2: {
9090
ifHelper: (handle: number, name: string, moduleName: string) => void;
91-
ifFallback: (name: string, moduleName: string) => void;
9291
}
9392
];
9493

@@ -99,7 +98,6 @@ export type ResolveOptionalComponentOrHelperOp = [
9998
ifComponent: (component: CompileTimeComponent) => void;
10099
ifHelper: (handle: number) => void;
101100
ifValue: (handle: number) => void;
102-
ifFallback: (name: string) => void;
103101
}
104102
];
105103

‎packages/@glimmer/opcode-compiler/lib/opcode-builder/helpers/resolution.ts

+3-8
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,7 @@ export function resolveOptionalHelper(
309309
resolver: CompileTimeResolver,
310310
constants: CompileTimeConstants & ResolutionTimeConstants,
311311
meta: ContainingMetadata,
312-
[, expr, { ifHelper, ifFallback }]: ResolveOptionalHelperOp
312+
[, expr, { ifHelper }]: ResolveOptionalHelperOp
313313
): void {
314314
assert(
315315
isGetFreeOptionalHelper(expr) || isGetFreeDeprecatedHelper(expr),
@@ -320,9 +320,7 @@ export function resolveOptionalHelper(
320320
let name = upvars[expr[1]];
321321
let helper = resolver.lookupHelper(name, owner);
322322

323-
if (helper === null) {
324-
ifFallback(name, meta.moduleName);
325-
} else {
323+
if (helper) {
326324
ifHelper(constants.helper(helper, name), name, meta.moduleName);
327325
}
328326
}
@@ -334,7 +332,7 @@ export function resolveOptionalComponentOrHelper(
334332
resolver: CompileTimeResolver,
335333
constants: CompileTimeConstants & ResolutionTimeConstants,
336334
meta: ContainingMetadata,
337-
[, expr, { ifComponent, ifHelper, ifValue, ifFallback }]: ResolveOptionalComponentOrHelperOp
335+
[, expr, { ifComponent, ifHelper, ifValue }]: ResolveOptionalComponentOrHelperOp
338336
): void {
339337
assert(
340338
isGetFreeOptionalComponentOrHelper(expr),
@@ -396,10 +394,7 @@ export function resolveOptionalComponentOrHelper(
396394

397395
if (helper !== null) {
398396
ifHelper(constants.helper(helper, name));
399-
return;
400397
}
401-
402-
ifFallback(name);
403398
}
404399
}
405400

‎packages/@glimmer/opcode-compiler/lib/syntax/expressions.ts

+1-44
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import { isGetFreeHelper } from '../opcode-builder/helpers/resolution';
1212
import { SimpleArgs } from '../opcode-builder/helpers/shared';
1313
import { Call, CallDynamic, Curry, PushPrimitiveReference } from '../opcode-builder/helpers/vm';
1414
import { Compilers, PushExpressionOp } from './compilers';
15-
import { DEBUG } from '@glimmer/env';
1615

1716
export const EXPRESSIONS = new Compilers<PushExpressionOp, ExpressionSexpOpcode>();
1817

@@ -57,23 +56,7 @@ EXPRESSIONS.add(SexpOpcodes.GetStrictFree, (op, [, sym, _path]) => {
5756
});
5857
});
5958

60-
EXPRESSIONS.add(SexpOpcodes.GetFreeAsFallback, (op, [, freeVar, path]) => {
61-
op(HighLevelResolutionOpcode.ResolveLocal, freeVar, (name: string, moduleName: string) => {
62-
if (DEBUG) {
63-
let propertyPath = path ? [name, ...path].join('.') : name;
64-
65-
deprecate(
66-
`The \`${propertyPath}\` property path was used in the \`${moduleName}\` template without using \`this\`. This fallback behavior has been deprecated, all properties must be looked up on \`this\` when used in the template: {{this.${propertyPath}}}`,
67-
false,
68-
{
69-
id: 'this-property-fallback',
70-
}
71-
);
72-
}
73-
74-
op(Op.GetVariable, 0);
75-
op(Op.GetProperty, name);
76-
});
59+
EXPRESSIONS.add(SexpOpcodes.GetFreeAsFallback, (op, [, , path]) => {
7760
withPath(op, path);
7861
});
7962

@@ -93,19 +76,6 @@ EXPRESSIONS.add(SexpOpcodes.GetFreeAsHelperHeadOrThisFallback, (op, expr) => {
9376
ifHelper: (handle: number) => {
9477
Call(op, handle, null, null);
9578
},
96-
97-
ifFallback: (name: string, moduleName: string) => {
98-
deprecate(
99-
`The \`${name}\` property was used in the \`${moduleName}\` template without using \`this\`. This fallback behavior has been deprecated, all properties must be looked up on \`this\` when used in the template: {{this.${name}}}`,
100-
false,
101-
{
102-
id: 'this-property-fallback',
103-
}
104-
);
105-
106-
op(Op.GetVariable, 0);
107-
op(Op.GetProperty, name);
108-
},
10979
});
11080
});
11181
});
@@ -140,19 +110,6 @@ EXPRESSIONS.add(SexpOpcodes.GetFreeAsDeprecatedHelperHeadOrThisFallback, (op, ex
140110

141111
Call(op, handle, null, null);
142112
},
143-
144-
ifFallback: (name: string, moduleName: string) => {
145-
deprecate(
146-
`The \`${name}\` property was used in the \`${moduleName}\` template without using \`this\`. This fallback behavior has been deprecated, all properties must be looked up on \`this\` when used in the template: {{this.${name}}}`,
147-
false,
148-
{
149-
id: 'this-property-fallback',
150-
}
151-
);
152-
153-
op(Op.GetVariable, 0);
154-
op(Op.GetProperty, name);
155-
},
156113
});
157114
});
158115
});

0 commit comments

Comments
 (0)