oneTime binding inside if inside repeat does not work as expected

[rluba]

I'm submitting a bug report

• Library Version:
  1.7.1

Please tell us about your environment:

• Operating System:
  OSX 10.x

• Node Version:
  8.9.1

• NPM Version:
  61.0

• JSPM OR Webpack AND Version
  [email protected]

• Browser:
  all

• Language:
  all

Current behavior:
Similar to aurelia/framework#301: repeat.for and oneTime bindings don’t work properly together.

The specific problem in aurelia/framework#301 is fixed, but as soon as you insert a if.bind between the repeat.for and the One-Time-Binding, the problem reappears:

welcome.html

<template>
    <button click.trigger="clickme()">Click me!</button>
    <div repeat.for="item of items">
	    <div if.bind="item">
	    	${item.prop1 & oneTime}
	    </div>
	</div>
</template>

welcome.js

export class Welcome {
    items = [{prop1:1},{prop1:2},{prop1:3}];

    clickme() {
        this.items = [{prop1:4},{prop1:5}];
    }
}

After clicking the button, the content reads

1
2

i.e. the number of items is updated, but their content is not.

Expected/desired behavior:
After clicking the button, the content should be

4
5

[rluba]

Contrary to the original issue (aurelia/framework#301) the bug does not depend on the number of bindings in the element and does not go away when using textcontent.one-time instead of the interpolation

[rluba]

After having a quick look at how repeat works, it is clear why this fails.

repeat simply tries to update all one-time bindings it knows about, but it fails to find one-time bindings within any child views, eg., when using custom elements or @templateController attributes like if.

The only reliable solution I see is disabling repeats view-reuse. It should only reuse views if the view’s bound element actually remains the same. (Similar to how ngRepeat in AngularJs works. There you can optionally add a track by expression to aid with reusing views, but the default is to throw away any views whose elements have disappeared.)

Edit: I’ve just discovered that there’s already a mechanism for this, but if is explicitly excluded from it.

It seems to me that the behaviors or viewFactories need to contain some information about whether they contain one-time bindings to trigger the full view lifecycle in that case.

[rluba]

After trying for a few hours, it seems to me like there’s no way to

• either reliably find all one-time bindings that might need updating from within repeat.updateBinding,
• or detect if a viewFactory might contain a one-time binding and flag it with _viewsRequireLifecycle (because & one-time looks like a regular binding until it’s instantiated much later).

I see two options:

1. try to monkey-patch if and custom elements with updateOneTimeBindings functions.
2. completely disable the repeats view-reuse logic, which would slow down Aurelia.

@EisenbergEffect Any better ideas?

[EisenbergEffect]

@rluba Is the reason that it's difficult to find because some oneTime bindings are the result of a binding behavior rather than an attribute binding command?

@jdanyow Worked most on the this area of code, so he might have some ideas on what we can do. If it's a result of what I mention above, maybe there's some way we can reliably and consistently tag things.

[rluba]

@EisenbergEffect Yes, that’s the reason. Currently, the binding behaviors can only be distinguished and enumerated after they have been instantiated. I’d be interested to hear @jdanyow’s suggestions because my solution seems to work, but is far from idea.

[EisenbergEffect]

@jdanyow Can you take a look at the PR linked above and let us know what you think?

[bigopon]

The fix proposed looks good enough for me for repeat, if. Is there any reasons why you didn't go for with, replaceable @rluba ?

[rluba]

Only because I haven’t used them yet. @bigopon Can you give me an example for both? Then I’ll update my solution to fix them, if necessary.

[bigopon]

@rluba with in aurelia template works like the way with works in JS. replaceable works similar to slot, with 2 additional notes: it's single level and works with dynamic template (read template controllers i.e if, repeat )

<template>
  <!-- instead of -->
  <my-big-form>
    <input value.bind="data.address.number">
    <input value.bind="data.address.street">
  </my-big-form>


  <!-- do -->
  <my-big-form with.bind="data.address">
    <input value.bind="number">
    <input value.bind="street">
  </my-big-form>
</template>

[rluba]

I finally gave it a try: I can’t find any unexpected behavior when using with or replaceable with oneTime bindings – at least not when using my fix for repeat.

[EisenbergEffect]

Thanks for testing that out @rluba and thanks for your patience in this! @bigopon Can you make a final review of the fix that @rluba is proposing and make a recommendation? If you don't see any other issues then we can get this merged and probably released in the next few days. Let me know what you think.