Use granular route.lazy for loaders/actions
#13339
Conversation
🦋 Changeset detectedLatest commit: 7ff03a1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 packages
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
changed the title
route.lazy for loaders/actionsroute.lazy for loaders/actions
| // @ts-expect-error | ||
| caseSensitive: async () => true, | ||
| // @ts-expect-error | ||
| path: async () => "/lazy/path", | ||
| // @ts-expect-error | ||
| id: async () => "lazy", | ||
| // @ts-expect-error | ||
| index: async () => true, | ||
| // @ts-expect-error |
There was a problem hiding this comment.
These directives were unused because of the // @ts-expect-error above lazy.
| m.route.lazy | ||
| ? loadLazyRoute(m.route, manifest, mapRouteProperties) | ||
| : undefined |
There was a problem hiding this comment.
The check for m.route.lazy was redundant since loadLazyRoute also performs this check internally. Removing this ternary made managing the types a lot simper since loadLazyRoute now returns an object of promises rather than a single promise.
| async function callLoaderOrAction({ | ||
| type, | ||
| request, | ||
| match, | ||
| lazyHandlerPromise, | ||
| lazyRoutePromise, | ||
| handlerOverride, | ||
| scopedContext, | ||
| }: { | ||
| type: "loader" | "action"; | ||
| request: Request; | ||
| match: AgnosticDataRouteMatch; | ||
| lazyHandlerPromise: Promise<void> | undefined; | ||
| lazyRoutePromise: Promise<void> | undefined; | ||
| handlerOverride: Parameters<DataStrategyMatch["resolve"]>[0]; | ||
| scopedContext: unknown; | ||
| }): Promise<DataStrategyResult> { |
There was a problem hiding this comment.
I swapped positional args out for an object here because we now have two args the same type (lazyHandlerPromise and lazyRoutePromise), so I wanted to be extra sure that we don't accidentally get the order wrong.
| // contains all logic for managing the lazy route. This chained promise is | ||
| // then awaited so that consumers of this function see the updated route. | ||
| let lazyRoutePromise = route.lazy().then((lazyRoute) => { | ||
| let lazyRoutePromise = (async () => { |
There was a problem hiding this comment.
Since loadLazyRoute is no longer an async function, and is instead a sync function that returns an object with multiple promises, we need to wrap the route.lazy() call within an async IIFE so that errors are handled correctly.
| lazyRoutePromise, | ||
| lazyHandlerPromise, |
There was a problem hiding this comment.
Not necessarily for this PR but should we be trying to cache/dedupe these as well?
brophdawg11
deleted the
markdalgleish/granular-route-lazy-loaders-actions
branch
|
🤖 Hello there, We just published version Thanks! |
|
🤖 Hello there, We just published version Thanks! |
2 participants
Now that we have granular
route.lazy.loaderandroute.lazy.actionfunctions, we should only block on the required function before calling the handler rather than waiting on the entire route module to load.TODO:
loadLazyRouteshould accept thetypearg ("loader" | "action")and return the specific handler promise rather than all property promises since the rest aren't used