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

[Bug]: lazy factory return value is not enforced in Typescript #10475

Closed
kwatkins-ld opened this issue May 10, 2023 · 18 comments · Fixed by #10634
Closed

[Bug]: lazy factory return value is not enforced in Typescript #10475

kwatkins-ld opened this issue May 10, 2023 · 18 comments · Fixed by #10634
Labels

Comments

@kwatkins-ld
Copy link

kwatkins-ld commented May 10, 2023

What version of React Router are you using?

6.9.0

Steps to Reproduce

The docs indicate that the return value of the factory function passed to lazy should be a Promise that resolves with a module that exports one or more of: Component, ErrorBoundary, action, or loader.

There is some type support for this already: the following code will produce a type error:

// The factory returns a Promise that resolves with undefined, so there is a type error that reads:
// "Type 'void' is not assignable to type 'Omit<NonIndexRouteObject, ImmutableRouteKey>'"
 <Route
    path="/foo"
    lazy={async () => {
      return Promise.resolve(); 
    }}
  />

But if you attempt to load a module that doesn't honor the convention, there is no type error:

// The factory returns a Promise that resolves with an empty object, which should produce a type
// error, but doesn't
 <Route
    path="/foo"
    lazy={async () => {
      return Promise.resolve({}); 
    }}
  />

Expected Behavior

Ideally the type system would indicate that the Promise returned from the lazy factory must have one or more of the following properties: Component, ErrorBoundary, action, loader.

Beyond type correctness, this would help prevent bugs: it is difficult to imagine a scenario where a consumer would want a route to render nothing and have no behavior.

Actual Behavior

It is currently possible to dynamically import a module that doesn't export one of Component, ErrorBoundary, action, loader, and there is no resulting type error.

@Huespal
Copy link

Huespal commented Jun 23, 2023

Was this fixed or modified somewhat?
I have a file A which is just a const A = () => <div>Hi!</div>; export default A;
And when importing it using React Router's lazy property it throws a very difficult to understand TS error:

Type '() => Promise<typeof import(".../src/components/A")>' is not assignable to type 'LazyRouteFunction<NonIndexRouteObject> | LazyRouteFunction<IndexRouteObject> | undefined'.
  Type '() => Promise<typeof import(".../src/components/A")>' is not assignable to type 'LazyRouteFunction<NonIndexRouteObject>'.
    Type 'Promise<typeof import(".../src/components/A")>' is not assignable to type 'Promise<Omit<NonIndexRouteObject, ImmutableRouteKey>>'.
      Type 'typeof import(".../src/components/A")' has no properties in common with type 'Omit<NonIndexRouteObject, ImmutableRouteKey>'.

I am just <Route path="a" lazy{() => import('../components/A')} /> using a data router like in the docs.

What am I doing wrong?

Thank you and sorry if this is not the place, but there's almost no info about this lazy property.

@brophdawg11
Copy link
Contributor

Have you reviewed https://reactrouter.com/en/main/route/lazy? Specifically, I think what you're missing is:

Then in your lazy route modules, export the properties you want defined for the route

You are not exporting any properties, you're exporting a default export. You instead need to export thigns like loader/action/element/Component/etc.

So if normally your route would be:

<Route path="a" element={<div>Hi!</div>} />

Then you lazy module needs to export the element property:

// lazy.tsx
export const element = {<div>Hi!</div>} />;

// app.tsx
<Route path="a" lazy={() => import('./lazy')} />

When using lazy I think it's cleaner to use the Component API though:

// lazy.tsx
export function Component() {
  return <div>Hi!</div>;
}

@brophdawg11 brophdawg11 linked a pull request Jun 23, 2023 that will close this issue
@brophdawg11 brophdawg11 added the awaiting release This issue have been fixed and will be released soon label Jun 30, 2023
@brophdawg11
Copy link
Contributor

This is fixed in #10634 and should be available in the next release after 6.14.1

@swirle13
Copy link

swirle13 commented Jul 6, 2023

I followed the createBrowserRouter tutorial to create a hash router, so my App.tsx looks like this

// App.tsx
// all my imports
const router = createHashRouter([
	{
		path: "/",
		lazy: () => import('./pages/Home'), 
	},
// the rest of my routes
// more boilerplate code
root.render(
    <React.StrictMode>
        <RouterProvider router={router} />
    </React.StrictMode>
);

and my Home.tsx is declared as a functional component:

export default function Home(): JSX.Element {
	return (
		<div>Hello World</div>
	)
}

and yet I still get the error Type 'Promise<typeof import("path/to/project/src/pages/Home")>' is not assignable to type 'LazyRouteFunction<IndexRouteObject> | LazyRouteFunction<NonIndexRouteObject> | undefined'.ts(2322)

I could only get this to work when I completely changed the function to a const of just a plain div, e.g.

// Home.tsx
export const element = <div>Hi</div>
export default element

But I cannot find any way to export a component and not have it complain about either missing default export or the same type error above. What am I doing wrong? I am using react-bootstrap components like Container, so it may be that, but I'm not sure why that would be the case..

In addition to this, can you please update your documentation for your page on lazy to show how to insert lazy calls in an object structure? Through trial and error, I found it to work when defined like this:

{
    path: "home",
    errorElement: <ErrorPage />,
    lazy: () => import('./pages/Home'),
},

Thank you, and apologies for the long comment on a closed issue, but it's been driving me nuts for how simple it should be and yet doesn't work.

@brophdawg11
Copy link
Contributor

Since the object returned from lazy is effectively spread onto the route, it expects you to return only properties that are valid on a <Route>/route object - properties such as element/Component, loader, action, errorElement/ErrorBoundary, etc.

When you do export default function Home() {...} you are exporting a default property, which is not a valid property to put on a Route object, that is likely why you're seeing an error. The error goes away when you do export const element because element is a valid route property. You could also use the Component property and do:

export function Component(): JSX.Element { ... }
``

Docs PRs are always welcome to clarify any confusion!

@wallace-sf
Copy link

wallace-sf commented Aug 3, 2023

@brophdawg11 How can i pass props to component? By example, i have a private route which i define the user claim needed in component props:

// routes.tsx
...
<Route
    path="/system"
    element={<PrivateRoute allowedUserType="system" />}
>
...

Then, how can i use following code with lazy?

// routes.tsx
...
<Route
     path="/system"
     lazy={async () => await import('~components/auth/PrivateRoute')} // where to pass property allowedUserType?
>
...

@brophdawg11
Copy link
Contributor

<Route path="/system" lazy={async () => {
  let mod = await import('~components/auth/PrivateRoute');
  return {
    ...mod,
    Component: undefined,
    element: <mod.Component allowedUserType="system" />
  };
}
>

@brophdawg11 brophdawg11 removed the awaiting release This issue have been fixed and will be released soon label Aug 10, 2023
@ponikar
Copy link

ponikar commented Aug 15, 2023

Documentation is not clear about how to lazy load components. Thank you. This worked.

So generally you have to export element from the file. Also you have to invoke that component in that file.

#10475 (comment)

@brophdawg11
Copy link
Contributor

Could you elaborate on how it's unclear? Docs PRs are always welcome if there's room for improvements!

Right now we keep it pretty simple with:

Then in your lazy route modules, export the properties you want defined for the route:

The idea is that you export whatever you would usually put directly on the <Route>. So:

// Before
<Route path="/" loader={someLoader} element={<SomeComponent />} />

// After - move `loader`/`element` to the lazy file
<Route path="/" lazy={() => import('./route') />

// ./route.js
export function loader() { ... }

function SomeComponent() {}
export const element = <SomeComponent />;

So generally you have to export element from the file. Also you have to invoke that component in that file.

You do not have to export element, we introduced Component for this exact reason - so you do not need to create the React Element inside your lazy import file.

// Before
<Route path="/" loader={someLoader} Component={SomeComponent} />

// After - move `loader`/`Component` to the lazy file
<Route path="/" lazy={() => import('./route') />

// ./route.js
export function loader() { ... }

export function Component() {}

@reisir
Copy link

reisir commented Aug 18, 2023

What's missing from all of these examples (and the docs!) is the default export.

lazy={() => import('./lazyRoute')} gets the default export, which doesn't contain anything if you don't explicitly set it.

// lazyRoute.js
export function Component(props) { ... }
export function loader() { ... }

// DEFAULT EXPORT OBJECT THAT IS SPREAD ON THE ROUTE!!! 
export default { Component, loader };

Edit: I'm mistaken. You just need to have anything but the Component be the default export and it'll work... Default exporting what's spread just makes semantic sense I guess

@brophdawg11
Copy link
Contributor

lazy={() => import('./lazyRoute')} only gets a default export if you include a default export in your file. The expectation is that you won't have a default export since default is not a valid route key. You should just export the route properties directly - loader, Component, etc.

I added a small note to the docs to call this out.

@ponikar
Copy link

ponikar commented Aug 19, 2023

how do you provide a fallback? like a suspense once?

@ponikar
Copy link

ponikar commented Aug 19, 2023

Hey @brophdawg11 I just exported my component as element and it worked earlier. I am curious is there any difference if you export your component as element or Component?

@brophdawg11
Copy link
Contributor

Folks, we really need to stop using this closed issue as a help forum :). Please ask questions in the Discord or open a Q&A Discussion on Github.

route.lazy is when you don't want to use Suspense and instead want to use useNavigation. You can use Suspense with React.lazy exactly as you would have before.

There is no difference between element and Component: https://reactrouter.com/en/main/route/route#elementcomponent.

@reidark
Copy link

reidark commented Oct 4, 2023

Sorry to commenting on this after @brophdawg11 asks to close or move, but just to give some workaround.

If you use default exports, an easy way to bypass the necessity of element or Component is to get the default value from import.

{
  path: "/pokemons",
  lazy: async () => {
    const Page = (await import("./pages/pokemons")).default;

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

@Xx-AmrHossam-xX
Copy link

Xx-AmrHossam-xX commented Nov 29, 2023

@reidark 's answer is the best if you want to still use the default export
#10475 (comment)

@mengdu
Copy link

mengdu commented Jan 12, 2024

I can solve it this way:

const lazyWrap = (factory: () => Promise<any>) => {
  return async () => {
    const page = await factory()
    // https://reactrouter.com/en/main/route/lazy
    return {
      Component: page.default || page.Component,
      ErrorBoundary: page.ErrorBoundary,
      loader: page.loader,
    }
  }
}

const router = createBrowserRouter([
   { path: '/demo', lazy: lazyWrap(() => import('./pages/Demo')) }
])

@LLmoskk
Copy link

LLmoskk commented Jul 5, 2024

I followed the createBrowserRouter tutorial to create a hash router, so my App.tsx looks like this我遵循了WebBrowserRouter教程来创建一个散列路由器,所以我的App.tsx看起来像这样

// App.tsx
// all my imports
const router = createHashRouter([
	{
		path: "/",
		lazy: () => import('./pages/Home'), 
	},
// the rest of my routes
// more boilerplate code
root.render(
    <React.StrictMode>
        <RouterProvider router={router} />
    </React.StrictMode>
);

and my Home.tsx is declared as a functional component:而我的Home.tsx被声明为一个功能组件:

export default function Home(): JSX.Element {
	return (
		<div>Hello World</div>
	)
}

and yet I still get the error Type 'Promise<typeof import("path/to/project/src/pages/Home")>' is not assignable to type 'LazyRouteFunction<IndexRouteObject> | LazyRouteFunction<NonIndexRouteObject> | undefined'.ts(2322)但我还是得到了错误 Type 'Promise<typeof import("path/to/project/src/pages/Home")>' is not assignable to type 'LazyRouteFunction<IndexRouteObject> | LazyRouteFunction<NonIndexRouteObject> | undefined'.ts(2322)

I could only get this to work when I completely changed the function to a const of just a plain div, e.g.只有当我完全将函数改为普通div的const时,我才能让它工作,例如。

// Home.tsx
export const element = <div>Hi</div>
export default element

But I cannot find any way to export a component and not have it complain about either missing default export or the same type error above. What am I doing wrong? I am using react-bootstrap components like Container, so it may be that, but I'm not sure why that would be the case..但是我找不到任何方法来导出一个组件,而不是让它抱怨缺少默认导出或上面的相同类型的错误。我做错什么了吗?我正在使用react-bootstrap组件,如 Container ,所以可能是这样,但我不确定为什么会这样。

In addition to this, can you please update your documentation for your page on lazy to show how to insert lazy calls in an object structure? Through trial and error, I found it to work when defined like this:除此之外,你能更新一下你的lazy页面的文档,展示一下如何在对象结构中插入lazy调用吗?通过试验和错误,我发现它在这样定义时工作:

{
    path: "home",
    errorElement: <ErrorPage />,
    lazy: () => import('./pages/Home'),
},

Thank you, and apologies for the long comment on a closed issue, but it's been driving me nuts for how simple it should be and yet doesn't work.谢谢你,并为一个封闭的问题上的长时间评论道歉,但它一直让我发疯,因为它应该是多么简单,但却不起作用。

I know the reason. First, your component cannot be export default. Second, if you change the name of your component Home to "Component", there will be no error. I also encountered the same problem. It requires that the name of the lazy component can only be "Component". This is how I solved this problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.