Skip to content

Commit 7bf23b4

Browse files
schiller-manuelschiller-manuel
authored
fix: head execution (#4189)
1 parent 13996c6 commit 7bf23b4

File tree

4 files changed

+158
-66
lines changed

4 files changed

+158
-66
lines changed

‎packages/react-router/tests/Scripts.test.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ describe('ssr HeadContent', () => {
123123
},
124124
{
125125
name: 'description',
126-
content: loaderData.description,
126+
content: loaderData?.description,
127127
},
128128
{
129129
name: 'image',
@@ -160,7 +160,7 @@ describe('ssr HeadContent', () => {
160160
},
161161
{
162162
name: 'description',
163-
content: loaderData.description,
163+
content: loaderData?.description,
164164
},
165165
{
166166
name: 'last-modified',

‎packages/react-router/tests/route.test.tsx

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
createRoute,
99
createRouter,
1010
getRouteApi,
11+
notFound,
1112
} from '../src'
1213

1314
afterEach(() => {
@@ -272,6 +273,85 @@ describe('route.head', () => {
272273
])
273274
})
274275

276+
test('meta is set when loader throws notFound', async () => {
277+
const rootRoute = createRootRoute({
278+
head: () => ({
279+
meta: [
280+
{ title: 'Root' },
281+
{
282+
charSet: 'utf-8',
283+
},
284+
],
285+
}),
286+
})
287+
const indexRoute = createRoute({
288+
getParentRoute: () => rootRoute,
289+
path: '/',
290+
head: () => ({
291+
meta: [{ title: 'Index' }],
292+
}),
293+
loader: async () => {
294+
throw notFound()
295+
},
296+
component: () => <div>Index</div>,
297+
})
298+
const routeTree = rootRoute.addChildren([indexRoute])
299+
const router = createRouter({ routeTree })
300+
render(<RouterProvider router={router} />)
301+
expect(await screen.findByText('Not Found')).toBeInTheDocument()
302+
303+
const metaState = router.state.matches.map((m) => m.meta)
304+
expect(metaState).toEqual([
305+
[
306+
{ title: 'Root' },
307+
{
308+
charSet: 'utf-8',
309+
},
310+
],
311+
[{ title: 'Index' }],
312+
])
313+
})
314+
315+
test('meta is set when loader throws an error', async () => {
316+
const rootRoute = createRootRoute({
317+
head: () => ({
318+
meta: [
319+
{ title: 'Root' },
320+
{
321+
charSet: 'utf-8',
322+
},
323+
],
324+
}),
325+
})
326+
const indexRoute = createRoute({
327+
getParentRoute: () => rootRoute,
328+
path: '/',
329+
head: () => ({
330+
meta: [{ title: 'Index' }],
331+
}),
332+
loader: async () => {
333+
throw new Error('Fly, you fools!')
334+
},
335+
component: () => <div>Index</div>,
336+
})
337+
const routeTree = rootRoute.addChildren([indexRoute])
338+
const router = createRouter({ routeTree })
339+
render(<RouterProvider router={router} />)
340+
341+
expect(await screen.findByText('Fly, you fools!')).toBeInTheDocument()
342+
343+
const metaState = router.state.matches.map((m) => m.meta)
344+
expect(metaState).toEqual([
345+
[
346+
{ title: 'Root' },
347+
{
348+
charSet: 'utf-8',
349+
},
350+
],
351+
[{ title: 'Index' }],
352+
])
353+
})
354+
275355
test('scripts', async () => {
276356
const rootRoute = createRootRoute({
277357
head: () => ({

‎packages/router-core/src/route.ts

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -960,7 +960,7 @@ type AssetFnContextOptions<
960960
TLoaderDeps
961961
>
962962
params: ResolveAllParamsFromParent<TParentRoute, TParams>
963-
loaderData: ResolveLoaderData<TLoaderFn>
963+
loaderData?: ResolveLoaderData<TLoaderFn>
964964
}
965965

966966
export interface DefaultUpdatableRouteOptionsExtensions {
@@ -1070,9 +1070,20 @@ export interface UpdatableRouteOptions<
10701070
TLoaderDeps
10711071
>,
10721072
) => void
1073-
headers?: (ctx: {
1074-
loaderData: ResolveLoaderData<TLoaderFn>
1075-
}) => Record<string, string>
1073+
headers?: (
1074+
ctx: AssetFnContextOptions<
1075+
TRouteId,
1076+
TFullPath,
1077+
TParentRoute,
1078+
TParams,
1079+
TSearchValidator,
1080+
TLoaderFn,
1081+
TRouterContext,
1082+
TRouteContextFn,
1083+
TBeforeLoadFn,
1084+
TLoaderDeps
1085+
>,
1086+
) => Record<string, string>
10761087
head?: (
10771088
ctx: AssetFnContextOptions<
10781089
TRouteId,

‎packages/router-core/src/router.ts

Lines changed: 61 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -1447,26 +1447,6 @@ export class RouterCore<
14471447
...match.__beforeLoadContext,
14481448
}
14491449
}
1450-
1451-
// If it's already a success, update headers and head content
1452-
// These may get updated again if the match is refreshed
1453-
// due to being stale
1454-
if (match.status === 'success') {
1455-
match.headers = route.options.headers?.({
1456-
loaderData: match.loaderData,
1457-
})
1458-
const assetContext = {
1459-
matches,
1460-
match,
1461-
params: match.params,
1462-
loaderData: match.loaderData,
1463-
}
1464-
const headFnContent = route.options.head?.(assetContext)
1465-
match.links = headFnContent?.links
1466-
match.headScripts = headFnContent?.scripts
1467-
match.meta = headFnContent?.meta
1468-
match.scripts = route.options.scripts?.(assetContext)
1469-
}
14701450
})
14711451

14721452
return matches
@@ -2609,6 +2589,35 @@ export class RouterCore<
26092589
!this.state.matches.find((d) => d.id === matchId),
26102590
}))
26112591

2592+
const executeHead = () => {
2593+
const match = this.getMatch(matchId)
2594+
// in case of a redirecting match during preload, the match does not exist
2595+
if (!match) {
2596+
return
2597+
}
2598+
const assetContext = {
2599+
matches,
2600+
match,
2601+
params: match.params,
2602+
loaderData: match.loaderData,
2603+
}
2604+
const headFnContent = route.options.head?.(assetContext)
2605+
const meta = headFnContent?.meta
2606+
const links = headFnContent?.links
2607+
const headScripts = headFnContent?.scripts
2608+
2609+
const scripts = route.options.scripts?.(assetContext)
2610+
const headers = route.options.headers?.(assetContext)
2611+
updateMatch(matchId, (prev) => ({
2612+
...prev,
2613+
meta,
2614+
links,
2615+
headScripts,
2616+
headers,
2617+
scripts,
2618+
}))
2619+
}
2620+
26122621
const runLoader = async () => {
26132622
try {
26142623
// If the Matches component rendered
@@ -2649,40 +2658,21 @@ export class RouterCore<
26492658

26502659
await potentialPendingMinPromise()
26512660

2652-
const assetContext = {
2653-
matches,
2654-
match: this.getMatch(matchId)!,
2655-
params: this.getMatch(matchId)!.params,
2656-
loaderData,
2657-
}
2658-
const headFnContent =
2659-
route.options.head?.(assetContext)
2660-
const meta = headFnContent?.meta
2661-
const links = headFnContent?.links
2662-
const headScripts = headFnContent?.scripts
2663-
2664-
const scripts = route.options.scripts?.(assetContext)
2665-
const headers = route.options.headers?.({
2666-
loaderData,
2667-
})
2668-
26692661
// Last but not least, wait for the the components
26702662
// to be preloaded before we resolve the match
26712663
await route._componentsPromise
26722664

2673-
updateMatch(matchId, (prev) => ({
2674-
...prev,
2675-
error: undefined,
2676-
status: 'success',
2677-
isFetching: false,
2678-
updatedAt: Date.now(),
2679-
loaderData,
2680-
meta,
2681-
links,
2682-
headScripts,
2683-
headers,
2684-
scripts,
2685-
}))
2665+
batch(() => {
2666+
updateMatch(matchId, (prev) => ({
2667+
...prev,
2668+
error: undefined,
2669+
status: 'success',
2670+
isFetching: false,
2671+
updatedAt: Date.now(),
2672+
loaderData,
2673+
}))
2674+
executeHead()
2675+
})
26862676
} catch (e) {
26872677
let error = e
26882678

@@ -2700,23 +2690,29 @@ export class RouterCore<
27002690
)
27012691
}
27022692

2703-
updateMatch(matchId, (prev) => ({
2704-
...prev,
2705-
error,
2706-
status: 'error',
2707-
isFetching: false,
2708-
}))
2693+
batch(() => {
2694+
updateMatch(matchId, (prev) => ({
2695+
...prev,
2696+
error,
2697+
status: 'error',
2698+
isFetching: false,
2699+
}))
2700+
executeHead()
2701+
})
27092702
}
27102703

27112704
this.serverSsr?.onMatchSettled({
27122705
router: this,
27132706
match: this.getMatch(matchId)!,
27142707
})
27152708
} catch (err) {
2716-
updateMatch(matchId, (prev) => ({
2717-
...prev,
2718-
loaderPromise: undefined,
2719-
}))
2709+
batch(() => {
2710+
updateMatch(matchId, (prev) => ({
2711+
...prev,
2712+
loaderPromise: undefined,
2713+
}))
2714+
executeHead()
2715+
})
27202716
handleRedirectAndNotFound(this.getMatch(matchId)!, err)
27212717
}
27222718
}
@@ -2752,6 +2748,11 @@ export class RouterCore<
27522748
(loaderShouldRunAsync && sync)
27532749
) {
27542750
await runLoader()
2751+
} else {
2752+
// if the loader did not run, still update head.
2753+
// reason: parent's beforeLoad may have changed the route context
2754+
// and only now do we know the route context (and that the loader would not run)
2755+
executeHead()
27552756
}
27562757
}
27572758
if (!loaderIsRunningAsync) {

0 commit comments

Comments
 (0)