feat(useProject): Create a hook to fetch individual projects and move away from the ProjectStore #68038
Conversation
… away from the ProjectStore
github-actions
bot
added
the
Scope: Frontend
| }, [id, slug, byIdCache, bySlugCache]); | ||
|
|
||
| const lookupId = id ?? slug; | ||
| return lookupId |
There was a problem hiding this comment.
It looks like this just returns the project data, but shouldn't it also return isLoading and isError?
| if (bySlugCache.data?.[slug]) { | ||
| bySlugCache.buffer([`slug:${slug}`]); | ||
| } else { | ||
| bySlugCache.buffer([`slug:${slug}`]); |
There was a problem hiding this comment.
Looks like this is the same as the line above it
There was a problem hiding this comment.
D'oh. I ripped out some stuff from that other PR, kept the wrong stuff!
| responseReducer: useMemo(() => makeResponseReducer('id'), []), | ||
| }); | ||
|
|
||
| const bySlugCache = useAggregatedQueryKeys<AggQueryKey, ProjectStore>({ |
There was a problem hiding this comment.
I don't have a strong opinion on this, but I wonder if we should split this into two hooks useProjectById and useProjectBySlug to avoid having to deal with two separate caches
There was a problem hiding this comment.
if we do end up splitting this into two hooks i'd vouch for splitting the useTeamsById hook into useTeamsById + useTeamsBySlug for consistency
|
This pull request has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you add the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
This first appeared in #66511. Trouble with that PR is the
useAllProjectsOptionsidea in there might not be great, so I wanted to skip that business for now, land what we can and come back to it after.In order to move away from the pre-populated
ProjectStoreanduseProjects()we need something else to move into. Why? We want to get rid of the pre-populated ProjectStore because it slows down every initial pageload by fetching every single project within an org. Not all orgs are needed on each page.useProjects()is the hook we us most often to interact with that store, but the trouble withuseProjects()is that while it does returnfetching: boolean, no callsites actually check the value. So if we were were to stop pre-populating the store, pages all over could look weird or throw because projects are not fetched.So we need to update each callsite to check that
fetchingvalue before reading the project value. If we're going to touch each callsite then we might as well change out the backend so it's not ProjectStore anymore, this is whereuseProject()(singular now, not plural) comes in. By using a different method name we create an easy way to verify that a given callsite is checkingisFetchingor not, calls to the new method must do the check!Related to #65949