reactjsreact-routerreact-router-dom

Is it bad approach to make wrappers for public/private routes in react-router v6 like this


In app I know that user is authenticated when there is token in redux.
When app loads I first fetch that token from server.
I made a wrapper for private and public routes.
I made wrapper for public routes because if user is on login page and reload app once token is fetched I need to make redirect to authenticated page.
What troubles me is this approach ok and does it make performance problems for the future? Without public route wrapper user is able to access login page when authenticated (which should not).
I am trying to make a clean solution as possible. This is my routes rendering:

return (
    <Routes>
       {routes.map((route, index) => {
        if (route.isProtected) {
          return (
            <Route
              key={index}
              path={route.path}
              element={
                <RequireAuth>            <<<< wrapper
                  <route.component />
                </RequireAuth>
              }
            />
          );
        } else {
          return (
            <Route
              key={index}
              path={route.path}
              element={<Public><route.component /></Public>} <<<< wrapper
            />
          );
        }
      })} 
      <Route path="*" element={<NotFoundPage />} />
    </Routes>
  );

Public wrapper

function Public({ children }: { children: JSX.Element }) {
  const { state } = useLocation();
  const {token, tokenChecking} = useAppSelector((state) => state.auth);
  
  if (tokenChecking) {
    return (
        <div>app is loading...</div>
    )
}
  
  if (token) {
    return <Navigate to={state?.from || "/dashboard"}  replace/>;
  }

  return children;
}

Private route is more or less reverse

function RequireAuth({ children }: { children: JSX.Element }) {
  let location = useLocation();

  const {token, tokenChecking} = useAppSelector((state) => state.auth);
  
  if (token) {
    return (
        <div>App is loading ...</div>
    )
}

  if (!token) {
    return <Navigate to={"/login"} state={{ from: location }} />;
  }

  return children;
}

Solution

  • What troubles me is this approach ok and does it make performance problems for the future?

    This approach seems reasonable to me and I don't foresee any performance issues with it.

    If you want to make the code a bit more DRY then I suggest looking at the isProtected flag first and instantiate the wrapper first, then render a single wrapper and component. Remember that React components are PascalCased, so create a Component to render into the wrapper.

    return (
      <Routes>
        {routes.map((route) => {
          const Wrapper = route.isProtected ? RequireAuth : Public;
          const Component = route.component;
          return (
            <Route
              key={route.path}
              path={route.path}
              element={
                <Wrapper> 
                  <Component />
                </Wrapper>
              }
            />
          );
        })} 
        <Route path="*" element={<NotFoundPage />} />
      </Routes>
    );