Regression, non-blocking model support and initial page of results not hiding infinityLoader

[Duder-onomy]

Hey,

Found an issue with the latest version 1.1.2
Seems that if using a non blocking model hook + hideOnInfinity and there is only one page of results, never seems to hide the infinity loader.

Here is a commit I made on my fork to demonstrate the issue, had to hack in some extra param support to the mirage serializer.

If you pull that commit in, you will see that the infinityLoader never drops.

Debugging this, it seems like the observer added here does not seem to observe through the promise proxy of the non-blocking model.

I thought about something like this:

didInsertElement() {
    this._super(...arguments);

    this._loadStatusDidChange();
    this.addObserver('infinityModelContent.reachedInfinity', this, this._loadStatusDidChange);
    this.addObserver('hideOnInfinity', this, this._loadStatusDidChange);

    // Wait for the infinity model THEN trigger the load status...
    get(this, 'infinityModelContent').then(() => this._loadStatusDidChange());

    let scrollableArea = get(this, 'scrollable');
    let infinityModel = get(this, 'infinityModelContent');
    if (infinityModel) {
      set(infinityModel, '_scrollable', scrollableArea);
    }
  },

That does not work ^.

Please let me know if there is anything I can do to help.

If you know the cause and can pseudo code it to me I don't mind adding failing tests and PR'ing. Just a little confused on what is happening under le hood.

[snewcomer]

@Duder-onomy 👋. Great catch! 👍 I believe I made a mistake. Here we are observing on didInsertElement. HOwever, at this point in the lifecycle, it is a promise. In the previous solution, it was able to observe the resolved value. So I think there is a slight difference in those two approaches. When we are checking state (reachedInfinity and such), we need to wait for the promise to resolve before asking the questions such as this

So we either revert to the old approach and try to solve hide on first load or we come up with a diff solution.

Does this make sense?

[Duder-onomy]

I am starting to understand what is going on .

These are the minimal changes i needed to make to get it working for me in my use case... but a bunch of other tests are failing: Duder-onomy@1252d38

I will spend some more time right now. Getting closer.

[Duder-onomy]

Aight, spent another few mins on this one today.
It seems to me that we should just always treat the infinityModel as a promise. This way any use case will be covered.

Change the will insert element hook:

  willInsertElement() {
    let ObjectPromiseProxy = ObjectProxy.extend(PromiseProxyMixin);
    let proxy = ObjectPromiseProxy.create({
      promise: resolve(get(this, 'infinityModel')),
    });

    set(this, 'infinityModelContent', proxy);
  },

Then all accessors use the then-able:

_loadStatusDidChange() {
    get(this, 'infinityModelContent')
      .then(() => {
        if (get(this, 'infinityModelContent.reachedInfinity') && get(this, 'hideOnInfinity')) {
          set(this, 'isVisible', false);
        }
      });
  },

This commit, Duder-onomy@76ac028?diff=split seems to fix everything.

I went through all the example routes in the dummy repo, and everything seems to work as expected.
The tests, however, are mostly failing.

[snewcomer]

@Duder-onomy That looks great! One outstanding question on my end - why can't we just wrap get(this, 'infinityModel') with resolve? I frequently use that trick like so. Or is it b/c infinityModel is an ArrayProxy?

function getTranslationURL(languageTag:string) {
  return `/locales/${languageTag}/translations.json`;
}

// I want languageTag to be either a value or a promise
function load(languageTag: string|Promise<string>) {
  // If languageTag is a value, then Promise.resolve creates a promise
  // that resolves to that very same value.
  // If languageTag is a promise, Promise.resolve would create a promise
  // that would behave like languageTag (it will either resolve or reject
  // the same way languageTag will do).
  return Promise.resolve(languageTag)
    .then((langTag) => getTranslationsURL(langTag))
    .then((url) => fetch(url));
}

[snewcomer]

@Duder-onomy Would you like to put up a PR for the work you did? I can help if you are strapped for time 🙌

[Duder-onomy]

Honestly, you can probably fix this allot faster than I can.
I was just trying to poke at it and see if it was easy.

I think I understand everything that is happening except the loadPrevious parts of the code.

But yeah, if you take a stab at it, shoot me a message and I will pull it into TheDyrt and see if it fixes the issue.

[snewcomer]

Ok perfect! Yeah I think your solution is it 👍. I'll try to do it this weekend