React-redux-firebase: bug(populate): isLoaded returns true when populated children are still loading

[marekolszewski]

Do you want to request a feature or report a bug?
Bug

What is the current behavior?
For populated queries, isLoaded returns true when the base query completes, even if the populated child queries are still outstanding.

What is the expected behavior?
Ideally it would return false while the populated children are being loaded. For backwards compatibility, it may make sense for isLoaded to take an extra parameter such as 'waitForPopulate'.

[prescottprue]

@marekolszewski This is indeed expected behavior, and if you are seeing it, it is for sure a bug.

Which version are you using?

Can you possibly provide a repo where the bug can be reproduced? Feel free to message me directly on gitter instead if the repo is private.

[prescottprue]

@marekolszewski I was able to replicate. It seems that due to the order of dispatches in the query action, the other actions are dispatched after the main data is update (even though all of the data has been retrieved from Firebase already).

Going to experiment with switching the order of these and see if that helps. The goal being that the main data (what will be used as the path within populatedDataToJS) is updated last, meaning that all of the other data needed for population will already be in redux. That should hopefully keep isLoaded from firing until populated data is available.

Will keep this issue updated with progress.

[marekolszewski]

@prescottprue Sounds good!

For anyone else encountering this issue, a simple workaround is to just do a typeof on the field being populated and waiting until it becomes an object.

[marekolszewski]

@prescottprue BTW, I also just noticed that adding type: 'once' to my queries is also breaking population for me. I wonder if it's related to this.

[prescottprue]

@marekolszewski I don't believe it is related. once does not currently support populate, but this should be added to the roadmap for sure.

I agree with type checking. It is how my team and I decided to work around it for now.

[prescottprue]

I believe I may have a fix for this, and it should make it into v1.4.0-rc.2. Testing it a bit more, but will hopefully be ready for release tomorrow. This ticket will continued to be updated until it is released into a v1.4.0 (non pre-release version).

As mentioned in the roadmap, there are future plans for making this configurable.

[prescottprue]

v1.4.0-rc.2 includes the fix for this. I tested plenty, but if anyone else get a chance, post your results.

[prescottprue]

v1.4.0 was released with this functionality included, so this is now out of pre-release.

[marekolszewski]

@prescottprue Awesome, thanks for fixing this!