Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot use defer on a loader with typesafeBrowserRouter #16

Open
stevan-borus opened this issue May 10, 2024 · 8 comments
Open

Cannot use defer on a loader with typesafeBrowserRouter #16

stevan-borus opened this issue May 10, 2024 · 8 comments

Comments

@stevan-borus
Copy link

I have a loader with defer:

import { defer } from 'react-router-typesafe';
...

export const dashboardLoader = (queryClient: QueryClient) => () => {
  return defer<{
    dashboardInfo: Promise<DashboardType>;
  }>({
    dashboardInfo: queryClient.ensureQueryData(dashboardInfoQueries.info()),
  });
};

and I have a RouteObject like this in typesafeBrowserRouter:

{
   index: true,
   loader: dashboardLoader(queryClient),
   async lazy() {
         const { Dashboard } = await import('@/views/dashboard/Dashboard');

         return { element: <Dashboard /> };
   },
},

I get an error: Type () => DeferredData<{ dashboardInfo: Promise<DashboardType>; }> is not assignable to type NarrowKeys<boolean | LoaderFunction<any> | undefined>

This doesn't happen when I am using a regular loader without defer.

Oh, and when I just switch from typesafeBrowserRouter to createBrowserRouter everything works just fine since then, loader doesn't have that NarrowKeys type.

@fredericoo
Copy link
Owner

fredericoo commented May 10, 2024

could I ask you to try to refactor your loader to the following? I believe it's the fact returning defer isn't overlapping with the LoaderFunction type. I’ll have a deeper look but this may do the trick:

import { defer, makeLoader } from 'react-router-typesafe';
...

export const dashboardLoader = (queryClient: QueryClient) => makeLoader(() => {
  return defer<{
    dashboardInfo: Promise<DashboardType>;
  }>({
    dashboardInfo: queryClient.ensureQueryData(dashboardInfoQueries.info()),
  });
});

the single change here is makeLoader: it’ll auto-add a satisfies keyword to make sure you’re returning a loader-compatible value. You may get a ts error if that defer doesn't match, which is my main bet!

on a personal note, refrain from declaring the generic parameter on the defer function if you can, and rely on the return of the function you have — it should infer just fine, and be more refactorable

@stevan-borus
Copy link
Author

I figured out that I get this error on a regular loader too since I moved it outside the lazy loaded module.

makeLoader doesn't solve the problem on both of those loaders. The funny thing is that the app works fine, I just get that typescript error.

Related to the typing of the defer definitely:
I see that it is inferring the type now correctly where I useLoaderData after I removed the declared generic.
I put it there because when I hover over dashboardLoader it gives my this: function dashboardLoader(queryClient: QueryClient): () => DeferredData<{dashboardInfo: any}> if there is no declared generic.
With it I get function dashboardLoader(queryClient: QueryClient): () => DeferredData<{dashboardInfo: DashboardType}> when I hover over it.
But it doesn't matter apparently.

@fredericoo
Copy link
Owner

and you’re saying it doesn’t error on a regular createBrowserRouter call?

could you make a minimal reproduction? I’d love to fix that

@stevan-borus
Copy link
Author

and you’re saying it doesn’t error on a regular createBrowserRouter call?

Yup

I made this real quick.. not sure what is going on with the type of typesafeBrowserRouter in the sandbox.
Don't have the time now to figure it out.

Here is the repro:
https://codesandbox.io/p/sandbox/typesafe-router-c4gryh

@stevan-borus
Copy link
Author

stevan-borus commented May 13, 2024

When I define the generic on makeLoader with LoaderFunction<any> it works and I don't get the error anymore..

Like this:

export const dashboardLoader = (queryClient: QueryClient) =>
  makeLoader<LoaderFunction<any>>(() => {
    return defer({
      dashboardInfo: queryClient.fetchQuery(dashboardInfoQueries.info()),
    });
  });

Edit: but then useLoaderData<ReturnType<typeof dashboardLoader>>() returns {} | null and I can't use the promise so it's not really a solution

@stevan-borus
Copy link
Author

Found out what the actual issue is and I've changed the code in the codesandbox repro.

As you can see, I get the error when I lazy load only the component. If I don't lazy load anything or lazy load both the component and loader then everything works correctly.

So these versions work:

{
   index: true,
   async lazy() {
         const { Dashboard, dashboardLoader } = await import('@/views/dashboard/Dashboard');

         return { element: <Dashboard />, loader: dashboardLoader(queryClient) };
   },
},

and

{
   index: true,
   element: <Dashboard />, 
   loader: dashboardLoader(queryClient)
},

but this one errors:

{
   index: true,
   loader: dashboardLoader(queryClient),
   async lazy() {
         const { Dashboard } = await import('@/views/dashboard/Dashboard');

         return { element: <Dashboard /> };
   },
},

@fredericoo
Copy link
Owner

hey mate,

appreciate the repro, I’m currently pretty sick so will be back next week. Have you had a look at the React Conf talk by Ryan? it looks like this library will be implemented internally as well, I’m really happy by that!

@stevan-borus
Copy link
Author

I hope you are well now.. watched it a couple of days ago. I started migrating an app just before to the new data router so I can slowly transfer it to remix SPA and SSR after that, so I am really excited about this 😃

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

No branches or pull requests

2 participants