aurelia / templating-resources

A standard set of behaviors, converters and other resources for use with the Aurelia templating library.
MIT License
59 stars 55 forks source link

binding behind if.binding still evaluated when if.binding removes the node #252

Closed 3cp closed 7 years ago

3cp commented 7 years ago

I'm submitting a bug report

Please tell us about your environment:

Current behavior: Other binding still evaluated when if binding is false, this cause error

Expected/desired behavior: https://gist.run/?id=55559f3bd606aa854502f3ddbbcad480 Toggle the SVG line, when removing line from DOM, error appears in Chrome console Error: <line> attribute x1: Expected length, "null".

I tried to move the if binding to parent. like this

<svg width="100" height="100" if.bind="line">
  <line
    x1.bind="line.sx"
    y1.bind="line.sy"
    x2.bind="line.ex"
    y2.bind="line.ey" stroke="black"></line>
</svg>

But it does not help.

Not sure whether this is related to Aurelia's SVG implementation. So far, I only saw this on SVG.

3cp commented 7 years ago

Question on the ordering of custom attributes.

In knockoutjs, when I define a bindingHandler (similar to aurelia's custom attributes), I can specify the execution order relative to other bindingHandlers. I was expecting similar thing in aurelia but I could not find clear clue in the source code.

How does aurelia makes sure repeat.for/with.bind/if.bind happen before other bindings on the same node? Are all magic controlled by bindingContext and overrideContext?

jdanyow commented 7 years ago

we're working on a fix for the issue you've reported- tracking this here:

Repeat, with and if are called template controllers. They bind before other bindings. You cannot use multiple template controller attributes on the same dom element.

3cp commented 7 years ago

Thx. Maybe the document should be clear about not using repeat/if/with on same element.

I actually used repeat and if together on many occasions. By luck, they work without any problem, repeat binding is performed before if binding at least for now. This is a good default behaviour to keep, otherwise I will break the bindings to 2 levels for future proof.

jdanyow commented 7 years ago

Make sure you test in multiple browsers. Some sort the attributes alphabetically, some leave them as-is.

3cp commented 7 years ago

Thx. I fix my code to avoid undetermined behaviour. When you got time, please update doc to reflect this.

atsu85 commented 7 years ago

@huochunpeng

Maybe the document should be clear about not using repeat/if/with on same element.

There is actually even much better solution: use aurelia-template-lint to detect this issue and many more at build time.