Fix - Bested async virtual fields

[yuriyyakym]

Description

Related issue: #5505

Because of how async traversal is done, nested virtual async fields push their promises too late, which results in these promises being ignored.

• I have read and understand the CONTRIBUTING.md document in this repository.

Type of change

• Bug fix (non-breaking change which fixes an issue)

Checklist:

• I have added tests that prove my fix is effective or that my feature works
• Existing test suite passes locally with my changes

[jmikrut]

Hey @yuriyyakym — thank you for the find and the PR!

Here are my thoughts. There really should be a few things done here to fix this fully.

Instead of pushing promises from nested fields to the root level array, parent promise could resolve only after its children's promises are resolved.

100%. Our beforeValidate and beforeChange hooks do exactly what you describe (a parent waits for its children promises to resolve) but up to now, afterRead uses all top-level promises as you've found. This is for performance, but clearly it's causing issues.

We could convert afterRead to function more similarly to beforeValidate / beforeChange, but that will take some work and we likely won't get to that for a bit yet. It will also come with a performance penalty, so that's not 100% ideal.

But luckily I think there is another simple solution here. Really, the problem stems from the fact that field promises and population promises are executed in parallel. The promises both run at similar times, but it's a race condition to see which promise (fieldPromise or populationPromise) finishes first. In your example, your populationPromise is finishing first before the fieldPromise, which is bad.

So we could fix that by instead of adding a promise directly to populationPromises, we instead push an anonymous function to the populationPromises array, and then once field promises are all resolved, then we loop the populationPromises, calling each anonymous function. This would actually start the population promises, and we could guarantee that all population promises are started and will finish after the field promises finish.

You could take a crack at updating this PR to reflect what I've said above, and I think that will fix the issue. We will happily merge.

What do you think? We can get back to this shortly in any case if you can't investigate the solution I've suggested above.

Also, I can't reproduce this on 3.0 for some reason. We copied your test and your config to 3.0 (alpha branch) and it did not fail. Interesting..