aurelia / templating-binding

An implementation of the templating engine's Binding Language abstraction which uses a pluggable command syntax.
MIT License
32 stars 26 forks source link

Let expression parsing and auto-generated let element attributes. #132

Closed graycrow closed 5 years ago

graycrow commented 6 years ago

I'm submitting a bug(?) report

Current behavior:

I mentioned this in the previous issue (#129), but I think it's deserves a separate issue.

Lets assume I have a let element:

<let full-name="${firstName} ${lastName}"></let>

It's not rendered on the page, but internally it's gets passed to TemplatingBindingLanguage.createLetExpressions() in this form:

<let full-name="${firstName} ${lastName}" class="au-target" au-target-id="1"></let>

Please note that auto-generated attributes class and au-target-id. The problem is that createLetExpressions threats all attributes equally. And because they are not typical binding attributes it throws a warning, in this particular case 2 of them:

[templating-binding-language] Detected string literal in let bindings. Did you mean "class.bind=au-target" or "class=${au-target}" ?
[templating-binding-language] Detected string literal in let bindings. Did you mean "auTargetId.bind=1" or "auTargetId=${1}" ?

It may look harmless, but I have I feeling that it's not that harmless because now I'm investigating some issues in my project when introducing let elements into the code causing last items in the near repeat.for elements become undefined.

bigopon commented 6 years ago

This should be fixed by the latest templating commit https://github.com/aurelia/templating/commit/93fa9cc7ac3f51cff47fb9353110f26bdf20cea2

It's been released. Can you give it a try? @graycrow

graycrow commented 6 years ago

I afraid it's not released yet. Last release is v1.10.0 from Sep. 30 and your commit is from Oct. 12. But I glad that it's fixed. I will apply it manually and check.

bigopon commented 6 years ago

@graycrow I thought it was release, that's why I kept asking you to upgrade. Sorry for that. @EisenbergEffect I think we should release this asap to avoid unexpected behavior for folks along with the PRs from @graycrow in templating-binding

graycrow commented 6 years ago

@bigopon no rush, because after applying all commits from aurelia-templating I started to get Cannot read property 'behaviorInstructions' of undefined error.

graycrow commented 6 years ago

@bigopon auTargetID is undefined on line 323 in aurelia-templating/view-compiler.js , because you moved assignment to the line 329. This leads to instruction being added to instructions under undefined key instead of 1. And then at line 471 in view-factory.js instructable.getAttribute('au-target-id') is 1, but this key does not exists in instructions.

Edit: added line number

bigopon commented 6 years ago

@graycrow Oh thanks for pointing that out too. 🤦‍♂️ This bug wasn't easily caught in test, I oversaw this. Here is a fix for that https://github.com/aurelia/templating/pull/651

bigopon commented 5 years ago

@graycrow @EisenbergEffect This can be closed