-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
fix(router-core): lane match loader rewrite #7805
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
Open
Sheraff
wants to merge
12
commits into
main
Choose a base branch
from
fix-router-core-lane-match-loader
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
ceaa4d2
fix: lane match loader rewrite
Sheraff adf73dd
fix client/server build
Sheraff 6f75f50
adopt preloaded beforeLoad
Sheraff f4b8934
fix adoption of server data
Sheraff 7b5436b
fix internal error propagation, only user code is try/catch
Sheraff fb21a42
fix: solid Transitionner unsub cleanup before early return
Sheraff bf90602
fix: react-router not found ambiguous assertion
Sheraff f2ccae2
docs
Sheraff da5cb82
fix: preload always releases its borrowed lease
Sheraff cc21be0
error priority handling
Sheraff 3a56878
Merge branch 'main' into fix-router-core-lane-match-loader
Sheraff 2e8bbbe
Merge remote-tracking branch 'origin/main' into fix-router-core-lane-…
Sheraff File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
48 changes: 48 additions & 0 deletions
48
e2e/solid-start/selective-ssr/src/routes/ssr-false-pending-min.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| import { onCleanup, onMount } from 'solid-js' | ||
| import { createFileRoute } from '@tanstack/solid-router' | ||
|
|
||
| const pendingMinMs = 1500 | ||
|
|
||
| function recordEvent(type: string) { | ||
| if (typeof window === 'undefined') { | ||
| return | ||
| } | ||
|
|
||
| const global = window as any | ||
| global.__events ??= [] | ||
| global.__events.push({ type, t: performance.now() }) | ||
| } | ||
|
|
||
| function Pending() { | ||
| onMount(() => { | ||
| recordEvent('pending-mounted') | ||
| }) | ||
| onCleanup(() => { | ||
| recordEvent('pending-unmounted') | ||
| }) | ||
|
|
||
| return <div data-testid="ssr-false-pending">Loading SSR false route...</div> | ||
| } | ||
|
|
||
| function Target() { | ||
| onMount(() => { | ||
| recordEvent('target-mounted') | ||
| }) | ||
|
|
||
| return <div data-testid="ssr-false-target">SSR false route loaded</div> | ||
| } | ||
|
|
||
| export const Route = createFileRoute('/ssr-false-pending-min')({ | ||
| ssr: false, | ||
| pendingMs: 0, | ||
| pendingMinMs, | ||
| pendingComponent: Pending, | ||
| loader: async () => { | ||
| recordEvent('loader-start') | ||
| await new Promise((resolve) => setTimeout(resolve, 50)) | ||
| recordEvent('loader-done') | ||
|
|
||
| return { ok: true } | ||
| }, | ||
| component: Target, | ||
| }) | ||
122 changes: 122 additions & 0 deletions
122
e2e/solid-start/selective-ssr/tests/issue-7283-dev-hydration.spec.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,122 @@ | ||
| import { spawn } from 'node:child_process' | ||
| import path from 'node:path' | ||
| import { expect, test } from '@playwright/test' | ||
| import type { ChildProcess } from 'node:child_process' | ||
| import type { Page } from '@playwright/test' | ||
|
|
||
| // Regression test for https://github.com/TanStack/router/issues/7283 | ||
| // | ||
| // Selective SSR routes (`ssr: false` / `ssr: 'data-only'`) that declare a | ||
| // `pendingComponent` must hydrate cleanly in DEV mode. Solid only validates | ||
| // hydration markers in dev, so the production webServer configured in | ||
| // playwright.config.ts masks the mismatch ("template is not a function", | ||
| // route never reaches its loaded component). This spec therefore boots its | ||
| // own `vite dev` server on a dedicated port and drives the app against it. | ||
|
|
||
| const DEV_PORT = 58283 | ||
| const devBaseURL = `http://localhost:${DEV_PORT}` | ||
|
|
||
| let devServer: ChildProcess | undefined | ||
|
|
||
| async function waitForServer(url: string, timeoutMs: number) { | ||
| const deadline = Date.now() + timeoutMs | ||
| let lastError: unknown | ||
| while (Date.now() < deadline) { | ||
| try { | ||
| const res = await fetch(url) | ||
| if (res.ok) return | ||
| } catch (err) { | ||
| lastError = err | ||
| } | ||
| await new Promise((resolve) => setTimeout(resolve, 1000)) | ||
| } | ||
| throw new Error(`dev server did not start on ${url}: ${lastError}`) | ||
| } | ||
|
|
||
| test.beforeAll(async () => { | ||
| const appDir = path.resolve(import.meta.dirname, '..') | ||
| devServer = spawn( | ||
| 'pnpm', | ||
| [ | ||
| 'dev:e2e', | ||
| '--host', | ||
| 'localhost', | ||
| '--port', | ||
| String(DEV_PORT), | ||
| '--strictPort', | ||
| ], | ||
| { | ||
| cwd: appDir, | ||
| env: { | ||
| ...process.env, | ||
| VITE_SERVER_PORT: String(DEV_PORT), | ||
| NODE_ENV: 'development', | ||
| }, | ||
| stdio: 'ignore', | ||
| detached: true, | ||
| }, | ||
| ) | ||
| await waitForServer(`${devBaseURL}/`, 90_000) | ||
| }) | ||
|
|
||
| test.afterAll(() => { | ||
| if (devServer?.pid) { | ||
| try { | ||
| process.kill(-devServer.pid, 'SIGTERM') | ||
| } catch { | ||
| // already gone | ||
| } | ||
| } | ||
| }) | ||
|
|
||
| function collectHydrationFailures(page: Page) { | ||
| const failures: Array<string> = [] | ||
| const isHydrationFailure = (text: string) => | ||
| text.includes('template is not a function') || | ||
| text.includes("wasn't caught by any route") || | ||
| text.includes('Hydration Mismatch') | ||
| page.on('pageerror', (err) => { | ||
| if (isHydrationFailure(err.message)) failures.push(err.message) | ||
| }) | ||
| page.on('console', (msg) => { | ||
| if (msg.type() === 'error' || msg.type() === 'warning') { | ||
| const text = msg.text() | ||
| if (isHydrationFailure(text)) failures.push(text) | ||
| } | ||
| }) | ||
| return failures | ||
| } | ||
|
|
||
| test.describe('dev-mode hydration of selective SSR routes with pendingComponent', () => { | ||
| test.setTimeout(120_000) | ||
|
|
||
| test('ssr:false route with pendingComponent hydrates cleanly and reaches its loaded component (issue #7283)', async ({ | ||
| page, | ||
| }) => { | ||
| const failures = collectHydrationFailures(page) | ||
|
|
||
| await page.goto(`${devBaseURL}/ssr-false-pending-min`) | ||
|
|
||
| // The loaded component must eventually replace the pending UI. | ||
| await expect(page.getByTestId('ssr-false-target')).toBeVisible({ | ||
| timeout: 15_000, | ||
| }) | ||
|
|
||
| // No dev hydration mismatch may occur along the way. | ||
| expect(failures).toEqual([]) | ||
| }) | ||
|
|
||
| test('data-only route with pendingComponent hydrates cleanly and reaches its loaded component (regression vs main)', async ({ | ||
| page, | ||
| }) => { | ||
| const failures = collectHydrationFailures(page) | ||
|
|
||
| await page.goto(`${devBaseURL}/data-only-pending-component`) | ||
|
|
||
| await expect( | ||
| page.getByTestId('data-only-pending-component-ready-label'), | ||
| ).toHaveText('OK - loader finished', { timeout: 15_000 }) | ||
|
|
||
| expect(failures).toEqual([]) | ||
| }) | ||
| }) |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: TanStack/router
Length of output: 4375
🏁 Script executed:
Repository: TanStack/router
Length of output: 3954
Type the event buffer instead of casting
windowtoany.window as anybypasses strict TS here; add a localWindowaugmentation for__eventsand push throughwindow.__events.🤖 Prompt for AI Agents
Source: Coding guidelines