Problem with if.bind + repeat.for + @children selector

[maxgrv]

I'm submitting a bug report

• Library Version:
  1.1.5

Please tell us about your environment:

• Operating System:
  Windows 10

• Node Version:
  8.4.0

• NPM Version:
  5.30

• JSPM OR Webpack AND Version
  webpack 3.5.5

• Browser:
  All

• Language:
  TypeScript 2

Current behavior:

When we have a situation like this:

    <test if.bind="visible">
        <test-child repeat.for="point of points"></test-child>
    </test>

and the test custom element has a property with the @children("test-child") decorator.

When we toggle the status of the visible variable (so the repeat.for is detached and attached again) we get the children array update notification but the children array has the wrong number of elements (it grows without limits each toggle, it seems that the new elements are added twice)

See this gist with console open:

https://gist.run/?id=923ce4224412a695890e8d134955c20c

Click on the toggle button. Notice how the array count increases by 3 to infinity without reason, even if the actual custom elements remain 3.

Expected/desired behavior:

The array length should be equal to the children element count.

[Infuser]

I have noticed all sorts of weird issues with if.bind, I know it does not solve your issue but a workaround I found was to delay rendering using if.bind but then use show.bind for visibility

[bigopon]

The issue comes from <slot/> and repeat, not the if. You can also workaround by wrapping the repeat element inside another element.

Duplicated of #541

[maxgrv]

Adding the wrapper element breaks the @children decorator as it sees only direct, first-level descendants... so no workaround for our use case :(

[bigopon]

@maxgrv The fix is at #592 , you can apply fix in viewslot-fix.js in the gist to resolve it.

[maxgrv]

I have created an updated gist with the fix applied but it does not seem to resolve the issure... please note that we are not using ShadowDOM.

[bigopon]

Thanks, I didn't check it carefully and assumed that slot was root of the issue. The issue was that value was not properly unset in if scenario, so here is gist with fix: https://gist.run/?id=9323e42332f7b2c581212df8609e5c8d

PR for the fix is here #593 but it's holidays season ... so probably you gotta patch it temporarily for awhile 😄 unless @EisenbergEffect / @jdanyow can review & merge it together with #592

[maxgrv]

Thanks @bigopon , the latest fix seems to solve part of the problem. Now the repeat.for child elements are updated correctly in the array. But if I add another element outside of the repeat.for loop it disappears after toggling the first if

<test if.bind="visible">
        <test-child></test-child>
        <test-child repeat.for="point of points"></test-child>
</test>

I have created an updated gist with the fix which shows the issue. Notice that console.log shows 4 elements in the array the first time and 3 the following. I do not know whether this behavior is related to this bug or not but it does not sound right :)

[bigopon]

@maxgrv The issues is because observer bind will ignore existing children if you are not using native shadowDOM (https://github.com/aurelia/templating/blob/master/src/child-observation.js#L173)
(decorating your test class with useShadowDOM, and change its element name to test-parent will work fine)

I'm not sure why we cannot have it for light shadow DOM, and commenting that line out works fine for me. So I'll cc @EisenbergEffect @jdanyow for more insight here

[maxgrv]

Any update on this? The @children selector is quite broken now...