Skip to content

feat: setup @tanstack/solid-query for data fetching, use in upload queue#326

Merged
incognitojam merged 53 commits intocommaai:masterfrom
greatgitsby:feat/tanstack-query
Mar 31, 2025
Merged

feat: setup @tanstack/solid-query for data fetching, use in upload queue#326
incognitojam merged 53 commits intocommaai:masterfrom
greatgitsby:feat/tanstack-query

Conversation

@greatgitsby
Copy link
Copy Markdown
Contributor

@greatgitsby greatgitsby commented Mar 29, 2025

https://tkdodo.eu/blog/react-query-data-transformations
https://tkdodo.eu/blog/testing-react-query
https://tkdodo.eu/blog/the-query-options-api#query-factories

  • setup solid-query for app, dev tools integration (note: does not bundle in prod). see PR deployment
  • ported existing behavior to solid-query
    • online queue is active, refetching on interval. offline queue always polling as well
    • once the queue has loaded at least once, the queue holds its current state. when the drawer is opened later, it will present that last known state and fetch the latest (e.g. you were uploading 10 files with the queue open, closed it, and reopened it. once you reopen, it'll show the progress from the last known fetch and schedule it to be updated). UX should be smoother (this is the normal behavior of solid-query)
  • follow up PRs can begin to migrate usage to createQuery

test cases:

  • on load, "waiting for device" msg
  • device connected, nothing uploading, "nothing to upload" msg
  • device connected, upload file, files appear in queue
  • device disconnected, upload file, appears in offline queue; on connect, eventually1 messages appear
  • device connected, uploading, reboot device; files go away, device in offline state; once reconnected, switches back to uploading
  • device disconnected, nothing in offline queue, "device offline" msg
  • device disconnected, something in offline queue, shows offline messages
  • cancel item
  • cancel all
  • uploading files, switch to new device; show that device's upload queue state

Footnotes

  1. the transition between offline and online with offline messages present is undefined behavior (could say "device offline" then messages appear, or "nothing to upload" then appear). existing behavior, created issue: https://github.com/commaai/connect/issues/354

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 29, 2025

Changes:

path lines diff
./api/query-client.ts 9 +9
./App.tsx 45 +7
./components/UploadQueue.tsx 135 -17

Total lines: 4519 (-1)

@greatgitsby greatgitsby force-pushed the feat/tanstack-query branch 2 times, most recently from a0d3ba3 to 0161e26 Compare March 29, 2025 19:14
@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 29, 2025

deployed preview: https://326.connect-d5y.pages.dev

Welcome to connect! Make sure to:

  • read the contributing guidelines
  • mark your PR as a draft until it's ready to review
  • post the preview on Discord; feedback from users will speedup the PR review

Mobile

Desktop

@greatgitsby greatgitsby force-pushed the feat/tanstack-query branch 7 times, most recently from 9cb82a8 to 7cc0f18 Compare March 30, 2025 14:56
@greatgitsby greatgitsby marked this pull request as ready for review March 30, 2025 17:08
@greatgitsby greatgitsby force-pushed the feat/tanstack-query branch from c4aa292 to d79c83e Compare March 30, 2025 23:18
@incognitojam
Copy link
Copy Markdown
Collaborator

incognitojam commented Mar 30, 2025

The test seems fail when <SolidQueryDevtools /> is included..

Sorry maybe I lead you down the wrong path for this, and we should create a different wrapper for testing 🤔

https://tkdodo.eu/blog/testing-react-query

@incognitojam
Copy link
Copy Markdown
Collaborator

incognitojam commented Mar 30, 2025

You can put <QueryClientProvider> and <SolidQueryDevtools> in <App> instead, around/next to the <Router>, and we'll create a separate wrapper and query client in tests once it's needed

@greatgitsby greatgitsby marked this pull request as draft March 30, 2025 23:46
@greatgitsby
Copy link
Copy Markdown
Contributor Author

great article you attached! we need to revisit this sooner than later

@greatgitsby greatgitsby marked this pull request as ready for review March 30, 2025 23:50
@greatgitsby greatgitsby marked this pull request as draft March 30, 2025 23:52
@greatgitsby
Copy link
Copy Markdown
Contributor Author

took the creator's advice and extracted the query defaults to make the tests more flexible

@greatgitsby greatgitsby marked this pull request as ready for review March 31, 2025 00:02
@greatgitsby greatgitsby marked this pull request as draft March 31, 2025 00:21
@greatgitsby
Copy link
Copy Markdown
Contributor Author

i don't like the query-client.ts approach. we should instead migrate over the src/api/ files to hold the queries, selects, etc for each relevant call

marking as draft for now

@greatgitsby
Copy link
Copy Markdown
Contributor Author

moved to co-locating queries and keys together. as we migrate queries over, we can add onto the new objects.

i tested out pre-fetching the upload queue even if it's not visible -- takes 3 lines of code now 😊

@greatgitsby greatgitsby marked this pull request as ready for review March 31, 2025 05:13
@greatgitsby greatgitsby force-pushed the feat/tanstack-query branch from 8022ad4 to bf25d5a Compare March 31, 2025 20:35
@greatgitsby
Copy link
Copy Markdown
Contributor Author

do we want to recreate the mutation for every row? we only need to recreate it when the dongle changes

@incognitojam
Copy link
Copy Markdown
Collaborator

oh I see

@incognitojam incognitojam merged commit 2efca09 into commaai:master Mar 31, 2025
7 checks passed
<Routes />
</Router>
<QueryClientProvider client={queryClient}>
<SolidQueryDevtools />
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this run in production? I also see we include the solid devtools plugin

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

no. see first point in PR description

just checked again. i don't see the tanstack dev tools in prod
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement new feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants