Skip to content

Conversation

cimigree
Copy link
Contributor

@cimigree cimigree commented Sep 2, 2025

towards #1345
There are some long wait times in the mobile app, in particular for the All Projects screen and the Exchange Screen.

  • Rather than right something hacky in mobile, it was suggested that we expose some prefetchQueries so that loading certain screens won't take as long.

  • So this PR adds one file with some prefetch helpers for the needed calls.

  • These are then exported so they can be used on focus in mobile.

  • I also did my best to make sure the api docs would work and would look similar to what already exists in here.

  • These helpers are not hooks and perform no subscription/side-effects beyond priming cache. It's all an addition so should not change anything (except the api docs) that already exists.

Questions:

  1. staleTime configurability
    Right now it’s hard-coded to Infinity to guarantee zero re-fetch during the menu focus window.
    Would you prefer we accept an optional staleTime argument (defaulting to Infinity) so mobile can send it in

  2. Tests
    Since nothing is returned I wasn't sure what/ how to test but I am open to suggestions!

  3. Placement / naming
    Kept these as small “lib/react-query” utilities and re-exported from the index. If you want them grouped differently, I can adjust. Just let me know.

@cimigree cimigree requested a review from achou11 September 2, 2025 22:09
@gmaclennan
Copy link
Member

I don't think this necessarily solves the issue, and at best just patches over the actual issue - all of these queries should be taking <5ms each so these big delays are pointing to a more serious issue somewhere. I think #1345 is actually caused by a different issue to some of our other slow IPC requests, and I've left a comment on that. Based on example Sentry traces from those slow AllProject screens, all requests are taking ~2s, so pre-fetching a few will not solve the issue.

PRs #97 and #95 should already partially address #1345 by significantly reducing the number of IPC requests, and digidem/comapeo-core#136 will reduce them further.

I think there are maintenance issues around expanding the API surface area of this module with things like this, and it ties the API more tightly to react query, making it much harder (e.g. more refactoring in apps) if we change it in the future to make things more performant. See #92 for more discussion.

Overall, I think this won't fix the issue, which has other causes, and it introduces a maintenance burden.

Copy link
Member

@achou11 achou11 left a comment

Choose a reason for hiding this comment

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

My initial question was going to be how much of a difference these preloading helpers make when it comes to the issues in mobile. At an initial glance, I would suspect that they don't make a significant difference. Think it'd be more effective to tackle other potential causes before relying on the changes here, which feel more like a hard-to-maintain workaround.

Tangentially, as one could guess from my venting in #92 , I appreciate the need for wanting to be able to use more of what react-query provides, but I think that shouldn't be exposed here in core-react (hence me posing the question about a separate module).

@cimigree
Copy link
Contributor Author

cimigree commented Sep 4, 2025

Thanks for the thoughtful feedback. I agree this doesn’t address the underlying latency and it does expand the public surface of core-react. Given the discussion in issue #92 and the ongoing work Gregor mentioned, I propose to close this PR and re-assess the All Projects / Exchange loads once those fixes are implemented, @ErikSin sound ok?

@achou11
Copy link
Member

achou11 commented Sep 8, 2025

Closing based on discussion here and elsewhere about reasons to not do this.

@achou11 achou11 closed this Sep 8, 2025
@achou11 achou11 deleted the feat/prefetch-queries branch September 8, 2025 14:03
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.

3 participants