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.

Sign up for GitHub

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

Fix typegen for routes with a client loader but no server loader #12117

Merged
merged 3 commits into from
Oct 11, 2024
Merged

Fix typegen for routes with a client loader but no server loader #12117

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8ea4f63
Fix typegen for routes with a client loader but no server loader
brophdawg11 Oct 11, 2024
325d660
Clean up warning
brophdawg11 Oct 11, 2024
e6b7aad
Assert build warning during e2e test
brophdawg11 Oct 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/little-cooks-pull.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"react-router": patch
---

Fix typegen for routes with a client loader but no server loader
11 changes: 6 additions & 5 deletions packages/react-router-dev/vite/plugin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1897,12 +1897,13 @@ function determineStaticPrerenderRoutes(
}
recurse(routes);

if (isBooleanUsage && paramRoutes) {
if (isBooleanUsage && paramRoutes.length > 0) {
viteConfig.logger.warn(
"The following paths were not prerendered because Dynamic Param and Splat " +
"routes cannot be prerendered when using `prerender:true`. You may want to " +
"consider using the `prerender()` API if you wish to prerender slug and " +
"splat routes."
[
"⚠️ Paths with dynamic/splat params cannot be prerendered when using `prerender: true`.",
"You may want to use the `prerender()` API to prerender the following paths:",
...paramRoutes.map((p) => " - " + p),
].join("\n")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops - this was just a miss when this warning got added - I noticed doing some testing yesterday that it prints unnecessarily because paramRoutes is always defined and it also doesn't actually log the routes it's referring to 🤦‍♂️

);
}

Expand Down
9 changes: 3 additions & 6 deletions packages/react-router/lib/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,15 +66,12 @@ type _CreateLoaderData<
ClientLoaderHydrate extends boolean,
HasHydrateFallback
> =
[HasHydrateFallback, ClientLoaderHydrate] extends [true, true] ?
[HasHydrateFallback, ClientLoaderHydrate] extends [true, true] ?
IsDefined<ClientLoaderData> extends true ? ClientLoaderData :
undefined
:
[IsDefined<ClientLoaderData>, IsDefined<ServerLoaderData>] extends [true, true] ? ServerLoaderData | ClientLoaderData :
IsDefined<ClientLoaderData> extends true ?
ClientLoaderHydrate extends true ? ClientLoaderData :
ClientLoaderData | undefined
:
IsDefined<ClientLoaderData> extends true ? ClientLoaderData :
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If a route has a client loader but no server loader, then clientLoader.hydrate is implied and we will always call the loader before rendering the component, so no need for this fork here to include undefined

Copy link
Contributor

Choose a reason for hiding this comment

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

Am I understanding this right that the absence of a loader and the presence of a clientLoader means that the route is automatically CSR only?

I don't think I ever realized that 😅

IsDefined<ServerLoaderData> extends true ? ServerLoaderData :
undefined

Expand Down Expand Up @@ -221,7 +218,7 @@ type __tests = [
CreateLoaderData<{
clientLoader: () => { a: string; b: Date; c: () => boolean };
}>,
undefined | { a: string; b: Date; c: () => boolean }
{ a: string; b: Date; c: () => boolean }
>
>,
Expect<
Expand Down
Loading