Skip to content

Commit

Permalink
[ENHANCEMENT+CLEANUP] Better compile time errors for strict mode
Browse files Browse the repository at this point in the history
Removing support for implicit this fallback {{foo}} -> {{this.foo}}

When a strict mode template references an undefined variable, it
really should produce a compile-time error, arguably that is the
point of the feature.

However, it seems like previously a lot of these errors ended up
propagating and is actually handled by the runtime compiler.

The original/primary goal here started out as cleaning up all the
code for implicit this fallback, but as I delete code it eventually
revaled all the places where strict mode was inappropiately using
the same infrastructure ("unresolved free variables", which isn't
really a sensible thing).

Also:

* Removes `{{#with}}`
* Removes "deprecated call" @foo={{bar}}
* Removes unused opcodes
* Syncs keyword list with Ember
  • Loading branch information
chancancode committed Feb 1, 2024
1 parent e813777 commit da81f20
Show file tree
Hide file tree
Showing 41 changed files with 781 additions and 999 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ export class GlimmerishComponents extends RenderTest {
})
'invoking dynamic component (local) via angle brackets'() {
this.registerComponent('Glimmer', 'Foo', 'hello world!');
this.render(`{{#with (component 'Foo') as |Other|}}<Other />{{/with}}`);
this.render(`{{#let (component 'Foo') as |Other|}}<Other />{{/let}}`);

this.assertHTML(`hello world!`);
this.assertStableRerender();
Expand All @@ -280,7 +280,7 @@ export class GlimmerishComponents extends RenderTest {
'invoking dynamic component (local path) via angle brackets'() {
this.registerHelper('hash', (_positional, named) => named);
this.registerComponent('Glimmer', 'Foo', 'hello world!');
this.render(`{{#with (hash Foo=(component 'Foo')) as |Other|}}<Other.Foo />{{/with}}`);
this.render(`{{#let (hash Foo=(component 'Foo')) as |Other|}}<Other.Foo />{{/let}}`);

this.assertHTML(`hello world!`);
this.assertStableRerender();
Expand All @@ -291,7 +291,7 @@ export class GlimmerishComponents extends RenderTest {
})
'invoking dynamic component (local) via angle brackets (ill-advised "htmlish element name" but supported)'() {
this.registerComponent('Glimmer', 'Foo', 'hello world!');
this.render(`{{#with (component 'Foo') as |div|}}<div />{{/with}}`);
this.render(`{{#let (component 'Foo') as |div|}}<div />{{/let}}`);

this.assertHTML(`hello world!`);
this.assertStableRerender();
Expand All @@ -302,7 +302,7 @@ export class GlimmerishComponents extends RenderTest {
})
'invoking dynamic component (local) via angle brackets supports attributes'() {
this.registerComponent('Glimmer', 'Foo', '<div ...attributes>hello world!</div>');
this.render(`{{#with (component 'Foo') as |Other|}}<Other data-test="foo" />{{/with}}`);
this.render(`{{#let (component 'Foo') as |Other|}}<Other data-test="foo" />{{/let}}`);

this.assertHTML(`<div data-test="foo">hello world!</div>`);
this.assertStableRerender();
Expand All @@ -313,7 +313,7 @@ export class GlimmerishComponents extends RenderTest {
})
'invoking dynamic component (local) via angle brackets supports args'() {
this.registerComponent('Glimmer', 'Foo', 'hello {{@name}}!');
this.render(`{{#with (component 'Foo') as |Other|}}<Other @name="world" />{{/with}}`);
this.render(`{{#let (component 'Foo') as |Other|}}<Other @name="world" />{{/let}}`);

this.assertHTML(`hello world!`);
this.assertStableRerender();
Expand All @@ -324,7 +324,7 @@ export class GlimmerishComponents extends RenderTest {
})
'invoking dynamic component (local) via angle brackets supports passing a block'() {
this.registerComponent('Glimmer', 'Foo', 'hello {{yield}}!');
this.render(`{{#with (component 'Foo') as |Other|}}<Other>world</Other>{{/with}}`);
this.render(`{{#let (component 'Foo') as |Other|}}<Other>world</Other>{{/let}}`);

this.assertHTML(`hello world!`);
this.assertStableRerender();
Expand All @@ -351,7 +351,7 @@ export class GlimmerishComponents extends RenderTest {
Foo
);
this.render(
`{{#with (component 'Foo') as |Other|}}<Other @staticNamedArg="static" data-test1={{this.outer}} data-test2="static" @dynamicNamedArg={{this.outer}}>template</Other>{{/with}}`,
`{{#let (component 'Foo') as |Other|}}<Other @staticNamedArg="static" data-test1={{this.outer}} data-test2="static" @dynamicNamedArg={{this.outer}}>template</Other>{{/let}}`,
{ outer: 'outer' }
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export class DebuggerSuite extends RenderTest {
});

this.render(
'{{#with this.foo as |bar|}}{{#if this.a.b}}true{{debugger}}{{else}}false{{debugger}}{{/if}}{{/with}}',
'{{#let this.foo as |bar|}}{{#if this.a.b}}true{{debugger}}{{else}}false{{debugger}}{{/if}}{{/let}}',
expectedContext
);
this.assert.strictEqual(callbackExecuted, 1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ export class ScopeSuite extends RenderTest {
'correct scope - conflicting local names'() {
this.render({
layout: stripTight`
{{#with @a as |item|}}{{@a}}: {{item}},
{{#with @b as |item|}} {{@b}}: {{item}},
{{#with @c as |item|}} {{@c}}: {{item}}{{/with}}
{{/with}}
{{/with}}`,
{{#let @a as |item|}}{{@a}}: {{item}},
{{#let @b as |item|}} {{@b}}: {{item}},
{{#let @c as |item|}} {{@c}}: {{item}}{{/let}}
{{/let}}
{{/let}}`,
args: { a: '"A"', b: '"B"', c: '"C"' },
});

Expand All @@ -25,7 +25,7 @@ export class ScopeSuite extends RenderTest {
'correct scope - conflicting block param and attr names'() {
this.render({
layout:
'Outer: {{@conflict}} {{#with @item as |conflict|}}Inner: {{@conflict}} Block: {{conflict}}{{/with}}',
'Outer: {{@conflict}} {{#let @item as |conflict|}}Inner: {{@conflict}} Block: {{conflict}}{{/let}}',
args: { item: '"from block"', conflict: '"from attr"' },
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -372,14 +372,14 @@ class CurlyScopeTest extends CurlyTest {
stripTight`
<div>
[Outside: {{this.zomg}}]
{{#with this.zomg as |lol|}}
{{#let this.zomg as |lol|}}
[Inside: {{this.zomg}}]
[Inside: {{lol}}]
<FooBar @foo={{this.zomg}}>
[Block: {{this.zomg}}]
[Block: {{lol}}]
</FooBar>
{{/with}}
{{/let}}
</div>`,
{ zomg: 'zomg' }
);
Expand Down Expand Up @@ -419,14 +419,14 @@ class CurlyScopeTest extends CurlyTest {
stripTight`
<div>
[Outside: {{this.zomg}}]
{{#with this.zomg as |lol|}}
{{#let this.zomg as |lol|}}
[Inside: {{this.zomg}}]
[Inside: {{lol}}]
{{#foo-bar foo=this.zomg}}
[Block: {{this.zomg}}]
[Block: {{lol}}]
{{/foo-bar}}
{{/with}}
{{/let}}
</div>`,
{ zomg: 'zomg' }
);
Expand Down Expand Up @@ -991,9 +991,9 @@ class CurlyClosureComponentsTest extends CurlyTest {

this.render(
stripTight`
{{#with (hash comp=(component 'foo-bar')) as |my|}}
{{#let (hash comp=(component 'foo-bar')) as |my|}}
{{#component my.comp arg1="World!"}}Test1{{/component}} Test2
{{/with}}
{{/let}}
`
);

Expand All @@ -1007,9 +1007,9 @@ class CurlyClosureComponentsTest extends CurlyTest {

this.render(
stripTight`
{{#with (hash comp=(component 'foo-bar')) as |my|}}
{{#let (hash comp=(component 'foo-bar')) as |my|}}
{{#component my.comp}}World!{{/component}} Test
{{/with}}
{{/let}}
`
);

Expand All @@ -1023,9 +1023,9 @@ class CurlyClosureComponentsTest extends CurlyTest {

this.render(
stripTight`
{{#with (hash comp=(component 'foo-bar')) as |my|}}
{{#let (hash comp=(component 'foo-bar')) as |my|}}
{{component my.comp arg1="World!"}} Test
{{/with}}
{{/let}}
`
);

Expand All @@ -1039,9 +1039,9 @@ class CurlyClosureComponentsTest extends CurlyTest {

this.render(
stripTight`
{{#with (hash comp=(component 'foo-bar')) as |my|}}
{{#let (hash comp=(component 'foo-bar')) as |my|}}
{{component my.comp}} World!
{{/with}}
{{/let}}
`
);

Expand Down Expand Up @@ -1131,9 +1131,9 @@ class CurlyClosureComponentsTest extends CurlyTest {

this.render(
stripTight`
{{#with (hash comp=(component 'foo-bar')) as |my|}}
{{#let (hash comp=(component 'foo-bar')) as |my|}}
{{my.comp}} World!
{{/with}}
{{/let}}
`
);

Expand Down Expand Up @@ -1190,11 +1190,11 @@ class CurlyClosureComponentsTest extends CurlyTest {

this.render(
stripTight`
{{#with (component "foo-bar" "outer 1" "outer 2" a="outer a" b="outer b" c="outer c" e="outer e") as |outer|}}
{{#with (component outer "inner 1" a="inner a" d="inner d" e="inner e") as |inner|}}
{{#let (component "foo-bar" "outer 1" "outer 2" a="outer a" b="outer b" c="outer c" e="outer e") as |outer|}}
{{#let (component outer "inner 1" a="inner a" d="inner d" e="inner e") as |inner|}}
{{#component inner "invocation 1" "invocation 2" a="invocation a" b="invocation b"}}---{{/component}}
{{/with}}
{{/with}}
{{/let}}
{{/let}}
`
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@ class ArrayTest extends RenderTest {
@test
'returns an array'() {
this.render(strip`
{{#with (array "Sergio") as |people|}}
{{#let (array "Sergio") as |people|}}
{{#each people as |personName|}}
{{personName}}
{{/each}}
{{/with}}`);
{{/let}}`);

this.assertHTML('Sergio');

Expand All @@ -20,11 +20,11 @@ class ArrayTest extends RenderTest {
@test
'can have more than one value'() {
this.render(strip`
{{#with (array "Sergio" "Robert") as |people|}}
{{#let (array "Sergio" "Robert") as |people|}}
{{#each people as |personName|}}
{{personName}},
{{/each}}
{{/with}}`);
{{/let}}`);

this.assertHTML('Sergio,Robert,');

Expand All @@ -34,11 +34,11 @@ class ArrayTest extends RenderTest {
@test
'binds values when variables are used'() {
this.render(
strip`{{#with (array this.personOne) as |people|}}
strip`{{#let (array this.personOne) as |people|}}
{{#each people as |personName|}}
{{personName}}
{{/each}}
{{/with}}`,
{{/let}}`,
{
personOne: 'Tom',
}
Expand All @@ -58,11 +58,11 @@ class ArrayTest extends RenderTest {
@test
'binds multiple values when variables are used'() {
this.render(
strip`{{#with (array this.personOne this.personTwo) as |people|}}
strip`{{#let (array this.personOne this.personTwo) as |people|}}
{{#each people as |personName|}}
{{personName}},
{{/each}}
{{/with}}`,
{{/let}}`,
{
personOne: 'Tom',
personTwo: 'Yehuda',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ class DynamicModifiersResolutionModeTest extends RenderTest {
this.registerComponent('TemplateOnly', 'Bar', '<div {{x.foo}}></div>');
},
syntaxErrorFor(
'You attempted to invoke a path (`{{#x.foo}}`) as a modifier, but x was not in scope. Try adding `this` to the beginning of the path',
'You attempted to invoke a path (`{{x.foo}}`) as a modifier, but x was not in scope',
'{{x.foo}}',
'an unknown module',
1,
Expand Down
Loading

0 comments on commit da81f20

Please sign in to comment.