Skip to content

Commit 5134aa2

Browse files
refactor(router-core): shouldExecuteBeforeLoad is always true (#4992)
Fix #4971 (comment) With the code now simplified by previous PRs, we noticed that the `shouldExecuteBeforeLoad` guard is always true, so we can clean it up. In a follow up PR, we'll revise the conditions under which a `beforeLoad` should be called or skipped. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Refactor** * Streamlined pre-load checks and execution flow, reducing branching and improving consistency across client and server rendering. * Unified orchestration of the before-load phase to simplify control flow and improve maintainability. * **Bug Fixes** * More reliable handling of redirects and “not found” states during preloading, preventing unintended execution paths and improving navigation stability. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
1 parent a7504ae commit 5134aa2

File tree

1 file changed

+12
-29
lines changed

1 file changed

+12
-29
lines changed

packages/router-core/src/load-matches.ts

Lines changed: 12 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -147,11 +147,10 @@ const shouldSkipLoader = (
147147
return true
148148
}
149149

150-
if (inner.router.isServer) {
151-
if (match.ssr === false) {
152-
return true
153-
}
150+
if (inner.router.isServer && match.ssr === false) {
151+
return true
154152
}
153+
155154
return false
156155
}
157156

@@ -302,11 +301,11 @@ const setupPendingTimeout = (
302301
}
303302
}
304303

305-
const shouldExecuteBeforeLoad = (
304+
const preBeforeLoadSetup = (
306305
inner: InnerLoadContext,
307306
matchId: string,
308307
route: AnyRoute,
309-
): boolean | Promise<boolean> => {
308+
): void | Promise<void> => {
310309
const existingMatch = inner.router.getMatch(matchId)!
311310

312311
// If we are in the middle of a load, either of these will be present
@@ -315,25 +314,21 @@ const shouldExecuteBeforeLoad = (
315314
!existingMatch._nonReactive.beforeLoadPromise &&
316315
!existingMatch._nonReactive.loaderPromise
317316
)
318-
return true
317+
return
319318

320319
setupPendingTimeout(inner, matchId, route, existingMatch)
321320

322321
const then = () => {
323-
let result = true
324322
const match = inner.router.getMatch(matchId)!
325-
if (match.status === 'error') {
326-
result = true
327-
} else if (
323+
if (
328324
match.preload &&
329325
(match.status === 'redirected' || match.status === 'notFound')
330326
) {
331327
handleRedirectAndNotFound(inner, match, match.error)
332328
}
333-
return result
334329
}
335330

336-
// Wait for the beforeLoad to resolve before we continue
331+
// Wait for the previous beforeLoad to resolve before we continue
337332
return existingMatch._nonReactive.beforeLoadPromise
338333
? existingMatch._nonReactive.beforeLoadPromise.then(then)
339334
: then()
@@ -494,24 +489,12 @@ const handleBeforeLoad = (
494489

495490
const queueExecution = () => {
496491
if (shouldSkipLoader(inner, matchId)) return
497-
const shouldExecuteBeforeLoadResult = shouldExecuteBeforeLoad(
498-
inner,
499-
matchId,
500-
route,
501-
)
502-
return isPromise(shouldExecuteBeforeLoadResult)
503-
? shouldExecuteBeforeLoadResult.then(execute)
504-
: execute(shouldExecuteBeforeLoadResult)
505-
}
506-
507-
const execute = (shouldExec: boolean) => {
508-
if (shouldExec) {
509-
// If we are not in the middle of a load OR the previous load failed, start it
510-
return executeBeforeLoad(inner, matchId, index, route)
511-
}
512-
return
492+
const result = preBeforeLoadSetup(inner, matchId, route)
493+
return isPromise(result) ? result.then(execute) : execute()
513494
}
514495

496+
const execute = () => executeBeforeLoad(inner, matchId, index, route)
497+
515498
return serverSsr()
516499
}
517500

0 commit comments

Comments
 (0)