digidem / comapeo-mobile

The next version of Mapeo mobile
GNU General Public License v3.0
5 stars 1 forks source link

fix: refactor active project setup to fix potential issues related to using stale client #363

Closed achou11 closed 3 months ago

achou11 commented 3 months ago

Seeing the comment here highlighted issues around the active project setup where one could be working with a stale project client instance. These changes were initially applied to that branch to confirm that they fixed the issue noted there, which it did.

Instead of relying on useEffects and manually fetching new project instances, we now use React Query to handle the fetching. More importantly, this makes it easier to properly invalidate caches when needed, which I believe is the root of the issue that initiated this refactor.

Took some liberties to also clean up some low-hanging fruit in other existing query hooks. Apologies for the extra noise if distracting

achou11 commented 3 months ago

We could possibly use the background fetching indicator, to block the UI, but the context lives outside the navigator, so conditionally rendering a loader will unmount the navigator which is not ideal. There are other ways we can block the ui without unmounting but im not sure of the most elegant way of doing that.

Not sure what the preferred solution is, but we can probably use an absolutely positioned translucent loader that overlays the whole app (i.e. conditionally rendered on top of the app) and blocks pointer events, at least to start.

A more extreme idea is prompting to restart the app (whether manually or via something like https://github.com/avishayil/react-native-restart), but that's a pretty heavy-handed solution and probably not something to initially consider

achou11 commented 3 months ago

do you think i should try to tackle that in this PR or do it as a follow up?

ErikSin commented 3 months ago

do you think i should try to tackle that in this PR or do it as a follow up?

ill create a ticket to tackle in a follow up PR