-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
fix(solid-router): implement navigation transitions #5691
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 20 commits
58ff44b
3dcfe18
41e699d
d4e69ff
e1bfa43
5f2b2cb
4cc1cfc
b358a9f
60dda68
772ed17
13470aa
7a80426
5d51095
8333c6f
c5dc9f2
9d9cb33
a37c72d
860bfa7
3dbdc9f
e0364bb
52749f5
ca69d40
5ff886c
3064313
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| import { Link, createFileRoute } from '@tanstack/solid-router' | ||
| import { Suspense, createResource } from 'solid-js' | ||
| import { z } from 'zod' | ||
|
|
||
| export const Route = createFileRoute('/transition/')({ | ||
| validateSearch: z.object({ | ||
| n: z.number().default(1), | ||
| }), | ||
| component: Home, | ||
| }) | ||
|
|
||
| function Home() { | ||
| return ( | ||
| <div class="p-2"> | ||
| <Link | ||
| data-testid="increase-button" | ||
| class="border bg-gray-50 px-3 py-1" | ||
| from="/transition" | ||
| search={(s) => ({ n: s.n + 1 })} | ||
| > | ||
| Increase | ||
| </Link> | ||
|
|
||
| <Result /> | ||
| </div> | ||
| ) | ||
| } | ||
|
|
||
| function Result() { | ||
| const searchQuery = Route.useSearch() | ||
|
|
||
| const [doubleQuery] = createResource( | ||
| () => searchQuery().n, | ||
| async (n) => { | ||
| await new Promise((r) => setTimeout(r, 1000)) | ||
| return n * 2 | ||
| }, | ||
| ) | ||
|
|
||
| return ( | ||
| <div class="mt-2"> | ||
| <Suspense fallback="Loading..."> | ||
| <div data-testid="n-value">n: {searchQuery().n}</div> | ||
| <div data-testid="double-value">double: {doubleQuery()}</div> | ||
| </Suspense> | ||
| </div> | ||
| ) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| import { expect, test } from '@playwright/test' | ||
|
|
||
| test('transitions should keep old values visible during navigation', async ({ | ||
| page, | ||
| }) => { | ||
| await page.goto('/transition') | ||
|
|
||
| await expect(page.getByTestId('n-value')).toContainText('n: 1') | ||
| await expect(page.getByTestId('double-value')).toContainText('double: 2') | ||
|
|
||
| const bodyTexts: Array<string> = [] | ||
|
|
||
| const pollInterval = setInterval(async () => { | ||
| const text = await page | ||
| .locator('body') | ||
| .textContent() | ||
| .catch(() => '') | ||
| if (text) bodyTexts.push(text) | ||
| }, 50) | ||
|
|
||
| await page.getByTestId('increase-button').click() | ||
|
|
||
| await page.waitForTimeout(200) | ||
|
|
||
| clearInterval(pollInterval) | ||
|
|
||
| await expect(page.getByTestId('n-value')).toContainText('n: 2', { | ||
| timeout: 2000, | ||
| }) | ||
| await expect(page.getByTestId('double-value')).toContainText('double: 4', { | ||
| timeout: 2000, | ||
| }) | ||
|
|
||
| // 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.', | ||
| ) | ||
| } | ||
| }) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| import { Link, createFileRoute } from '@tanstack/solid-router' | ||
| import { Suspense, createMemo } from 'solid-js' | ||
| import { queryOptions, useQuery } from '@tanstack/solid-query' | ||
| import { z } from 'zod' | ||
|
|
||
| const searchSchema = z.object({ | ||
| n: z.number().default(1), | ||
| }) | ||
|
|
||
| const doubleQueryOptions = (n: number) => | ||
| queryOptions({ | ||
| queryKey: ['transition-double', n], | ||
| queryFn: async () => { | ||
| await new Promise((resolve) => setTimeout(resolve, 1000)) | ||
| return n * 2 | ||
| }, | ||
| placeholderData: (oldData) => oldData, | ||
| }) | ||
|
|
||
| export const Route = createFileRoute('/transition/')({ | ||
| validateSearch: searchSchema, | ||
| loader: ({ context: { queryClient }, location }) => { | ||
| const { n } = searchSchema.parse(location.search) | ||
| return queryClient.ensureQueryData(doubleQueryOptions(n)) | ||
| }, | ||
| component: TransitionPage, | ||
| }) | ||
|
|
||
| function TransitionPage() { | ||
| const search = Route.useSearch() | ||
|
|
||
| const doubleQuery = createMemo(() => | ||
| useQuery(() => doubleQueryOptions(search().n)), | ||
| ) | ||
|
|
||
| return ( | ||
| <Suspense fallback="Loading..."> | ||
| <div class="p-2"> | ||
| <Link | ||
| data-testid="increase-button" | ||
| class="border bg-gray-50 px-3 py-1" | ||
| from="/transition" | ||
| search={(s) => ({ n: s.n + 1 })} | ||
| > | ||
| Increase | ||
| </Link> | ||
|
|
||
| <div class="mt-2"> | ||
| <div data-testid="n-value">n: {search().n}</div> | ||
| <div data-testid="double-value">double: {doubleQuery().data}</div> | ||
| </div> | ||
| </div> | ||
| </Suspense> | ||
| ) | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,37 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { expect, test } from '@playwright/test' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| test('solid-query transitions keep previous data during navigation', async ({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| page, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await page.goto('/transition') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await expect(page.getByTestId('n-value')).toContainText('n: 1') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await expect(page.getByTestId('double-value')).toContainText('double: 2') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const bodySnapshots: Array<string> = [] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const interval = setInterval(async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const text = await page | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .locator('body') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .textContent() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .catch(() => '') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (text) bodySnapshots.push(text) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, 50) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await page.getByTestId('increase-button').click() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await page.waitForTimeout(200) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| clearInterval(interval) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await expect(page.getByTestId('n-value')).toContainText('n: 2', { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| timeout: 2_000, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await expect(page.getByTestId('double-value')).toContainText('double: 4', { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| timeout: 2_000, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const sawLoading = bodySnapshots.some((text) => text.includes('Loading...')) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(sawLoading).toBeFalsy() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+11
to
+36
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Critical flaw in test timing logic. The interval stops capturing snapshots at 200ms (line 25), but the expectations wait up to 2 seconds for updated values (lines 27-32). If the "Loading..." fallback appears between 200ms and when the values actually update, the test won't detect it—creating a false positive. Move - await page.waitForTimeout(200)
-
- clearInterval(interval)
-
await expect(page.getByTestId('n-value')).toContainText('n: 2', {
timeout: 2_000,
})
await expect(page.getByTestId('double-value')).toContainText('double: 4', {
timeout: 2_000,
})
+
+ clearInterval(interval)
const sawLoading = bodySnapshots.some((text) => text.includes('Loading...'))📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add cleanup for interval to prevent resource leak.
If the test fails or times out before line 25,
setIntervalwill continue running. Wrap the interval logic in a try-finally block or use Playwright's test.afterEach for cleanup.🤖 Prompt for AI Agents