Skip to content

Add client middleware to split route modules #13210

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

Merged
merged 8 commits into from
Mar 19, 2025

Conversation

markdalgleish
Copy link
Member

@markdalgleish markdalgleish commented Mar 12, 2025

When future.unstable_splitRouteModules is enabled, the unstable_clientMiddleware route export is now split into its own chunk so it can be downloaded and executed ASAP.

For this to work, we've had to introduce a new route.unstable_lazyMiddleware property in react-router and ignore middleware defined in the return value from route.lazy. This allows us to know ahead of time whether we need to wait for the middleware, whereas with route.lazy we'd have no idea if there's middleware or not until after it's resolved.

Copy link

changeset-bot bot commented Mar 12, 2025

🦋 Changeset detected

Latest commit: e878b8d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 11 packages
Name Type
@react-router/dev Patch
react-router Patch
@react-router/fs-routes Patch
@react-router/remix-routes-option-adapter Patch
@react-router/architect Patch
@react-router/cloudflare Patch
react-router-dom Patch
@react-router/express Patch
@react-router/node Patch
@react-router/serve Patch
create-react-router Patch

Not sure what this means? Click here to learn what changesets are.

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

@markdalgleish markdalgleish marked this pull request as ready for review March 12, 2025 11:05
@brophdawg11 brophdawg11 self-assigned this Mar 12, 2025
@@ -1074,7 +1253,7 @@ test.describe("Middleware", () => {
await page.waitForSelector("[data-child]");

// 2 separate server requests made
expect(requests).toEqual([
expect(requests.sort()).toEqual([
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test was flaky in Firefox due to the order of requests not being deterministic.

expect.stringMatching(
/\/parent\/child\.data\?_routes=routes%2Fparent\.child\._index$/
),
expect(requests.sort()).toEqual([
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above.

Copy link
Contributor

@brophdawg11 brophdawg11 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Gave it a run in the playground locally and everything looked good

let clientMiddlewareModule = await import(
/* @vite-ignore */
/* webpackIgnore: true */
route.clientMiddlewareModule || route.module
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯 This is really clever - it took me a sec to figure out how it was working without the route chunks since we weren't waiting on the route module promise in dataStrategy anymore

@brophdawg11 brophdawg11 removed their assignment Mar 13, 2025
@markdalgleish markdalgleish merged commit 91c7bac into dev Mar 19, 2025
14 of 17 checks passed
@markdalgleish markdalgleish deleted the markdalgleish/lazy-middleware branch March 19, 2025 07:43
Copy link
Contributor

🤖 Hello there,

We just published version 7.4.1-pre.0 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

Copy link
Contributor

🤖 Hello there,

We just published version 7.4.1 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

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.

2 participants