Infinite loop with ember-data query and ember-concurrency

[anehx]

Hi

First of all, thanks for this awesome addon - I'm using it a lot. Sadly, I've run into a bug with a very common use case: I have a task that fetches some data via ember-data .query method. This task is triggered by useTask. However, this results in an infinite loop of fetching. Somehow the query triggers a tracking update which then causes the task to be run again and so on.

I prepared an app that reproduces the problem: https://github.com/anehx/ember-resources-bug

[NullVoxPopuli]

what happens if you add yield Promise.resolve() to the top of your task?

[anehx]

@NullVoxPopuli Hmm, that fixes the issue completely, why though?

[NullVoxPopuli]

There is some discussion here: emberjs/rfcs#769

but regarding ember-resources, what this allows is that before any yield or await, you can use @tracked data, and then your task will automatically re-run if that tracked data changes.

personally, I think we need a lint rule for this kinda stuff -- but I haven't had time to PR to eslint-plugin-ember.

maybe @bmish has cycles ;)

• PR new lint rule to require await or yield in tasks if tracked data is consumed.
• unrelated to ember-resources (but resources to exacerbate the lack of learning our guides currently have on this problem 🙈 )

[bmish]

@NullVoxPopuli can you file a ticket with that lint rule idea?

[NullVoxPopuli]

sure thing!

[NullVoxPopuli]

ember-cli/eslint-plugin-ember#1413

[NullVoxPopuli]

Thanks for reporting @anehx !

We'll hopefully / eventually be able to come up with something to help guide developers towards not running in to this problem in the future <3

[anehx]

Now it makes sense. I think a linter rule is a good compromise for now to help people with that issue since that behaviour is not really well known. Thanks for looking into it @NullVoxPopuli !

[vstefanovic97]

@NullVoxPopuli first of great work with this addon I really like it!
I wanted to ask if you can explain why Promise.resolve on top of the task fixes the infinite revalidation issue or point me to some docs that can help me understand. I tried reading rfc#769 but still didn't understand why it fixes the issue, and is it safe to use this, is there a change this can break with future ember versions?

[NullVoxPopuli]

I really like it!

eee <3 thanks!

why Promise.resolve on top of the task fixes the infinite revalidation issue or point

To talk about why it can be needed, it's helpful to talk about what you can do without it.
For example, say you have a class defined as the following:

class Demo {
  @tracked firstName;
  @tracked appName
  
  getName = useTask(this, this._getName);
  
  @task
  *_getName() {
     let { firstName } = this;
     
     return yield getFullName(firstName);
  }
}

Hello, {{this.getName.value}}

so, what happens then firstName changes?
You'd expect that the greeting in your template would update, yeah?
without the yield Promise.resolve(), this is possible.

So, what happens if we set data after the yield?

  @task
  *_getName() {
     let { firstName } = this;
     
     let result = yield getFullName(firstName);
     
     this.firstName = result.split(' ')[0];
     
     return result;
  }

Things "just work as expected" because _auto tracking is synchronous.
For more information on details of how auto-tracking works, this may seem useful: https://www.pzuraq.com/how-autotracking-works/

the tldr: is that, synchrnously, auto-tracking works like this:

• for every reference in a template, open a tracking frame
• while executing code, push encountered references into that tracking frame
• when execution completes, close the tracking frame.

And because new now know about those other references, we know that when one of those references changes, the reference in template should be called again. (in this case this.getName.value is called again).

the tldr as code (super tldr):

openFrame();
// ... ✂️ (no await)
this.getName.value;
// record every encountered tracked thing 
closeFrame();

So, then what happens when we assign firstName before the await/yield?

 @task
 *_getName() {
    this.firstName = this.firstName.split().reverse().join('');     
    return yield getFullName(this.firstName);
 }

When that assignment happens, we're instructing the renderer / VM to queue an update.
but, we're already trying to figure out what our this.getName.value depends on.

We cannot set tracked data while there is an open tracking frame.

Or said in a framework-agnostic way (this problem exists in every framework)

We cannot set data while we are rendering it, because it causes infinite rerenders.

So how does await/yield Promise.resolve() help?

 @task
 *_getName() {
    await Promise.resolve();
    this.firstName = this.firstName.split().reverse().join('');     
    return yield getFullName(this.firstName);
 }

Looking back out our super tldr auto-tracking implementation:

openFrame();
// ... ✂️ (no await)
this.getName.value;
// record every encountered tracked thing 
closeFrame();

anything asynchronous that occurs during this.getName.value will execute after the frame is closed.

It's this classic example of async timing:

async function doWork() { 
  await Promise.resolve();
  console.log('a') 
}

console.log('b');
doWork();
console.log('c');

spoiler

output is:

b
c
a

so any interaction with tracked data here in this last example evades the infinite invalidation assertion.

So does does this come in to play with the problem from the original issue report?

(thanks again for the repro-repo, @anehx )

  @task
  *fetchUsers() {
    return yield this.store.query('user', {});
  }

It's a bit dubious, because all the details are hidden.

• no user-land tracked data is consumed
• no user-land tracked data is set
• everything happens internally to this.store.query

There must be something going inside query (or synchronously caused by query) that is tracked, and causing the renderer, to be like "ope, something changed, time to re-render"
(I haven't dug into the specifics of this, because ember-data + mirage is a lot of code, and 10s of debugging didn't reveal anything obvious -- We really need some good tracked debugging tools to debug stuff like this tho -- I think @wycats had something prototyped a while back)

is there a change this can break with future ember versions?

which thing are you referencing?

the await Promise.resolve() trick will probably work forever.
useTask is stable.
nothing in ember-resources is using private APIs.

[vstefanovic97]

@NullVoxPopuli that explanation makes total sense, I had previously read https://www.pzuraq.com/how-autotracking-works/ but guess I didn't fully understand it before.
Thanks to your explanation I think I finally get how it works or at least have a better understanding than before, thank you so much for taking the time to write such a detailed explanation!

[cah-brian-gantzler]

I was using the additional await Promise.resolve() which was working fine until ember data 4.8. Now the infinite recursion is back See emberjs/data#8312