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

Lazy Loading react component get some warning #25

Closed
bsnman opened this issue Sep 20, 2023 · 17 comments
Closed

Lazy Loading react component get some warning #25

bsnman opened this issue Sep 20, 2023 · 17 comments

Comments

@bsnman
Copy link

bsnman commented Sep 20, 2023

I am building a Vite react ts app. I wanted to code split my components by using React.lazy
I tried import the component like so

import { lazy } from "react";
const Login = lazy(() => import("./Login.tsx"));

Is this supported? or am I doing something wrong

@maxxxymum
Copy link

@bsnman

You can check the Vite output in console for something like:

[vite] hmr invalidate /src/components/Login.tsx Could not Fast Refresh. Learn more at https://github.com/vitejs/vite-plugin-react/tree/main/packages/plugin-react#consistent-components-exports

Your component is breaking fast refresh if this message is present I guess.

@bsnman
Copy link
Author

bsnman commented Sep 21, 2023

No, it does not output

[vite] hmr invalidate /src/components/Login.tsx Could not Fast Refresh. Learn more at https://github.com/vitejs/vite-plugin-react/tree/main/packages/plugin-react#consistent-components-exports

HMR Fast Refresh actually works, but the eslint react-refresh gives a warning on my IDE

ESLint: Fast refresh only works when a file only exports components. Move your component(s) to a separate file.(react-refresh/only-export-components)

Should this be fix on this eslint plugin?

@ArnaudBarre
Copy link
Owner

Can I have more code that trigger the issue. I don't think this two lines triggers a warning

@bsnman
Copy link
Author

bsnman commented Sep 21, 2023

Yes, here is the code for my router.ts that I want to lazy load some components.

// router.ts
import { createBrowserRouter } from "react-router-dom";
import { lazy } from "react";

const Login = lazy(() => import("/src/pages/Login"));   // warning here
const Home = lazy(() => import("/src/pages/Home"));    // and here

const router = createBrowserRouter([
  {
    path: "/login",
    element: <Login />,
  },
  {
    path: "",
    element: <Home />,
  },
]);

export default router;

Login component looks like so

import ...

function Login() {
    ...
    return <>...</>
}

export default Login;

Importing them without React.lazy does not trigger a warning

@ArnaudBarre
Copy link
Owner

ArnaudBarre commented Sep 21, 2023

Yeah this kind of a false warning, I will see what I can do.
There are two solutions:

  • Put a disable comment for this rule for all the file. This will not actually break fast refresh so this ok
  • Instead of exporting the router, export a component that render the Router provider

The advantage of the second solution is that if react-rouder handle runtime change in the router config, you could HMR updates to the config

@bsnman
Copy link
Author

bsnman commented Sep 25, 2023

I'll try implementing the second solution first. Thanks for the quick feedback.

@netcoding87
Copy link

Second option works fine for me 👍

Example:

import {
  RouteObject,
  RouterProvider,
  createBrowserRouter,
} from 'react-router-dom'

const MyComponent = React.lazy(() => import('./MyComponent'))

const routes: RouteObject[] = [
  {
    element: <MyComponent />,
    path: 'example',
  },
]

const router = createBrowserRouter(routes)

export const Routes: React.FC = () => {
  return <RouterProvider router={router} />
}

@ArnaudBarre
Copy link
Owner

I looked at it more carefully and both the Babel plugin and SWC are generating some registration code for const Login = lazy(() => import("./pages/Login"));, so actually the warning may not be wrong. I need to look at it in more details, I will comeback to it affer other issues but I think exporting a wrapper of the RouterProvider is a good way to fix the issue for now

@bsnman
Copy link
Author

bsnman commented Oct 26, 2023

Second option works for me also. Thanks

@kurpav
Copy link

kurpav commented Jan 25, 2024

@netcoding87 thanks for the example.
I wonder how to export router then to do navigation outside of react components.
Are there any ideas?

@ArnaudBarre
Copy link
Owner

@kurpav In that case you can have a lazy-routes.tsx which contains exports like const ViewA = React.lazy(() => import('./views/ViewA.tsx')) and then have a router.tsx which can import all components and export the router.

Be careful to where you import the router so it doesn't create circular imports, which are bad for HMR.

@Nik96i
Copy link

Nik96i commented Apr 6, 2024

@ArnaudBarre
I want to implement option two. But what if I have two route files (public and protected) like React Bulletproof?

I can't create two browser routers. my route files are very similar to React bulletproof.

@ArnaudBarre
Copy link
Owner

Same as the answer above, you need to put the the lazy wrappers into a separate file so that the router file doesn't contains any component and can safely be exported

@ImVictorM
Copy link

I was able to get around this by using react's createElement() and changing the file extension from .tsx to .ts.
So the code went from this:

// routesPublic.tsx
import { lazy } from "react";
import { RouteObject } from "react-router-dom";

// Fast refresh warning!
const Login = lazy(() => import("@/pages/Login"));

const routesPublic: RouteObject[] = [
  {
    path: "/",
    element: <Login />,
  },
];

export default routesPublic;

to this:

// routesPublic.ts
import { createElement, lazy } from "react";
import { RouteObject } from "react-router-dom";

// No warning!
const Login = lazy(() => import("@/pages/Login"));

const routesPublic: RouteObject[] = [
  {
    path: "/",
    element: createElement(Login),
  },
];

export default routesPublic;

In my case, I'm using separate files for public and private routes. I wrapped them in a routes.tsx file and used them normally in the router.

// routes.tsx
import { RouteObject } from "react-router-dom";
import { RouteAuthRequired } from "../components";

import routesPrivate from "./routesPrivate";
import routesPublic from "./routesPublic";

export const routes: RouteObject[] = [
  ...routesPublic,
  {
    element: <RouteAuthRequired />,
    children: routesPrivate,
  },
];

export default routes;
// router.ts
import { createBrowserRouter } from "react-router-dom";
import routes from ".";

export const router = createBrowserRouter(routes);

export default router;

zhangwh754 added a commit to zhangwh754/survey-fe that referenced this issue Aug 14, 2024
zhangwh754 added a commit to zhangwh754/survey-fe that referenced this issue Aug 14, 2024
@DavidArmendariz
Copy link

any updates?

@ArnaudBarre
Copy link
Owner

Maybe I should close the issue, I don't think I can do much than adding a section to the documentation explaining that const Login = lazy(() => import("./pages/Login.tsx")); is creating a component and so that this files should obey to the Fast refresh rules

@ArnaudBarre ArnaudBarre pinned this issue Nov 24, 2024
@ArnaudBarre
Copy link
Owner

I've pinned the issue because I think other people will run into it, but yeah the limitation is annoying here but necessary to get nice DX. You can choose to ignore the warning on this router file, but I think exporting the route provider is what will get the trade off between number of files and DX: #25 (comment)

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

8 participants