diff --git a/e2e/solid-start/basic/tests/transition.spec.ts b/e2e/solid-start/basic/tests/transition.spec.ts new file mode 100644 index 00000000000..b777c528626 --- /dev/null +++ b/e2e/solid-start/basic/tests/transition.spec.ts @@ -0,0 +1,95 @@ +import { expect, test } from '@playwright/test' + +test('transitions/count/create-resource should keep old values visible during navigation', async ({ + page, +}) => { + await page.goto('/transition/count/create-resource') + + await expect(page.getByTestId('n-value')).toContainText('n: 1') + await expect(page.getByTestId('double-value')).toContainText('double: 2') + + const bodyTexts: Array = [] + + const pollInterval = setInterval(async () => { + const text = await page + .locator('body') + .textContent() + .catch(() => '') + if (text) bodyTexts.push(text) + }, 50) + + // 1 click + + page.getByTestId('increase-button').click() + + await expect(page.getByTestId('n-value')).toContainText('n: 1', { + timeout: 2_000, + }) + await expect(page.getByTestId('double-value')).toContainText('double: 2', { + timeout: 2_000, + }) + + await page.waitForTimeout(200) + + await expect(page.getByTestId('n-value')).toContainText('n: 2', { + timeout: 2000, + }) + await expect(page.getByTestId('double-value')).toContainText('double: 4', { + timeout: 2000, + }) + + // 2 clicks + + page.getByTestId('increase-button').click() + page.getByTestId('increase-button').click() + + await expect(page.getByTestId('n-value')).toContainText('n: 2', { + timeout: 2000, + }) + await expect(page.getByTestId('double-value')).toContainText('double: 4', { + timeout: 2000, + }) + + await page.waitForTimeout(200) + + await expect(page.getByTestId('n-value')).toContainText('n: 4', { + timeout: 2000, + }) + await expect(page.getByTestId('double-value')).toContainText('double: 8', { + timeout: 2000, + }) + + // 3 clicks + + page.getByTestId('increase-button').click() + page.getByTestId('increase-button').click() + page.getByTestId('increase-button').click() + + await expect(page.getByTestId('n-value')).toContainText('n: 4', { + timeout: 2000, + }) + await expect(page.getByTestId('double-value')).toContainText('double: 8', { + timeout: 2000, + }) + + await page.waitForTimeout(200) + + await expect(page.getByTestId('n-value')).toContainText('n: 7', { + timeout: 2000, + }) + await expect(page.getByTestId('double-value')).toContainText('double: 14', { + timeout: 2000, + }) + + clearInterval(pollInterval) + + // With proper transitions, old values should remain visible until new ones arrive + const hasLoadingText = bodyTexts.some((text) => text.includes('Loading...')) + + if (hasLoadingText) { + throw new Error( + 'FAILED: "Loading..." appeared during navigation. ' + + 'Solid Router should use transitions to keep old values visible.', + ) + } +}) diff --git a/packages/react-router/tests/store-updates-during-navigation.test.tsx b/packages/react-router/tests/store-updates-during-navigation.test.tsx index a7ccb4f4dfd..fcbc841bddf 100644 --- a/packages/react-router/tests/store-updates-during-navigation.test.tsx +++ b/packages/react-router/tests/store-updates-during-navigation.test.tsx @@ -223,7 +223,7 @@ describe("Store doesn't update *too many* times during navigation", () => { // This number should be as small as possible to minimize the amount of work // that needs to be done during a navigation. // Any change that increases this number should be investigated. - expect(updates).toBe(14) + expect(updates).toBe(16) }) test('navigate, w/ preloaded & async loaders', async () => { diff --git a/packages/router-core/src/Matches.ts b/packages/router-core/src/Matches.ts index d5e8be52a35..3b39b8eee7b 100644 --- a/packages/router-core/src/Matches.ts +++ b/packages/router-core/src/Matches.ts @@ -148,6 +148,8 @@ export interface RouteMatch< displayPendingPromise?: Promise minPendingPromise?: ControlledPromise dehydrated?: boolean + /** @internal */ + error?: unknown } loaderData?: TLoaderData /** @internal */ diff --git a/packages/router-core/src/load-matches.ts b/packages/router-core/src/load-matches.ts index 0ea5068ad0f..d0e22d6c9a0 100644 --- a/packages/router-core/src/load-matches.ts +++ b/packages/router-core/src/load-matches.ts @@ -111,6 +111,8 @@ const handleRedirectAndNotFound = ( const status = isRedirect(err) ? 'redirected' : 'notFound' + match._nonReactive.error = err + inner.updateMatch(match.id, (prev) => ({ ...prev, status, @@ -560,9 +562,23 @@ const getLoaderContext = ( route: AnyRoute, ): LoaderFnContext => { const parentMatchPromise = inner.matchPromises[index - 1] as any - const { params, loaderDeps, abortController, context, cause } = + const { params, loaderDeps, abortController, cause } = inner.router.getMatch(matchId)! + let context = inner.router.options.context ?? {} + + for (let i = 0; i <= index; i++) { + const innerMatch = inner.matches[i] + if (!innerMatch) continue + const m = inner.router.getMatch(innerMatch.id) + if (!m) continue + context = { + ...context, + ...(m.__routeContext ?? {}), + ...(m.__beforeLoadContext ?? {}), + } + } + const preload = resolvePreload(inner, matchId) return { @@ -750,8 +766,9 @@ const loadRouteMatch = async ( } await prevMatch._nonReactive.loaderPromise const match = inner.router.getMatch(matchId)! - if (match.error) { - handleRedirectAndNotFound(inner, match, match.error) + const error = match._nonReactive.error || match.error + if (error) { + handleRedirectAndNotFound(inner, match, error) } } else { // This is where all of the stale-while-revalidate magic happens diff --git a/packages/router-core/src/router.ts b/packages/router-core/src/router.ts index 994900543cc..a29a07a6889 100644 --- a/packages/router-core/src/router.ts +++ b/packages/router-core/src/router.ts @@ -2295,23 +2295,27 @@ export class RouterCore< } updateMatch: UpdateMatchFn = (id, updater) => { - const matchesKey = this.state.pendingMatches?.some((d) => d.id === id) - ? 'pendingMatches' - : this.state.matches.some((d) => d.id === id) - ? 'matches' - : this.state.cachedMatches.some((d) => d.id === id) - ? 'cachedMatches' - : '' - - if (matchesKey) { - this.__store.setState((s) => ({ - ...s, - [matchesKey]: s[matchesKey]?.map((d) => (d.id === id ? updater(d) : d)), - })) - } + this.startTransition(() => { + const matchesKey = this.state.pendingMatches?.some((d) => d.id === id) + ? 'pendingMatches' + : this.state.matches.some((d) => d.id === id) + ? 'matches' + : this.state.cachedMatches.some((d) => d.id === id) + ? 'cachedMatches' + : '' + + if (matchesKey) { + this.__store.setState((s) => ({ + ...s, + [matchesKey]: s[matchesKey]?.map((d) => + d.id === id ? updater(d) : d, + ), + })) + } + }) } - getMatch: GetMatchFn = (matchId: string) => { + getMatch: GetMatchFn = (matchId: string): AnyRouteMatch | undefined => { const findFn = (d: { id: string }) => d.id === matchId return ( this.state.cachedMatches.find(findFn) ?? diff --git a/packages/solid-router/tests/store-updates-during-navigation.test.tsx b/packages/solid-router/tests/store-updates-during-navigation.test.tsx index b9f2fa86c41..c69643e1c3d 100644 --- a/packages/solid-router/tests/store-updates-during-navigation.test.tsx +++ b/packages/solid-router/tests/store-updates-during-navigation.test.tsx @@ -156,7 +156,7 @@ describe("Store doesn't update *too many* times during navigation", () => { // that needs to be done during a navigation. // Any change that increases this number should be investigated. // Note: Solid has different update counts than React due to different reactivity - expect(updates).toBe(5) + expect(updates).toBe(6) }) test('sync beforeLoad', async () => { @@ -173,7 +173,7 @@ describe("Store doesn't update *too many* times during navigation", () => { // that needs to be done during a navigation. // Any change that increases this number should be investigated. // Note: Solid has different update counts than React due to different reactivity - expect(updates).toBe(9) + expect(updates).toBe(8) }) test('nothing', async () => { @@ -226,7 +226,7 @@ describe("Store doesn't update *too many* times during navigation", () => { // This number should be as small as possible to minimize the amount of work // that needs to be done during a navigation. // Any change that increases this number should be investigated. - expect(updates).toBe(14) + expect(updates).toBe(16) }) test('navigate, w/ preloaded & async loaders', async () => { @@ -242,7 +242,7 @@ describe("Store doesn't update *too many* times during navigation", () => { // This number should be as small as possible to minimize the amount of work // that needs to be done during a navigation. // Any change that increases this number should be investigated. - expect(updates).toBe(8) + expect(updates).toBe(9) }) test('navigate, w/ preloaded & sync loaders', async () => { @@ -259,7 +259,7 @@ describe("Store doesn't update *too many* times during navigation", () => { // that needs to be done during a navigation. // Any change that increases this number should be investigated. // Note: Solid has one fewer update than React due to different reactivity - expect(updates).toBe(6) + expect(updates).toBe(7) }) test('navigate, w/ previous navigation & async loader', async () => { @@ -293,6 +293,6 @@ describe("Store doesn't update *too many* times during navigation", () => { // This number should be as small as possible to minimize the amount of work // that needs to be done during a navigation. // Any change that increases this number should be investigated. - expect(updates).toBe(1) + expect(updates).toBe(2) }) })