Skip to content

Conversation

@greatgitsby
Copy link
Contributor

@greatgitsby greatgitsby commented Apr 6, 2025

here's another simple way to dedupe all route retrievals. when we've fetched a page, store the route in the tanstack query cache, and fetch the route in the RouteActivity using tanstack. this solves two major things:

  1. Suspense behaves exactly as before (routes.data will only trigger suspense if the data wasn't previously recorded). see demo video and test yourself
  2. handles navigating to a route directly smoothly. the data isn't in the cache yet (maybe RouteList isn't populated), but it still fetches w/o any special handling, and Suspense is triggered for route metadata like stats in RouteActivity

i think introduction of solid-query at this point is going to help solve the remaining performance requirements we have, like deduping requests and eventually introducing metadata caching like old connect. but before that, no unnecessary requests!

looking for feedback and opinions on the approach

demo.mov

@greatgitsby greatgitsby changed the title reafctor: retrieve routes using tanstack; store in the cache before load reafctor: retrieve routes using solid-query Apr 6, 2025
@github-actions
Copy link

github-actions bot commented Apr 6, 2025

Changes:

path lines diff
./App.browser.test.tsx 60 +20
./pages/dashboard/activities/RouteActivity.tsx 98 +7
./pages/dashboard/components/RouteList.tsx 162 +4
./api/query-client.ts 11 +2

Total lines: 4501 (+33)

@github-actions
Copy link

github-actions bot commented Apr 6, 2025

deployed preview: https://514.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 changed the title reafctor: retrieve routes using solid-query refactor: retrieve routes using solid-query Apr 6, 2025
@@ -1,13 +1,36 @@
import { VoidComponent } from 'solid-js'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this would be broken out into a separate PR

@sshane
Copy link
Contributor

sshane commented Apr 9, 2025

The timeline still flashes for a few frames when clicking routes. There shouldn't be any resource usage there as we already have the data

@greatgitsby
Copy link
Contributor Author

greatgitsby commented Apr 9, 2025

there is still resource usage in this file, events/statistics data requires extra requests even if we have the Route; that's why it flashes

these can and will be migrated to this same method, just not now. this PR was meant to migrate only the Route usage to minimize scope

by the time we load RouteActivity having loaded route data, there will be zero request required in RouteActivity.

@greatgitsby
Copy link
Contributor Author

@sshane try this version: commaai/connect#539

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