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

feat(react-router-dom): add createModuleRoutes utility function #9830

Closed
wants to merge 9 commits into from

Conversation

rossipedia
Copy link

@rossipedia rossipedia commented Jan 7, 2023

Discussion - Demo Project

The createModuleRoutes() utility is inspired by the Remix Route module convention. It allows a dev to provide a module function on their route that dynamically imports a route module with named exports, instead of explicitly adding those properties to the Route object.

That is to say, it turns this:

import Foo, { fooLoader, fooAction, FooErrorBoundary } from './pages/foo';

{
  path: "foo",
  element: <Foo />,
  loader: fooLoader,
  action: fooAction,
  errorElement: <FooErrorBoundary />,
}

into this:

{
  path: "foo",
  module: () => import('./pages/foo'),
}

Any properties set directly on the route object take precedence over module exports. So if you have specific routes you want to not lazy load the loader so it can fetch data as fast as possible, you just specify loader directly on the route alongside the module, and that will be the loader that gets invoked (the implementation won't even both trying to set up a lazy-loaded loader in that case):

{
  path: "foo",
  module: () => import('./pages/foo'),
  loader: fooLoader,
}

The same goes for action, element

By leveraging import() and React.lazy() the implementation is super straightforward, and practically every build tool and bundler knows how to code-split when it sees that.

I think this could be useful for a large number of React Router users by providing a "happy path" for code-splitting and lazy loading without being too heavy-handed.

It can also serve as a foundation for file-based routing plugins for things like Webpack/Vite/Parcel/etc.

This gives the developer a lever to pull between:

  • bundling all your loaders in the primary chunk for fastest possible execution of data fetching, at the cost of larger bundle sizes
  • lazily loading your loader/action code along with your UI component for smallest initial bundle size at the cost of additional latency when lazy loading the route module code
  • or any configuration in between

Which I think is very much in the Remix spirit of things :) Feedback welcome!


PS: I know I probably left out a bunch of stuff (docs/tests/etc), if this is something y'all are interested in merging I'll be sure to flesh out this PR with all those goodies. I also have next to zero experience with React Native, so not sure if this is something useful in that space either.

@changeset-bot
Copy link

changeset-bot bot commented Jan 7, 2023

⚠️ No Changeset found

Latest commit: 0ff9113

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@remix-cla-bot
Copy link
Contributor

remix-cla-bot bot commented Jan 7, 2023

Hi @rossipedia,

Welcome, and thank you for contributing to React Router!

Before we consider your pull request, we ask that you sign our Contributor License Agreement (CLA). We require this only once.

You may review the CLA and sign it by adding your name to contributors.yml.

Once the CLA is signed, the CLA Signed label will be added to the pull request.

If you have already signed the CLA and received this response in error, or if you have any questions, please contact us at hello@remix.run.

Thanks!

- The Remix team

@remix-cla-bot
Copy link
Contributor

remix-cla-bot bot commented Jan 7, 2023

Thank you for signing the Contributor License Agreement. Let's get this merged! 🥳

Co-authored-by: Sergio Xalambrí <hello@sergiodxa.com>
@rossipedia
Copy link
Author

Ahh that was the bit I was worried about. Does this kind of thing make sense for RR Native? If not, then I'd assume it should be moved to react-router-dom.

@sergiodxa
Copy link
Member

Ahh that was the bit I was worried about. Does this kind of thing make sense for RR Native? If not, then I'd assume it should be moved to react-router-dom.

There’s no createNativeRouter to use loaders and actions in RR Native so this only work for the DOM package IMO

@rossipedia
Copy link
Author

Ok that's fine, I can move it directly into react-router-dom them 👍

@rossipedia rossipedia marked this pull request as ready for review January 8, 2023 23:16
@MichaelDeBoey MichaelDeBoey changed the title add createModuleRoutes() utility function for lazy loading route modules feat(react-router-dom): add createModuleRoutes utility function Jan 27, 2023
@brophdawg11 brophdawg11 mentioned this pull request Feb 3, 2023
2 tasks
@brophdawg11
Copy link
Contributor

Thanks for the proposal and POC @rossipedia! I'm going to close this now that it's being superseded by #10045.

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

Successfully merging this pull request may close these issues.

None yet

4 participants