Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: loading state does not work with syncing empty collection #270

Merged
merged 1 commit into from
Jun 23, 2023

Conversation

luke-rogers
Copy link
Contributor

fixes #238

This ports the changes from this suggested patch of rxfire.

This fix only works when includeMetadataChanges is false. When true then collectionChanges is used directly and as such requires the suggested patch of rxfire to work around.

I've had a go at adding a test which seems to prove that there is no emission on an empty collection without this change, and as such the test fails with a timeout. Not sure if there is a neater way to avoid failing on a timeout and actually asserting on the store loading state.

@hakimio
Copy link
Contributor

hakimio commented Jun 7, 2023

Looks good to me.
@GrandSchtroumpf @randallmeeker what do you think?

@GrandSchtroumpf
Copy link
Collaborator

I haven't encounter this issue myself , but PR looks good 👍

@randallmeeker
Copy link
Collaborator

lgtm!

@randallmeeker
Copy link
Collaborator

I'm happy to merge unless someone gets to it before me. Probably will run through ng deps and update those and then do a version update, also unless someone gets to it before me)

@luke-rogers
Copy link
Contributor Author

Pleased you're happy with the changes.

Any chance we could get this released?

@luke-rogers
Copy link
Contributor Author

@randallmeeker Are you able to release this fix, please?

Currently blocking our upgrade to Firebase 9.

@randallmeeker
Copy link
Collaborator

Sorry I was with a client on site for the last 2 weeks and left everything go by the wayside. I'm back and I will get this done this evening and much appreciate the help

@randallmeeker
Copy link
Collaborator

sorry caught a bug was sick for a day or so. So lets try this again. Getting through my day jobs tasks this morning and then will do this.

@randallmeeker randallmeeker merged commit 46583b6 into dappsnation:v7 Jun 23, 2023
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Loading state does not work with syncing empty collection
4 participants