Using RR loaders all over for clean page transitions #1101

david-crespo · 2022-07-29T22:41:33Z

This started out being about making error handling easier, but now that I figured out how to call prefetchQuery in a loader, I'm going around adding loaders to most pages so there is no visible pop-in when the API call is not instantaneous.

requireParams, a version of useRequiredParams that works in loaders, where we get params as an arg instead of calling useParams
- useRequiredParams now has a one-line definition because all the logic is in requireParams
Move queryClient into @oxide/api so we can
- Define a typed queryClient.fetchQuery with an API like useApiQuery
- Access it outside the component tree in loaders by importing it directly
Use it to fetch the VPC for the VPC detail page

What loaders do for page transitions

Both of these are with a 1s sleep in the VPC endpoint handler to simulate a slow response.

Before: render, then fetch

After: fetch first

React Router holds off on the route transition until the loader is done. Much more like a traditional web app despite still being all client-side.

vercel · 2022-07-29T22:41:36Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated
console-ui-storybook	✅ Ready (Inspect)	Visit Preview	Aug 4, 2022 at 4:57AM (UTC)

david-crespo · 2022-07-29T22:52:55Z

app/pages/project/networking/VpcPage/VpcPage.tsx

-  const { data: vpc } = useApiQuery('vpcView', vpcParams)
+  // could do `as Vpc`, but this keeps us more honest until they get Remix's
+  // loader type inference into the router
+  const vpc = useLoaderData() as Awaited<ReturnType<typeof loader>>


Interesting that this ends up being a one-line change in the component but the behavior is quite different:

This can never fail to be present because this component will only render if the loader is successful

Hence deleting the ? below

The fetch has to be done before the page will render, so we don't see the layout first and then the page contents

david-crespo · 2022-07-29T22:53:47Z

app/hooks/use-params.ts

+export const requireInstanceParams = requireParams('orgName', 'projectName', 'instanceName')
+export const requireVpcParams = requireParams('orgName', 'projectName', 'vpcName')
+export const requireProjectParams = requireParams('orgName', 'projectName')
+


could live without the helpers but they're kind of nice?

Yeah, I think they're good. Shortens the params and makes them a bit explicit, so I'm good with it.

app/hooks/use-params.ts

zephraph · 2022-08-01T19:51:54Z

app/routes.tsx


 export const Router = () => (
-  <DataBrowserRouter fallbackElement={<span>loading</span>}>
+  <DataBrowserRouter fallbackElement={null}>


I assume the null is what makes it wait before transitioning? Would the loading indicator be provided here somewhere?

No, the null is just telling it to show a blank screen while loading instead of flashing the word "loading". This is where you would drop a full-page loading state. I think a more integrated loading state like the little 1-2px bar across the top would have to be built right into the layout component using useNavigation().state to tell when you're in a loading state.

loaders 4 all! you get a loader! you get a loader!

david-crespo · 2022-08-04T04:55:36Z

app/pages/OrgAccessPage.tsx

 const colHelper = createColumnHelper<UserRow>()

-export const OrgAccessPage = () => {
+export function OrgAccessPage() {


changed all of these to use function because of hoisting — it lets me define OrgAccessPage.loader above rather than below

david-crespo · 2022-08-04T04:56:42Z

app/pages/OrgAccessPage.tsx

+    // used in useUserAccessRows to resolve user names
+    apiQueryClient.prefetchQuery('userList', { limit: 200 }),
+  ])
+}


this Promise.all is nifty. so easy. I heard you like parallel loading so I put parallel loading in your parallel loading

david-crespo · 2022-08-04T04:57:40Z

libs/api-mocks/msw/handlers.ts

 const alreadyExistsBody = { error_code: 'ObjectAlreadyExists' } as const
 type AlreadyExists = typeof alreadyExistsBody
-const alreadyExistsErr = json(alreadyExistsBody, 400)
+const alreadyExistsErr = json(alreadyExistsBody, { status: 400 })


Sorry for the noise in this file, you can basically ignore it. I was using the delay option here a lot for testing so I made it easier to use

david-crespo · 2022-08-04T04:58:29Z

libs/api-mocks/msw/util.ts

+): ResponseTransformer<B> {
+  const { status = 200, delay = 0 } = options
+  return compose(context.status(status), context.json(body), context.delay(delay))
+}


use an object instead of positional args for the options so you don't have to pass in a dummy status in order to pass a delay

david-crespo · 2022-08-04T04:59:50Z

libs/api/hooks.ts

      options
    )

+export const wrapQueryClient = <A extends ApiClient>(api: A, queryClient: QueryClient) => ({


fetch, prefetch, and refetch needed both api and queryClient here

david-crespo · 2022-08-04T05:01:35Z

libs/api/hooks.ts

-  }
+  <A extends ApiClient>(api: A) =>
+  () =>
+    wrapQueryClient(api, useQueryClient())


tbh we don't even really need this anymore since we can just import the right queryClient directly. this one just makes it so you are pulling it off of context from the QueryClientProvider. Not sure what the point of that is. Maybe if you had different query clients for different sections of the route tree and you wanted the component to transparently pull the right one, but my god why would you do that?

david-crespo · 2022-08-04T05:02:12Z

libs/api/index.ts

+
+// to be used in loaders, which are outside the component tree and therefore
+// don't have access to context
+export const apiQueryClient = wrapQueryClient(api.methods, queryClient)


this is important, we're using this for all the prefetch calls

david-crespo · 2022-08-04T05:02:51Z

patches/react-router+6.4.0-pre.11.patch

+  }, []);
+   return null;
+ }
+


pulling the fix from this PR. I will remove the patch as soon as they release this (should be this week)
remix-run/react-router#9124

david-crespo · 2022-08-04T05:05:56Z

app/pages/project/networking/VpcsPage.tsx

+    limit: 10,
+  })
+}
+


These are all extremely straightforward. The big problem I see is making sure you are actually making the right request with the right params. I left the wrong method in a few times and it wasn't always easy to notice. Params are even worse — the limit: 10 I stuck everywhere is to match QueryTable but it's way too easy to mess up. We can think about it.

david-crespo · 2022-08-04T05:13:32Z

I started writing a doc for the docs dir describing the data fetching architecture overall, but if I'm gonna write it out (and I already wrote like 3/4 of it) then it might as well be an RFD? Console internals are underrepresented in the RFDs.

https://github.com/oxidecomputer/console/blob/data-arch-doc/docs/data-fetching.md

zephraph · 2022-08-04T17:47:02Z

@david-crespo on the RFD note... I'm uncertain. Does it require a discussion? It feels like internal documentation might be the better way to tackle it. If we were outlining general principles or something that could always be an RFD, but this seems like it'd be more cost than gain.

david-crespo · 2022-08-04T17:55:34Z

It does not require discussion. L'état, c'est moi.

vercel bot deployed to Preview July 29, 2022 22:41 View deployment

david-crespo force-pushed the try-loaders branch from 868d175 to 0bc898f Compare July 29, 2022 22:51

david-crespo commented Jul 29, 2022

View reviewed changes

use a react-router loader to fetch the VPC for VpcPage

f21c7a9

david-crespo force-pushed the try-loaders branch from 0bc898f to f21c7a9 Compare July 29, 2022 22:53

david-crespo commented Jul 29, 2022

View reviewed changes

vercel bot deployed to Preview July 29, 2022 22:57 View deployment

do a prefetch instead of a fetch in the loader, use RQ in component

2164f85

david-crespo force-pushed the try-loaders branch from 312ad17 to 2164f85 Compare July 30, 2022 04:54

vercel bot deployed to Preview July 30, 2022 05:00 View deployment

zephraph reviewed Aug 1, 2022

View reviewed changes

app/hooks/use-params.ts Show resolved Hide resolved

zephraph reviewed Aug 1, 2022

View reviewed changes

bump RR to 6.4.0-pre.11

c7f36e4

vercel bot deployed to Preview August 2, 2022 17:22 View deployment

use loader for vpcs index too

5c17e8a

vercel bot deployed to Preview August 2, 2022 20:50 View deployment

david-crespo added 2 commits August 3, 2022 15:12

Merge main into try-loaders

1c3b0e2

rework json helper to make adding delay easier

159875e

vercel bot deployed to Preview August 3, 2022 20:35 View deployment

david-crespo changed the title ~~Try using RR loaders as a basis for better error handling~~ Using RR loaders in more places for clean page transitions Aug 3, 2022

use loaders to prefetch a bunch of queries

6e9c445

loaders 4 all! you get a loader! you get a loader!

david-crespo changed the title ~~Using RR loaders in more places for clean page transitions~~ Using RR loaders all over for clean page transitions Aug 4, 2022

david-crespo commented Aug 4, 2022

View reviewed changes

vercel bot deployed to Preview August 4, 2022 04:57 View deployment

david-crespo commented Aug 4, 2022

View reviewed changes

david-crespo marked this pull request as ready for review August 4, 2022 05:06

david-crespo merged commit 57fda45 into main Aug 4, 2022

david-crespo deleted the try-loaders branch August 4, 2022 15:38

david-crespo added a commit that referenced this pull request Aug 12, 2022

bump RR to 6.4.0-pre.12 so we can get rid of the patch from #1101

b4cbb08

david-crespo mentioned this pull request Oct 20, 2022

Notes on using React Router loaders and React Query together #1245

Closed

Using RR loaders all over for clean page transitions #1101

Using RR loaders all over for clean page transitions #1101

Uh oh!

Conversation

david-crespo commented Jul 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What loaders do for page transitions

Before: render, then fetch

After: fetch first

Uh oh!

vercel bot commented Jul 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zephraph Aug 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

david-crespo Aug 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

david-crespo commented Aug 4, 2022

Uh oh!

zephraph commented Aug 4, 2022

Uh oh!

david-crespo commented Aug 4, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

david-crespo commented Jul 29, 2022 •

edited

Loading

vercel bot commented Jul 29, 2022 •

edited

Loading

zephraph Aug 1, 2022 •

edited

Loading

david-crespo Aug 4, 2022 •

edited

Loading