Double load of the first page of a PageableList

[harbalk]

The modification of CustomElement attachedCallback function in this commit ibm-js/delite@b3638c5 lead to a problem of double load of the first page of a list.
It is due to a double pass in the if of "initial loading" in the computeProperties function of PageableList.
There is, first, a modification of the store attribute and then a modification of the _collection attribute.

computeProperties: function (props) {
        if (this.pageLength > 0 && this.attached) {
          if ("store" in props || "query" in props || "_collection" in props)  {
                    // Initial loading of the list
                    ...

In the previous version, the store was in the props list before the attached attribute was set at true. Also the only time we passed in the "initial loading if" was when the _collection was modified.

But in this version the store is in the props list at the same as this.attached attribute. There is then one pass because the store has been modified and another because of the _collection, and the page is loaded twice.

There are two solutions of this problem :

• The first is to set the attached attribute of the CustomElement in the attachedCallback.after(), as it was done in the previous version.
• The second one is to remove the modification of the store attribute of the list of the conditions of initial loading (remove "store" in props condition). At first seen, I think there is kind of redundancy with _collection modification.

I don't think I have enough knowledge of the code to be able to properly grasp all the impact of this modifications.
Tell me what you think about it.

[cjolif]

cc: @wkeese it is strange that attached is now set before attachment?

[wkeese]

The first is to set the attached attribute of the CustomElement in the attachedCallback.after(), as it was done in the previous version.

I think the reason attached is currently set early is so whenhas("inherited-dir") is true, Bidi#getInheritedDir() will call getComputedStyle() upon page attach.

But I think setting attached later makes sense, especially given the API documentation:

/**
 * Set to true when `attachedCallback()` has completed, and false when `detachedCallback()` called.
 * @member {boolean}
 * @protected
 */
attached: false,

So I'll make that change.

The second one is to remove the modification of the store attribute of the list of the conditions of initial loading (remove "store" in props condition). At first seen, I think there is kind of redundancy with _collection modification.

Perhaps that also makes sense, but I'll do the first change for now.

[wkeese]

PS: Thanks for tracking this down.