Skip to content

Commit

Permalink
disallow @arguments on regular HTML nodes
Browse files Browse the repository at this point in the history
Adds an error message in the compile phase to inform the developer that
`@arguments` are not allowed on HTML nodes. This can often come from
copy/pasting refactoring errors.

For example, given the following template:

Before, the developer would get a vague error at runtime ([Example taken from
ember.js#18951](emberjs/ember.js#18951)):

```
jquery.js:3827 Uncaught TypeError: func is not a function
    at StatementCompilers.compile (VM32 ember.debug.js:43121)
    at compileStatements (VM32 ember.debug.js:44323)
    at maybeCompile (VM32 ember.debug.js:44312)
    at CompilableTemplateImpl.compile (VM32 ember.debug.js:44292)
    at Object.evaluate (VM32 ember.debug.js:51011)
    at AppendOpcodes.evaluate (VM32 ember.debug.js:49382)
    at LowLevelVM.evaluateSyscall (VM32 ember.debug.js:52284)
    at LowLevelVM.evaluateInner (VM32 ember.debug.js:52240)
    at LowLevelVM.evaluateOuter (VM32 ember.debug.js:52232)
    at JitVM.next (VM32 ember.debug.js:53175)
```

Now, they get an error message that looks like:

```
SyntaxError: ${attribute.name} is not a valid attribute name. @arguments
are only allowed on components, but the tag for this element
(\`${elementNode.tag}\`) is a regular, non-component HTML element. (location
line and column)
```

Given that there's no special behavior glimmer adds for `@arguments` on regular
HTML nodes, and it seems invalid to put `@` in attribute names in regular HTML
(validator.w3c.org gives ` Attribute @OnClick is not serializable as XML 1.0`
error message for the example template below), it seems helpful for the glimmer
compiler to fail at build time to prevent copy/paste errors and mistakes.

Fixes emberjs/ember.js#18951
  • Loading branch information
fivetanley committed May 18, 2020
1 parent 140af2a commit 3abccbf
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 3 deletions.
20 changes: 17 additions & 3 deletions packages/@glimmer/compiler/lib/template-compiler.ts
Expand Up @@ -134,11 +134,11 @@ export default class TemplateCompiler implements Processor<InputOps> {
typeAttr = attrs[i];
continue;
}
this.attribute([attrs[i]], !simple || actionIsComponent);
this.attribute([attrs[i]], !simple || actionIsComponent, action);
}

if (typeAttr) {
this.attribute([typeAttr], !simple || actionIsComponent);
this.attribute([typeAttr], !simple || actionIsComponent, action);
}

for (let i = 0; i < action.modifiers.length; i++) {
Expand All @@ -161,7 +161,8 @@ export default class TemplateCompiler implements Processor<InputOps> {
}
}

attribute([action]: [AST.AttrNode], isComponent: boolean) {
attribute([action]: [AST.AttrNode], isComponent: boolean, elementNode: AST.ElementNode) {
assertValidArgumentName(action, isComponent, elementNode);
let { name, value } = action;

let namespace = getAttrNamespace(name);
Expand Down Expand Up @@ -690,6 +691,19 @@ function assertIsSimplePath(path: AST.Expression, loc: AST.SourceLocation, conte
}
}

function assertValidArgumentName(
attribute: AST.AttrNode,
isComponent: boolean,
elementNode: AST.ElementNode
) {
if (!isComponent && attribute.name.startsWith('@')) {
throw new SyntaxError(
`${attribute.name} is not a valid attribute name. @arguments are only allowed on components, but the tag for this element (\`${elementNode.tag}\`) is a regular, non-component HTML element.`,
attribute.loc
);
}
}

function assertValidYield(statement: AST.MustacheStatement): string {
let { pairs } = statement.hash;

Expand Down
13 changes: 13 additions & 0 deletions packages/@glimmer/compiler/test/compiler-test.ts
Expand Up @@ -55,6 +55,19 @@ function test(desc: string, template: string, ...expectedStatements: BuilderStat
const Append = Builder.Append;
const Concat = Builder.Concat;

QUnit.test(
'@arguments are on regular non-component/regular HTML nodes throws syntax error',
function(assert) {
const template = strip`
<a @onClick={{action "hi"}}>Link</a>
`;
assert.throws(
() => compile(template),
/@onClick is not a valid attribute name. @arguments are only allowed on components, but the tag for this element \(`a`\) is a regular, non-component HTML element/
);
}
);

test('HTML text content', 'content', s`content`);

test('Text curlies', '<div>{{title}}<span>{{title}}</span></div>', [
Expand Down

0 comments on commit 3abccbf

Please sign in to comment.