Skip to content

Commit 7880ad7

Browse files
birkskyumbirkskyum
authored
fix(solid-start): navigation transitions (#5792)
1 parent 2e0959c commit 7880ad7

File tree

6 files changed

+143
-25
lines changed

6 files changed

+143
-25
lines changed

‎e2e/solid-start/basic/tests/transition.spec.ts‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
import { expect, test } from '@playwright/test'
2+
3+
test('transitions/count/create-resource should keep old values visible during navigation', async ({
4+
page,
5+
}) => {
6+
await page.goto('/transition/count/create-resource')
7+
8+
await expect(page.getByTestId('n-value')).toContainText('n: 1')
9+
await expect(page.getByTestId('double-value')).toContainText('double: 2')
10+
11+
const bodyTexts: Array<string> = []
12+
13+
const pollInterval = setInterval(async () => {
14+
const text = await page
15+
.locator('body')
16+
.textContent()
17+
.catch(() => '')
18+
if (text) bodyTexts.push(text)
19+
}, 50)
20+
21+
// 1 click
22+
23+
page.getByTestId('increase-button').click()
24+
25+
await expect(page.getByTestId('n-value')).toContainText('n: 1', {
26+
timeout: 2_000,
27+
})
28+
await expect(page.getByTestId('double-value')).toContainText('double: 2', {
29+
timeout: 2_000,
30+
})
31+
32+
await page.waitForTimeout(200)
33+
34+
await expect(page.getByTestId('n-value')).toContainText('n: 2', {
35+
timeout: 2000,
36+
})
37+
await expect(page.getByTestId('double-value')).toContainText('double: 4', {
38+
timeout: 2000,
39+
})
40+
41+
// 2 clicks
42+
43+
page.getByTestId('increase-button').click()
44+
page.getByTestId('increase-button').click()
45+
46+
await expect(page.getByTestId('n-value')).toContainText('n: 2', {
47+
timeout: 2000,
48+
})
49+
await expect(page.getByTestId('double-value')).toContainText('double: 4', {
50+
timeout: 2000,
51+
})
52+
53+
await page.waitForTimeout(200)
54+
55+
await expect(page.getByTestId('n-value')).toContainText('n: 4', {
56+
timeout: 2000,
57+
})
58+
await expect(page.getByTestId('double-value')).toContainText('double: 8', {
59+
timeout: 2000,
60+
})
61+
62+
// 3 clicks
63+
64+
page.getByTestId('increase-button').click()
65+
page.getByTestId('increase-button').click()
66+
page.getByTestId('increase-button').click()
67+
68+
await expect(page.getByTestId('n-value')).toContainText('n: 4', {
69+
timeout: 2000,
70+
})
71+
await expect(page.getByTestId('double-value')).toContainText('double: 8', {
72+
timeout: 2000,
73+
})
74+
75+
await page.waitForTimeout(200)
76+
77+
await expect(page.getByTestId('n-value')).toContainText('n: 7', {
78+
timeout: 2000,
79+
})
80+
await expect(page.getByTestId('double-value')).toContainText('double: 14', {
81+
timeout: 2000,
82+
})
83+
84+
clearInterval(pollInterval)
85+
86+
// With proper transitions, old values should remain visible until new ones arrive
87+
const hasLoadingText = bodyTexts.some((text) => text.includes('Loading...'))
88+
89+
if (hasLoadingText) {
90+
throw new Error(
91+
'FAILED: "Loading..." appeared during navigation. ' +
92+
'Solid Router should use transitions to keep old values visible.',
93+
)
94+
}
95+
})

‎packages/react-router/tests/store-updates-during-navigation.test.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ describe("Store doesn't update *too many* times during navigation", () => {
223223
// This number should be as small as possible to minimize the amount of work
224224
// that needs to be done during a navigation.
225225
// Any change that increases this number should be investigated.
226-
expect(updates).toBe(14)
226+
expect(updates).toBe(16)
227227
})
228228

229229
test('navigate, w/ preloaded & async loaders', async () => {

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,8 @@ export interface RouteMatch<
148148
displayPendingPromise?: Promise<void>
149149
minPendingPromise?: ControlledPromise<void>
150150
dehydrated?: boolean
151+
/** @internal */
152+
error?: unknown
151153
}
152154
loaderData?: TLoaderData
153155
/** @internal */

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

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,8 @@ const handleRedirectAndNotFound = (
111111

112112
const status = isRedirect(err) ? 'redirected' : 'notFound'
113113

114+
match._nonReactive.error = err
115+
114116
inner.updateMatch(match.id, (prev) => ({
115117
...prev,
116118
status,
@@ -560,9 +562,23 @@ const getLoaderContext = (
560562
route: AnyRoute,
561563
): LoaderFnContext => {
562564
const parentMatchPromise = inner.matchPromises[index - 1] as any
563-
const { params, loaderDeps, abortController, context, cause } =
565+
const { params, loaderDeps, abortController, cause } =
564566
inner.router.getMatch(matchId)!
565567

568+
let context = inner.router.options.context ?? {}
569+
570+
for (let i = 0; i <= index; i++) {
571+
const innerMatch = inner.matches[i]
572+
if (!innerMatch) continue
573+
const m = inner.router.getMatch(innerMatch.id)
574+
if (!m) continue
575+
context = {
576+
...context,
577+
...(m.__routeContext ?? {}),
578+
...(m.__beforeLoadContext ?? {}),
579+
}
580+
}
581+
566582
const preload = resolvePreload(inner, matchId)
567583

568584
return {
@@ -750,8 +766,9 @@ const loadRouteMatch = async (
750766
}
751767
await prevMatch._nonReactive.loaderPromise
752768
const match = inner.router.getMatch(matchId)!
753-
if (match.error) {
754-
handleRedirectAndNotFound(inner, match, match.error)
769+
const error = match._nonReactive.error || match.error
770+
if (error) {
771+
handleRedirectAndNotFound(inner, match, error)
755772
}
756773
} else {
757774
// This is where all of the stale-while-revalidate magic happens

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

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2295,23 +2295,27 @@ export class RouterCore<
22952295
}
22962296

22972297
updateMatch: UpdateMatchFn = (id, updater) => {
2298-
const matchesKey = this.state.pendingMatches?.some((d) => d.id === id)
2299-
? 'pendingMatches'
2300-
: this.state.matches.some((d) => d.id === id)
2301-
? 'matches'
2302-
: this.state.cachedMatches.some((d) => d.id === id)
2303-
? 'cachedMatches'
2304-
: ''
2305-
2306-
if (matchesKey) {
2307-
this.__store.setState((s) => ({
2308-
...s,
2309-
[matchesKey]: s[matchesKey]?.map((d) => (d.id === id ? updater(d) : d)),
2310-
}))
2311-
}
2298+
this.startTransition(() => {
2299+
const matchesKey = this.state.pendingMatches?.some((d) => d.id === id)
2300+
? 'pendingMatches'
2301+
: this.state.matches.some((d) => d.id === id)
2302+
? 'matches'
2303+
: this.state.cachedMatches.some((d) => d.id === id)
2304+
? 'cachedMatches'
2305+
: ''
2306+
2307+
if (matchesKey) {
2308+
this.__store.setState((s) => ({
2309+
...s,
2310+
[matchesKey]: s[matchesKey]?.map((d) =>
2311+
d.id === id ? updater(d) : d,
2312+
),
2313+
}))
2314+
}
2315+
})
23122316
}
23132317

2314-
getMatch: GetMatchFn = (matchId: string) => {
2318+
getMatch: GetMatchFn = (matchId: string): AnyRouteMatch | undefined => {
23152319
const findFn = (d: { id: string }) => d.id === matchId
23162320
return (
23172321
this.state.cachedMatches.find(findFn) ??

‎packages/solid-router/tests/store-updates-during-navigation.test.tsx‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ describe("Store doesn't update *too many* times during navigation", () => {
156156
// that needs to be done during a navigation.
157157
// Any change that increases this number should be investigated.
158158
// Note: Solid has different update counts than React due to different reactivity
159-
expect(updates).toBe(5)
159+
expect(updates).toBe(6)
160160
})
161161

162162
test('sync beforeLoad', async () => {
@@ -173,7 +173,7 @@ describe("Store doesn't update *too many* times during navigation", () => {
173173
// that needs to be done during a navigation.
174174
// Any change that increases this number should be investigated.
175175
// Note: Solid has different update counts than React due to different reactivity
176-
expect(updates).toBe(9)
176+
expect(updates).toBe(8)
177177
})
178178

179179
test('nothing', async () => {
@@ -226,7 +226,7 @@ describe("Store doesn't update *too many* times during navigation", () => {
226226
// This number should be as small as possible to minimize the amount of work
227227
// that needs to be done during a navigation.
228228
// Any change that increases this number should be investigated.
229-
expect(updates).toBe(14)
229+
expect(updates).toBe(16)
230230
})
231231

232232
test('navigate, w/ preloaded & async loaders', async () => {
@@ -242,7 +242,7 @@ describe("Store doesn't update *too many* times during navigation", () => {
242242
// This number should be as small as possible to minimize the amount of work
243243
// that needs to be done during a navigation.
244244
// Any change that increases this number should be investigated.
245-
expect(updates).toBe(8)
245+
expect(updates).toBe(9)
246246
})
247247

248248
test('navigate, w/ preloaded & sync loaders', async () => {
@@ -259,7 +259,7 @@ describe("Store doesn't update *too many* times during navigation", () => {
259259
// that needs to be done during a navigation.
260260
// Any change that increases this number should be investigated.
261261
// Note: Solid has one fewer update than React due to different reactivity
262-
expect(updates).toBe(6)
262+
expect(updates).toBe(7)
263263
})
264264

265265
test('navigate, w/ previous navigation & async loader', async () => {
@@ -293,6 +293,6 @@ describe("Store doesn't update *too many* times during navigation", () => {
293293
// This number should be as small as possible to minimize the amount of work
294294
// that needs to be done during a navigation.
295295
// Any change that increases this number should be investigated.
296-
expect(updates).toBe(1)
296+
expect(updates).toBe(2)
297297
})
298298
})

0 commit comments

Comments
 (0)