Skip to content

Commit 9af59c7

Browse files
birkskyumbirkskyum
committed
fix(solid-start): navigation transitions
1 parent 01682c5 commit 9af59c7

File tree

6 files changed

+146
-25
lines changed

6 files changed

+146
-25
lines changed

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

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
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+
// TODO - the below tests should be possible to uncomment
42+
43+
// 2 clicks
44+
45+
page.getByTestId('increase-button').click()
46+
page.getByTestId('increase-button').click()
47+
48+
await expect(page.getByTestId('n-value')).toContainText('n: 2', {
49+
timeout: 2000,
50+
})
51+
await expect(page.getByTestId('double-value')).toContainText('double: 4', {
52+
timeout: 2000,
53+
})
54+
55+
await page.waitForTimeout(200)
56+
57+
await expect(page.getByTestId('n-value')).toContainText('n: 4', {
58+
timeout: 2000,
59+
})
60+
await expect(page.getByTestId('double-value')).toContainText('double: 8', {
61+
timeout: 2000,
62+
})
63+
64+
// 3 clicks
65+
66+
page.getByTestId('increase-button').click()
67+
page.getByTestId('increase-button').click()
68+
page.getByTestId('increase-button').click()
69+
70+
await expect(page.getByTestId('n-value')).toContainText('n: 4', {
71+
timeout: 2000,
72+
})
73+
await expect(page.getByTestId('double-value')).toContainText('double: 8', {
74+
timeout: 2000,
75+
})
76+
77+
await page.waitForTimeout(200)
78+
79+
await expect(page.getByTestId('n-value')).toContainText('n: 7', {
80+
timeout: 2000,
81+
})
82+
await expect(page.getByTestId('double-value')).toContainText('double: 14', {
83+
timeout: 2000,
84+
})
85+
86+
clearInterval(pollInterval)
87+
88+
// With proper transitions, old values should remain visible until new ones arrive
89+
const hasLoadingText = bodyTexts.some((text) => text.includes('Loading...'))
90+
91+
if (hasLoadingText) {
92+
throw new Error(
93+
'FAILED: "Loading..." appeared during navigation. ' +
94+
'Solid Router should use transitions to keep old values visible.',
95+
)
96+
}
97+
})

‎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: 21 additions & 4 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,8 +562,22 @@ const getLoaderContext = (
560562
route: AnyRoute,
561563
): LoaderFnContext => {
562564
const parentMatchPromise = inner.matchPromises[index - 1] as any
563-
const { params, loaderDeps, abortController, context, cause } =
564-
inner.router.getMatch(matchId)!
565+
const match = inner.router.getMatch(matchId)!
566+
const { params, loaderDeps, abortController, cause } = match
567+
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 || !m._nonReactive) continue
575+
context = {
576+
...context,
577+
...(m.__routeContext ?? {}),
578+
...(m.__beforeLoadContext ?? {}),
579+
}
580+
}
565581

566582
const preload = resolvePreload(inner, matchId)
567583

@@ -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 & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1454,6 +1454,7 @@ export class RouterCore<
14541454
__routeContext: undefined,
14551455
_nonReactive: {
14561456
loadPromise: createControlledPromise(),
1457+
error: (existingMatch as any)?._nonReactive?.error,
14571458
},
14581459
__beforeLoadContext: undefined,
14591460
context: {},
@@ -2295,20 +2296,24 @@ export class RouterCore<
22952296
}
22962297

22972298
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-
}
2299+
this.startTransition(() => {
2300+
const matchesKey = this.state.pendingMatches?.some((d) => d.id === id)
2301+
? 'pendingMatches'
2302+
: this.state.matches.some((d) => d.id === id)
2303+
? 'matches'
2304+
: this.state.cachedMatches.some((d) => d.id === id)
2305+
? 'cachedMatches'
2306+
: ''
2307+
2308+
if (matchesKey) {
2309+
this.__store.setState((s) => ({
2310+
...s,
2311+
[matchesKey]: s[matchesKey]?.map((d) =>
2312+
d.id === id ? updater(d) : d,
2313+
),
2314+
}))
2315+
}
2316+
})
23122317
}
23132318

23142319
getMatch: GetMatchFn = (matchId: string) => {

‎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)