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

[Feature]: The Route component can provide loader data #9768

Closed
debel27 opened this issue Dec 21, 2022 · 2 comments
Closed

[Feature]: The Route component can provide loader data #9768

debel27 opened this issue Dec 21, 2022 · 2 comments
Labels

Comments

@debel27
Copy link

debel27 commented Dec 21, 2022

What is the new or updated feature that you are suggesting?

Proposal: Update the element prop of the Route component so it also accepts a function. If provided, the function is given the loader data.

function SomeComponent() {
  return (
    <Route
      path="/user/:id"
      loader={({ params }) => loadUser(params.id)}
      element={({ data }) => ( 
        // ...
      )}
    />
  );
}

Why should this feature be included?

This proposal aims at achieving similar results than #9189, but with a more "type safe" approach.

Currently, to access loader data you need to use the useLoaderData hook. The usage of a hook makes Typescript lose track of the type of the data. If Route provides the data to its element prop, then react-router can make use of generics to ensure that type information is not lost.

Before

type RouteProps = {
  // ...
  loader?: (args: LoaderFunctionArgs): Promise<Response> | Response | Promise<any> | any;
  element?: React.ReactNode | null;
}

After

type RouteProps<T> = {
  // ...
  loader?: (args: LoaderFunctionArgs): Promise<Response> | Response | Promise<T> | T;
  element?: React.ReactNode |  null | ((args: { data: T }) => React.ReactNode | null);
}

If loader returns a Promise<Response> or Response, and if element is a function, then we'll have to trust that the deserialized data will be of type T.

I do not have the full picture about the ins and outs of useLoaderData, so I'm aware that my proposal may not be in line with react-router's architecture.

EDIT 2023-01-04

I just stumbled upon the FAQ that explains why Route no longer accepts render props. I understand the motivation behind this choice and I don't expect the Remix team to make this issue a priority.

However I'm sure the team is aware that using react-router's hooks is not the panacea. Not just for the type-safety issue mentioned above. As Fiona described below, they tend to add more coupling between your components and react-router. I can only hope you'd reconsider the tradeoffs of render-props :)

@debel27 debel27 changed the title [Feature]: Route's "element" prop to provide loader data [Feature]: The Route component can provide loader data Dec 22, 2022
@fionawhim
Copy link

This is also appealing because components that call useLoaderData are trickier to test using things like Storybook owing to needing to mock how to get data in there.

For my project I’ve made a typesafe helper to achieve the same as the above feature request:

  {
    path: 'survey',
    ...renderWithLoader(loadSurvey, (props) => <SurveyContainer {...props} />),
  }

expands to the following, enforcing the type invariant that awaited type of loadSurvey matches that of props passed into the renderer function.

  {
    path: 'survey',
    loader: loadSurvey,
    element: <LoaderDataWrapper renderer={(props) => <SurveyContainer {...props} />} />
  }

This doesn’t handle the “loader function returning a Request” capability, but I don’t intend to use that anyway.

export type Awaitable<T> = T | PromiseLike<T>;

export function renderWithLoader<T extends object>(
  loader: (args: LoaderFunctionArgs) => Awaitable<T>,
  renderer: (props: T) => React.ReactElement
): Pick<RouteObject, 'element' | 'loader'> {
  return { element: <LoaderDataWrapper renderer={renderer} />, loader };
}

function LoaderDataWrapper<T>({
  renderer,
}: {
  renderer: (props: T) => React.ReactElement;
}): React.ReactElement {
  const props = useLoaderData() as T;

  return renderer(props);
}

@brophdawg11
Copy link
Contributor

Superseded by #9687

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

No branches or pull requests

3 participants