Integrating React Query to improve performance #2822
Conversation
| queries: { | ||
| refetchOnWindowFocus: false, | ||
| retry: 1, | ||
| staleTime: 1000 * 60 * 5, |
There was a problem hiding this comment.
Sounds reasonable for here.
For pages like the user dashboard, as soon as we have a timestamp (i.e not on the first page of my listens) we can cache the results for a loooong time since the API results should be the same (barring deleted listens, the deletion process is run once an hour I think?)
We're disabling the tests and placing a placeholder till we have react-testing-library in place
frontend/js/src/user/Dashboard.tsx
Outdated
| setListens(initialListens || []); | ||
| }, [initialListens]); | ||
|
|
||
| const createWebsocketsConnection = React.useCallback((): void => { |
There was a problem hiding this comment.
I guess eventually we're going to want to extract the websocket connection management to a higher level and have it available via a react context.
However you know my refrain by now :) : let's keep it for a separate PR to avoid overcharging this one
There was a problem hiding this comment.
Correct, that the plan for the next PR, since socket is being used by multiple components
There was a problem hiding this comment.
I mentioned this in our video chat, I'm not so sure anymore that we want to do that 😅
Basically, we use websocket connections for two pages only at the moment: the user listens page and the playlist page,
In both cases, once the user navigates away to another tab or page, there's really no reason to keep using resources by keeping the websocket connection open.
I can see a future in which we get website notifications show in the UI, or a Playing Now conmponent visible across the website, or something like that, but it might be best to wait until we have that feature in mind.
We also talked with lucifer a little while back about possibly replacing websockets with server-sent events, so maybe we should also revisit that idea before refactoring WS on the frontend.
| websocketsUrl, | ||
| ]); | ||
|
|
||
| const updateFollowingList = ( |
There was a problem hiding this comment.
Note to self, refactor this into a FollowingList component that will take in a user prop, and take care of holding state for the list of followers as well as UI update code that we have here,
Replacing { data as MyTypes} force typing with the recommended useQuery<MyTypes>(...)
+ remove unused removeListenFromListenList function
This effect should be run exactly once on startup
There was a problem hiding this comment.
Let's goooooo !
I took the liberty to fix up the typescript types of the useQuery calls, as the data can be undefined we were potentially creating error scenarios where forcing the type hid the underlying issue.
Also updated the branch with latest single-page-app so that it's ready to merge (there was one conflict)
This PR adds
React-Queryto the loader to enable caching of the results.