Skip to content

Commit c8824b0

Browse files
authored
Merge pull request #13649 from chadhietala/dashed-properties
[GLIMMER] Dashed property names
2 parents 2f3230a + 1589903 commit c8824b0

File tree

5 files changed

+72
-37
lines changed

5 files changed

+72
-37
lines changed

packages/ember-glimmer/lib/environment.js

Lines changed: 58 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,43 @@ export default class Environment extends GlimmerEnvironment {
121121
};
122122
}
123123

124+
// Hello future traveler, welcome to the world of syntax refinement.
125+
// The method below is called by Glimmer's runtime compiler to allow
126+
// us to take generic statement syntax and refine it to more meaniful
127+
// syntax for Ember's use case. This on the fly switch-a-roo sounds fine
128+
// and dandy, however Ember has precedence on statement refinement that you
129+
// need to be aware of. The presendence for language constructs is as follows:
130+
//
131+
// ------------------------
132+
// Native & Built-in Syntax
133+
// ------------------------
134+
// User-land components
135+
// ------------------------
136+
// User-land helpers
137+
// ------------------------
138+
//
139+
// The one caveat here is that Ember also allows for dashed references that are
140+
// not a component or helper:
141+
//
142+
// export default Component.extend({
143+
// 'foo-bar': 'LAME'
144+
// });
145+
//
146+
// {{foo-bar}}
147+
//
148+
// The heuristic for the above situation is a dashed "key" in inline form
149+
// that does not resolve to a defintion. In this case refine statement simply
150+
// isn't going to return any syntax and the Glimmer engine knows how to handle
151+
// this case.
152+
124153
refineStatement(statement) {
154+
// 1. resolve any native syntax – if, unless, with, each, and partial
155+
let nativeSyntax = super.refineStatement(statement);
156+
157+
if (nativeSyntax) {
158+
return nativeSyntax;
159+
}
160+
125161
let {
126162
isSimple,
127163
isInline,
@@ -133,37 +169,37 @@ export default class Environment extends GlimmerEnvironment {
133169
templates
134170
} = statement;
135171

136-
if (key !== 'partial' && isSimple && (isInline || isBlock)) {
172+
assert(`You attempted to overwrite the built-in helper "${key}" which is not allowed. Please rename the helper.`, !(builtInHelpers[key] && this.owner.hasRegistration(`helper:${key}`)));
173+
174+
if (isSimple && (isInline || isBlock)) {
175+
// 2. built-in syntax
137176
if (key === 'component') {
138177
return new DynamicComponentSyntax({ args, templates });
139178
} else if (key === 'outlet') {
140179
return new OutletSyntax({ args });
180+
}
181+
182+
let internalKey = builtInComponents[key];
183+
let definition = null;
184+
185+
if (internalKey) {
186+
definition = this.getComponentDefinition([internalKey]);
141187
} else if (key.indexOf('-') >= 0) {
142-
let definition = this.getComponentDefinition(path);
143-
144-
if (definition) {
145-
wrapClassBindingAttribute(args);
146-
wrapClassAttribute(args);
147-
return new CurlyComponentSyntax({ args, definition, templates });
148-
} else if (isBlock && !this.hasHelper(key)) {
149-
assert(`A helper named '${path[0]}' could not be found`, false);
150-
}
151-
} else {
152-
// Check if it's a keyword
153-
let mappedKey = builtInComponents[key];
154-
if (mappedKey) {
155-
let definition = this.getComponentDefinition([mappedKey]);
156-
wrapClassBindingAttribute(args);
157-
wrapClassAttribute(args);
158-
return new CurlyComponentSyntax({ args, definition, templates });
159-
}
188+
definition = this.getComponentDefinition(path);
189+
}
190+
191+
if (definition) {
192+
wrapClassBindingAttribute(args);
193+
wrapClassAttribute(args);
194+
return new CurlyComponentSyntax({ args, definition, templates });
160195
}
196+
197+
assert(`Could not find component named "${key}" (no component or template with that name was found)`, !isBlock || this.hasHelper(key));
161198
}
162199

163-
let nativeSyntax = super.refineStatement(statement);
164-
assert(`Helpers may not be used in the block form, for example {{#${key}}}{{/${key}}}. Please use a component, or alternatively use the helper in combination with a built-in Ember helper, for example {{#if (${key})}}{{/if}}.`, !nativeSyntax && key && this.hasHelper(key) ? !isBlock : true);
200+
assert(`Helpers may not be used in the block form, for example {{#${key}}}{{/${key}}}. Please use a component, or alternatively use the helper in combination with a built-in Ember helper, for example {{#if (${key})}}{{/if}}.`, !isBlock || !this.hasHelper(key));
201+
165202
assert(`Helpers may not be used in the element form.`, !nativeSyntax && key && this.hasHelper(key) ? !isModifier : true);
166-
return nativeSyntax;
167203
}
168204

169205
hasComponentDefinition() {
@@ -179,8 +215,6 @@ export default class Environment extends GlimmerEnvironment {
179215

180216
if (ComponentClass || layout) {
181217
definition = this._components[name] = new CurlyComponentDefinition(name, ComponentClass, layout);
182-
} else if (!this.hasHelper(name)) {
183-
assert(`Glimmer error: Could not find component named "${name}" (no component or template with that name was found)`, !!(ComponentClass || layout));
184218
}
185219
}
186220

packages/ember-glimmer/lib/syntax/dynamic-component.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { ArgsSyntax, StatementSyntax } from 'glimmer-runtime';
22
import { ConstReference, isConst, UNDEFINED_REFERENCE } from 'glimmer-reference';
3+
import { assert } from 'ember-metal/debug';
34

45
function dynamicComponentFor(vm) {
56
let env = vm.env;
@@ -10,6 +11,8 @@ function dynamicComponentFor(vm) {
1011
let name = nameRef.value();
1112
let definition = env.getComponentDefinition([name]);
1213

14+
assert(`Could not find component named "${name}" (no component or template with that name was found)`, definition);
15+
1316
return new ConstReference(definition);
1417
} else {
1518
return new DynamicComponentReference({ nameRef, env });
@@ -44,6 +47,9 @@ class DynamicComponentReference {
4447

4548
if (typeof name === 'string') {
4649
let definition = env.getComponentDefinition([name]);
50+
51+
assert(`Could not find component named "${name}" (no component or template with that name was found)`, definition);
52+
4753
return definition;
4854
} else {
4955
return null;

packages/ember-glimmer/tests/integration/components/curly-components-test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -905,7 +905,7 @@ moduleFor('Components test: curly components', class extends RenderingTest {
905905

906906
this.assertComponentElement(this.firstChild, { content: 'true' });
907907
}
908-
['@htmlbars lookup of component takes priority over property']() {
908+
['@test lookup of component takes priority over property']() {
909909
this.registerComponent('some-component', {
910910
template: 'some-component'
911911
});

packages/ember-glimmer/tests/integration/helpers/custom-helper-test.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,13 @@ let assert = QUnit.assert;
66

77
moduleFor('Helpers test: custom helpers', class extends RenderingTest {
88

9+
['@glimmer it cannot override built-in syntax']() {
10+
this.registerHelper('if', () => 'Nope');
11+
expectAssertion(() => {
12+
this.render(`{{if foo 'LOL'}}`, { foo: true });
13+
}, /You attempted to overwrite the built-in helper \"if\" which is not allowed. Please rename the helper./);
14+
}
15+
916
['@test it can resolve custom simple helpers with or without dashes']() {
1017
this.registerHelper('hello', () => 'hello');
1118
this.registerHelper('hello-world', () => 'hello world');

packages/ember/tests/component_registration_test.js

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -144,18 +144,6 @@ QUnit.test('Late-registered components can be rendered with ONLY the template re
144144
ok(!helpers['borf-snorlax'], 'Component wasn\'t saved to global helpers hash');
145145
});
146146

147-
test('Component-like invocations are treated as bound paths if neither template nor component are registered on the container', function() {
148-
setTemplate('application', compile('<div id=\'wrapper\'>{{user-name}} hello {{api-key}} world</div>'));
149-
150-
boot(() => {
151-
appInstance.register('controller:application', Controller.extend({
152-
'user-name': 'machty'
153-
}));
154-
});
155-
156-
equal(jQuery('#wrapper').text(), 'machty hello world', 'The component is composed correctly');
157-
});
158-
159147
QUnit.test('Assigning layoutName to a component should setup the template as a layout', function() {
160148
expect(1);
161149

0 commit comments

Comments
 (0)