let element not always define value before rendering

[fragsalat]

I'm submitting a bug report

• Library Version: 1.5.0 (probably). I could reproduce it with the 1.3.0 script build.

Please tell us about your environment:

• Operating System: Ubuntu 18.04

• Node Version: all

• NPM Version: all

• *Script build** 1.3.0

• Browser: Chrome 70.x

• Language: all

Current behavior: Defining a computed value with the let element does not always happen on time. Sometimes it is defined after rendering and requires a re-rendering. I could reproduce this with a loop which has more html elements to render.

Checkout this pen and open your console to see the log. It should be that for each loop the computed value is defined and then rendered but it happens that it is not defined and rendered as undefined and later it is defined and re-rendered. https://codepen.io/fragsalat/pen/zyJbdj?editors=1010

Expected/desired behavior:

• What is the expected behavior? Computed values defined with let element should be defined before the following code executes.

• What is the motivation / use case for changing the behavior? Given the fact data is passed down to child elements and I expect a value to be there but get an undefined value, I would consider this as a bug.

[bigopon]

This is probably because of task queue limit size. Though I couldn't detect any abnormality in order of define / render. Doesn't seem to happen with vNext https://codesandbox.io/s/mj1r6v1z5p

cc @fkleuver

[fkleuver]

This is because of the connect queue. Only up to 100 connect calls are processed immediately, the remainder is queued to go with the next RAF call. There's 2 bindings per <let> in that example (the let binding itself and the interpolation) -> 100 / 2 = 50, and that's how you get the value for 50 of them immediately and delayed for the rest.

@bigopon Given the nature of LetBinding I don't think it makes sense to only enqueueBindingConnect(this) during bind, because you're not otherwise updating the value first. This applies to other bindings as well. I think it might be better to just updateTarget during bind and perhaps put some guard in place to not do it again if the connect queue is still within its budget of 100.

It doesn't happen in vNext because I refactored binding precisely because of these kinds of issues. The whole point was to address all these things :)

However I also think that, similarly to vNext updateTarget should always be invoked if things are not attached yet, regardless. Because that's one DOM update instead of two. But it's hard to accomplish in vCurrent unless you know of an easy way to get this information. I designed the lifecycles in vNext from the ground up with the purpose of having full control over all these things. It's not something we can just backport..

[fkleuver]

I have an idea though. We could export something like this from binding:

export function setConnectQueueThreshold(value: number): void {
  minimumImmediate = value;
}
export function disableConnectQueue(): void {
  minimumImmediate = Number.MAX_SAFE_INTEGER;
}

Then folks can adjust or even disable the connect queue if they have lots of binding and want things to be updated immediately.

What do you think @EisenbergEffect ?

[EisenbergEffect]

This seems like a reasonable solution to me, given the constraints of vCurrent.

[fragsalat]

Could this affect the performance of the render process? I would omit using the let binding before I slow down the rendering. If not it seem to be an easy workaround :)

[fkleuver]

Not really. If anything it might speed it up. The connect queue is there primarily to speed up initial page load, with the consequence that if it's too large then when the page is loaded not everything might be ready yet. This is generally favorable as for visitors of a website it's not noticeable that this happens (apart from the app loading faster).

To clarify, the connect-queue simply throttles the binding process. Properties are observed via Object.defineProperty that wraps properties in getters/setters which are then intercepted by observers that notify subscribers whenever they're set. This is a relatively expensive operation, but it's also what makes Aurelia much faster than SPA's that use dirty checking and differs. To counter the slightly longer initial load this can cause, any of these operations beyond the first 100 are queued to happen after the next requestAnimationFrame.

Disabling this, if you'd have 10.000 elements, you'd have to look at a white non-loaded page a bit longer as opposed to seeing immediate results. For 100-1000 elements it likely doesn't matter, that's all sub-second stuff.

[dannyBies]

Unrelated to this issue but I have tried the approach @fkleuver suggested. I am seeing a 50% performance increase on initial page load by disabling the connect queue which would contain about 100.000 elements. In my case this is faster and has the added benefit that the user does not see the DOM rendering that happens between every animation frame.

I am working on a PR to enable this option :)

[fkleuver]

@dannyBies Yeah we're aware of the trade-offs. It works well in a lot of cases but causes issues in some. This is one of the reasons why I took it out entirely in vNext and went with a different mechanism to optimize stuff with a bit more configurability. Users should have the freedom to choose which of these trade-offs they make.

benefit the user does not see the DOM rendering that happens between every animation frame.

Yep, for some this is a benefit and for others a drawback. No size fits all :)

I am working on a PR to enable this option :)

Excellent, thank you!

[fragsalat]

That sounds nice :D Actually for me it is great if the portion is only shown once everything is rendered as I show a spinner anyways.

Thansk for you work guys :)

[EisenbergEffect]

Closing since the binding library APIs are now released and we've got new techniques in vnext.