-
-
Notifications
You must be signed in to change notification settings - Fork 10.5k
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
Add lazy route property API, use for loading middleware #13294
Merged
+2,377
−532
Merged
Changes from all commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
e29c07b
Add initial lazy property logic
markdalgleish 9d5e920
Fix type errors
markdalgleish 1ef679b
Remove outdated test for `unstable_lazyMiddleware` check
markdalgleish 8cc1b2b
Add tests for falsy lazy properties
markdalgleish adda7b7
Add tests for unsupported properties, refactor
markdalgleish 18c8733
Hook lazy property API up to framework
markdalgleish 4ec8ed3
Clean up redundant await
markdalgleish f0d3b89
Unify lazy route property loading logic
markdalgleish a61207a
Improve lazy route property types
markdalgleish 9c48bb8
Fix null and undefined check
markdalgleish 55e1d27
Update docs
markdalgleish 04287d2
Merge branch 'dev' into markdalgleish/lazy-route-properties
markdalgleish 4366a72
Update changeset
markdalgleish 2a16477
Refactor
markdalgleish ad8ce76
Merge branch 'dev' into markdalgleish/lazy-route-properties
markdalgleish 8c9c4ad
Simplify clientLoader/Action logic, refactor
markdalgleish File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
--- | ||
"react-router": minor | ||
--- | ||
|
||
Add granular object-based API for `route.lazy` to support lazy loading of individual route properties, for example: | ||
|
||
```ts | ||
createBrowserRouter([ | ||
{ | ||
path: "/show/:showId", | ||
lazy: { | ||
loader: async () => (await import("./show.loader.js")).loader, | ||
action: async () => (await import("./show.action.js")).action, | ||
Component: async () => (await import("./show.component.js")).Component, | ||
}, | ||
}, | ||
]); | ||
``` | ||
|
||
**Breaking change for `route.unstable_lazyMiddleware` consumers** | ||
|
||
The `route.unstable_lazyMiddleware` property is no longer supported. If you want to lazily load middleware, you must use the new object-based `route.lazy` API with `route.lazy.unstable_middleware`, for example: | ||
|
||
```ts | ||
createBrowserRouter([ | ||
{ | ||
path: "/show/:showId", | ||
lazy: { | ||
unstable_middleware: async () => | ||
(await import("./show.middleware.js")).middleware, | ||
// etc. | ||
}, | ||
}, | ||
]); | ||
``` |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -562,26 +562,30 @@ describe("context/middleware", () => { | |
{ | ||
id: "parent", | ||
path: "/parent", | ||
unstable_lazyMiddleware: async () => [ | ||
async ({ context }, next) => { | ||
await next(); | ||
// Grab a snapshot at the end of the upwards middleware chain | ||
snapshot = context.get(orderContext); | ||
}, | ||
getOrderMiddleware(orderContext, "a"), | ||
getOrderMiddleware(orderContext, "b"), | ||
], | ||
lazy: { | ||
unstable_middleware: async () => [ | ||
async ({ context }, next) => { | ||
await next(); | ||
// Grab a snapshot at the end of the upwards middleware chain | ||
snapshot = context.get(orderContext); | ||
}, | ||
getOrderMiddleware(orderContext, "a"), | ||
getOrderMiddleware(orderContext, "b"), | ||
], | ||
}, | ||
loader({ context }) { | ||
context.get(orderContext).push("parent loader"); | ||
}, | ||
children: [ | ||
{ | ||
id: "child", | ||
path: "child", | ||
unstable_lazyMiddleware: async () => [ | ||
getOrderMiddleware(orderContext, "c"), | ||
getOrderMiddleware(orderContext, "d"), | ||
], | ||
lazy: { | ||
unstable_middleware: async () => [ | ||
getOrderMiddleware(orderContext, "c"), | ||
getOrderMiddleware(orderContext, "d"), | ||
], | ||
}, | ||
loader({ context }) { | ||
context.get(orderContext).push("child loader"); | ||
}, | ||
|
@@ -634,10 +638,12 @@ describe("context/middleware", () => { | |
{ | ||
id: "child", | ||
path: "child", | ||
unstable_lazyMiddleware: async () => [ | ||
getOrderMiddleware(orderContext, "c"), | ||
getOrderMiddleware(orderContext, "d"), | ||
], | ||
lazy: { | ||
unstable_middleware: async () => [ | ||
getOrderMiddleware(orderContext, "c"), | ||
getOrderMiddleware(orderContext, "d"), | ||
], | ||
}, | ||
loader({ context }) { | ||
context.get(orderContext).push("child loader"); | ||
}, | ||
|
@@ -663,7 +669,7 @@ describe("context/middleware", () => { | |
]); | ||
}); | ||
|
||
it("ignores middleware returned from route.lazy", async () => { | ||
it("ignores middleware returned from route.lazy function", async () => { | ||
let snapshot; | ||
|
||
let consoleWarn = jest | ||
|
@@ -679,15 +685,17 @@ describe("context/middleware", () => { | |
{ | ||
id: "parent", | ||
path: "/parent", | ||
unstable_lazyMiddleware: async () => [ | ||
async ({ context }, next) => { | ||
await next(); | ||
// Grab a snapshot at the end of the upwards middleware chain | ||
snapshot = context.get(orderContext); | ||
}, | ||
getOrderMiddleware(orderContext, "a"), | ||
getOrderMiddleware(orderContext, "b"), | ||
], | ||
lazy: { | ||
unstable_middleware: async () => [ | ||
async ({ context }, next) => { | ||
await next(); | ||
// Grab a snapshot at the end of the upwards middleware chain | ||
snapshot = context.get(orderContext); | ||
}, | ||
getOrderMiddleware(orderContext, "a"), | ||
getOrderMiddleware(orderContext, "b"), | ||
], | ||
}, | ||
loader({ context }) { | ||
context.get(orderContext).push("parent loader"); | ||
}, | ||
|
@@ -726,70 +734,6 @@ describe("context/middleware", () => { | |
"Route property unstable_middleware is not a supported property to be returned from a lazy route function. This property will be ignored." | ||
); | ||
}); | ||
|
||
it("ignores lazy middleware returned from route.lazy", async () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test was removed because |
||
let snapshot; | ||
|
||
let consoleWarn = jest | ||
.spyOn(console, "warn") | ||
.mockImplementation(() => {}); | ||
|
||
router = createRouter({ | ||
history: createMemoryHistory(), | ||
routes: [ | ||
{ | ||
path: "/", | ||
}, | ||
{ | ||
id: "parent", | ||
path: "/parent", | ||
unstable_lazyMiddleware: async () => [ | ||
async ({ context }, next) => { | ||
await next(); | ||
// Grab a snapshot at the end of the upwards middleware chain | ||
snapshot = context.get(orderContext); | ||
}, | ||
getOrderMiddleware(orderContext, "a"), | ||
getOrderMiddleware(orderContext, "b"), | ||
], | ||
loader({ context }) { | ||
context.get(orderContext).push("parent loader"); | ||
}, | ||
children: [ | ||
{ | ||
id: "child", | ||
path: "child", | ||
// @ts-expect-error | ||
lazy: async () => ({ | ||
unstable_lazyMiddleware: async () => [ | ||
getOrderMiddleware(orderContext, "c"), | ||
getOrderMiddleware(orderContext, "d"), | ||
], | ||
}), | ||
loader({ context }) { | ||
context.get(orderContext).push("child loader"); | ||
}, | ||
}, | ||
], | ||
}, | ||
], | ||
}); | ||
|
||
await router.navigate("/parent/child"); | ||
|
||
expect(snapshot).toEqual([ | ||
"a middleware - before next()", | ||
"b middleware - before next()", | ||
"parent loader", | ||
"child loader", | ||
"b middleware - after next()", | ||
"a middleware - after next()", | ||
]); | ||
|
||
expect(consoleWarn).toHaveBeenCalledWith( | ||
"Route property unstable_lazyMiddleware is not a supported property to be returned from a lazy route function. This property will be ignored." | ||
); | ||
}); | ||
}); | ||
|
||
describe("throwing", () => { | ||
|
@@ -1581,37 +1525,41 @@ describe("context/middleware", () => { | |
{ | ||
id: "parent", | ||
path: "/parent", | ||
unstable_lazyMiddleware: async () => [ | ||
async (_, next) => { | ||
let res = (await next()) as Response; | ||
res.headers.set("parent1", "yes"); | ||
return res; | ||
}, | ||
async (_, next) => { | ||
let res = (await next()) as Response; | ||
res.headers.set("parent2", "yes"); | ||
return res; | ||
}, | ||
], | ||
lazy: { | ||
unstable_middleware: async () => [ | ||
async (_, next) => { | ||
let res = (await next()) as Response; | ||
res.headers.set("parent1", "yes"); | ||
return res; | ||
}, | ||
async (_, next) => { | ||
let res = (await next()) as Response; | ||
res.headers.set("parent2", "yes"); | ||
return res; | ||
}, | ||
], | ||
}, | ||
loader() { | ||
return "PARENT"; | ||
}, | ||
children: [ | ||
{ | ||
id: "child", | ||
path: "child", | ||
unstable_lazyMiddleware: async () => [ | ||
async (_, next) => { | ||
let res = (await next()) as Response; | ||
res.headers.set("child1", "yes"); | ||
return res; | ||
}, | ||
async (_, next) => { | ||
let res = (await next()) as Response; | ||
res.headers.set("child2", "yes"); | ||
return res; | ||
}, | ||
], | ||
lazy: { | ||
unstable_middleware: async () => [ | ||
async (_, next) => { | ||
let res = (await next()) as Response; | ||
res.headers.set("child1", "yes"); | ||
return res; | ||
}, | ||
async (_, next) => { | ||
let res = (await next()) as Response; | ||
res.headers.set("child2", "yes"); | ||
return res; | ||
}, | ||
], | ||
}, | ||
loader() { | ||
return "CHILD"; | ||
}, | ||
|
@@ -2507,37 +2455,41 @@ describe("context/middleware", () => { | |
{ | ||
id: "parent", | ||
path: "/parent", | ||
unstable_lazyMiddleware: async () => [ | ||
async ({ context }, next) => { | ||
let res = (await next()) as Response; | ||
res.headers.set("parent1", "yes"); | ||
return res; | ||
}, | ||
async ({ context }, next) => { | ||
let res = (await next()) as Response; | ||
res.headers.set("parent2", "yes"); | ||
return res; | ||
}, | ||
], | ||
lazy: { | ||
unstable_middleware: async () => [ | ||
async ({ context }, next) => { | ||
let res = (await next()) as Response; | ||
res.headers.set("parent1", "yes"); | ||
return res; | ||
}, | ||
async ({ context }, next) => { | ||
let res = (await next()) as Response; | ||
res.headers.set("parent2", "yes"); | ||
return res; | ||
}, | ||
], | ||
}, | ||
loader() { | ||
return new Response("PARENT"); | ||
}, | ||
children: [ | ||
{ | ||
id: "child", | ||
path: "child", | ||
unstable_lazyMiddleware: async () => [ | ||
async ({ context }, next) => { | ||
let res = (await next()) as Response; | ||
res.headers.set("child1", "yes"); | ||
return res; | ||
}, | ||
async ({ context }, next) => { | ||
let res = (await next()) as Response; | ||
res.headers.set("child2", "yes"); | ||
return res; | ||
}, | ||
], | ||
lazy: { | ||
unstable_middleware: async () => [ | ||
async ({ context }, next) => { | ||
let res = (await next()) as Response; | ||
res.headers.set("child1", "yes"); | ||
return res; | ||
}, | ||
async ({ context }, next) => { | ||
let res = (await next()) as Response; | ||
res.headers.set("child2", "yes"); | ||
return res; | ||
}, | ||
], | ||
}, | ||
loader({ context }) { | ||
return new Response("CHILD"); | ||
}, | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
huh - I didn't realize our docs showed this type of granular file splitting. I feel like the more common use case is
lazy: () => import("./route")
so I would actually vote to document that version with a note on how to get more granular if/when neededThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel this is the better API to document because it handles all properties, whereas the lazy function can't handle middleware. I also feel like this version is a bit clearer in terms of what's going on.