Don't concat unquoted attribute values

[mmun]

An AttrNode is composed of a name and a value. The value can be either a string primitive or a MustacheNode. Here is a summary of the different cases today (the MustacheNodes are written as their equivalent mustache template syntax for brevity).

<div foo="bar">            -->     name: "foo", value: "bar"
<div foo="bar{{baz}}">     -->     name: "foo", value: {{concat 'bar' baz}}
<div foo="{{baz}}">        -->     name: "foo", value: {{concat baz}}
<div foo={{baz}}>          -->     name: "foo", value: {{concat baz}}

Note that 3 & 4 both call concat on baz. Instead we need 4 to be just

<div foo={{baz}}>          -->     name: "foo", value: {{baz}}

The work required is pretty minimal. Right now we use the same handler for unquoted and quoted attribute values. See https://github.com/tildeio/htmlbars/blob/master/packages/htmlbars-compiler/lib/html-parser/token-handlers.js#L9-L11. Just need to add the new behavior in the unquoted case.

[marcioj]

Hi @mmun, looks like this test already asserts this behavior. Am I missing something?

[mmun]

@marcioj Oh great :D I don't remember doing that.

[mmun]

There are still a couple issues. These three tests should throw a compiler warning (we can do an exception for now):

• https://github.com/tildeio/htmlbars/blob/master/packages/htmlbars-compiler/tests/combined_ast_node-test.js#L411-L414
• https://github.com/tildeio/htmlbars/blob/master/packages/htmlbars-compiler/tests/combined_ast_node-test.js#L421-L424
• https://github.com/tildeio/htmlbars/blob/master/packages/htmlbars-compiler/tests/combined_ast_node-test.js#L426-L429

Reasoning: Though <div class=a{{foo}}b></div> technically parses, but <div class="a{{foo}}b"></div> is preferred because it makes the string interpolation clear.

[mmun]

I believe <div foo="{{baz}}"> is parsed as name: "foo", value: {{baz}}, but it needs to go through concat, so name: "foo", value: {{concat baz}}. TLDR: quoted → concat, unqouted → direct.

[marcioj]

I was looking this issue but I am unsure if just changing the mustache handler in token-handler will be enough, because isn't possible to raise a warning in this scenario: <div foo={{baz}}a>. The a is wrong here, but since the mustache was already processed we have no handler to catch this problem. I think we could change the simple-html-tokenizer to emit a kind of StartTag.prototype.setAttributeValueType, which will set the values "quoted", "unquoted", "single-quoted", then in the custom finalizeAttributeValue, we get the current attributeValueType and raise if it's "unquoted" and there is a least one mustache. What do you think?

[mmun]

Might be easier to chat in realtime. Can you add me on google chat? im.mmun@gmail.com