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 states observables #253

Merged
merged 1 commit into from
Nov 18, 2021
Merged

fix loading states observables #253

merged 1 commit into from
Nov 18, 2021

Conversation

jondayton
Copy link
Contributor

We have a loadingStates property in the datastore, which is an observable Map object keyed by a query tag, and with values of sets of query information that is added and removed as fetches start / complete.

Previously, the Map and Set were initialized separately, with the Set initialized as a shallow observable so that its objects would not be observable, and thus would not be proxied objects and could be compared directly.

Map() // observable
  => Set() // observable
    => Object() // not observable

However, because mobx propagates observability down, this seems to have broken after a recent change in a mobx Typescript interceptor

This PR changes the data structure to initialize observability once, and change the query reference objects to JSON strings so that they will not be proxied themselves.

Map() // observable
  => Set() // observable (by extension)
    => JSON String() // not observable

src/Store.js Outdated
@@ -499,11 +498,12 @@ class Store {
*/
@action
deleteLoadingState = (state) => {
const querySet = this.loadingStates.get(state.queryTag)
const { loadingStates } = this
loadingStates.get(state.queryTag)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like this redundant line got left in

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

@jondayton jondayton merged commit 4e74ab7 into main Nov 18, 2021
@jondayton jondayton deleted the fix-mobx-upgrade branch November 18, 2021 14:57
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.

2 participants