Skip to content

Commit 5a08dfd

Browse files
committed
fix: address review feedback and audit all 28 skills against source
- Address all 9 Sheraff review comments (ensureQueryData, auth patterns, open redirect, useLocation migration, reusable components, zodValidator) - Address all 11 actionable CodeRabbit comments (naming collisions, framework-agnostic comments, missing imports, bin/intent.js error handling) - Full source-verified audit of all 28 skills across 7 re-audited packages - Fix solid-start: add HydrationScript, move HeadContent to body, use shellComponent pattern - Fix solid-router: useLoaderDeps returns Accessor<T>, ScrollRestoration deprecated, add missing hooks/components - Fix vue-router: useLinkProps returns LinkHTMLAttributes, useLoaderDeps returns Ref<T>, fix split-file convention - Fix start-server-core: correct file path, server fn prefix, getValidatedQuery API, add createServerFn imports to all examples - Fix router-core skills: preloadStaleTime wording, loader route param, error retry pattern, search-params adapter scoping - All 28 skills pass @tanstack/intent validate
1 parent baf9f5d commit 5a08dfd

36 files changed

Lines changed: 364 additions & 227 deletions

File tree

packages/react-router/bin/intent.js

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,12 @@
66
try {
77
await import('@tanstack/intent/intent-library')
88
} catch (e) {
9-
if (e?.code === 'ERR_MODULE_NOT_FOUND' || e?.code === 'MODULE_NOT_FOUND') {
9+
const isModuleNotFound =
10+
e?.code === 'ERR_MODULE_NOT_FOUND' || e?.code === 'MODULE_NOT_FOUND'
11+
const missingIntentLibrary =
12+
typeof e?.message === 'string' && e.message.includes('@tanstack/intent')
13+
14+
if (isModuleNotFound && missingIntentLibrary) {
1015
console.error('@tanstack/intent is not installed.')
1116
console.error('')
1217
console.error('Install it as a dev dependency:')

packages/react-router/eslint.config.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@ import type { Linter } from 'eslint'
66

77
export default [
88
...rootConfig,
9+
{
10+
ignores: ['bin/**'],
11+
},
912
{
1013
files: ['src/**/*.{ts,tsx}', 'tests/**/*.{ts,tsx}'],
1114
},

packages/react-router/package.json

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -90,10 +90,7 @@
9090
"sideEffects": false,
9191
"files": [
9292
"dist",
93-
"src",
94-
"skills",
95-
"bin",
96-
"!skills/_artifacts"
93+
"src"
9794
],
9895
"engines": {
9996
"node": ">=20.19"
@@ -107,7 +104,6 @@
107104
"tiny-warning": "^1.0.3"
108105
},
109106
"devDependencies": {
110-
"@tanstack/intent": "^0.0.14",
111107
"@testing-library/jest-dom": "^6.6.3",
112108
"@testing-library/react": "^16.2.0",
113109
"@vitejs/plugin-react": "^4.3.4",
@@ -121,8 +117,5 @@
121117
"peerDependencies": {
122118
"react": ">=18.0.0 || >=19.0.0",
123119
"react-dom": ">=18.0.0 || >=19.0.0"
124-
},
125-
"bin": {
126-
"intent": "./bin/intent.js"
127120
}
128121
}

packages/react-router/skills/compositions/router-query/SKILL.md

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ export const Route = createRootRouteWithContext<{
8686

8787
```tsx
8888
// src/router.tsx
89-
import { QueryClient } from '@tanstack/react-query'
89+
import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
9090
import { createRouter } from '@tanstack/react-router'
9191
import { routeTree } from './routeTree.gen'
9292

@@ -190,14 +190,21 @@ This is the recommended pattern. The loader ensures data is in the cache before
190190
import { queryOptions, useSuspenseQuery } from '@tanstack/react-query'
191191
import { createFileRoute } from '@tanstack/react-router'
192192

193+
interface Post {
194+
id: string
195+
title: string
196+
}
197+
193198
const postsQueryOptions = queryOptions({
194199
queryKey: ['posts'],
195-
queryFn: () => fetch('/api/posts').then((r) => r.json()),
200+
queryFn: (): Promise<Array<Post>> =>
201+
fetch('/api/posts').then((r) => r.json()),
196202
})
197203

198204
export const Route = createFileRoute('/posts')({
199205
loader: ({ context }) => {
200-
// ensureQueryData returns cached data if fresh, fetches if stale
206+
// ensureQueryData returns cached data if available, fetches if not in cache
207+
// To also refetch stale data, pass revalidateIfStale: true
201208
return context.queryClient.ensureQueryData(postsQueryOptions)
202209
},
203210
component: PostsPage,
@@ -209,7 +216,7 @@ function PostsPage() {
209216

210217
return (
211218
<ul>
212-
{posts.map((post: any) => (
219+
{posts.map((post) => (
213220
<li key={post.id}>{post.title}</li>
214221
))}
215222
</ul>
@@ -224,6 +231,12 @@ function PostsPage() {
224231
import { queryOptions, useSuspenseQuery } from '@tanstack/react-query'
225232
import { createFileRoute } from '@tanstack/react-router'
226233

234+
interface Post {
235+
id: string
236+
title: string
237+
content: string
238+
}
239+
227240
const postQueryOptions = (postId: string) =>
228241
queryOptions({
229242
queryKey: ['posts', postId],
@@ -250,6 +263,8 @@ function PostPage() {
250263
For non-critical data, start the fetch without blocking navigation:
251264

252265
```tsx
266+
import { useQuery, useSuspenseQuery } from '@tanstack/react-query'
267+
253268
export const Route = createFileRoute('/dashboard')({
254269
loader: ({ context }) => {
255270
// Await critical data
@@ -340,15 +355,15 @@ A module-level singleton `QueryClient` is shared across all server requests, lea
340355
```tsx
341356
// WRONG — shared across SSR requests
342357
const queryClient = new QueryClient()
343-
export function createRouter() {
358+
export function createAppRouter() {
344359
return createRouter({
345360
routeTree,
346361
context: { queryClient },
347362
})
348363
}
349364

350-
// CORRECT — new QueryClient per createRouter call
351-
export function createRouter() {
365+
// CORRECT — new QueryClient per createAppRouter call
366+
export function createAppRouter() {
352367
const queryClient = new QueryClient()
353368
return createRouter({
354369
routeTree,

packages/react-router/skills/lifecycle/migrate-from-react-router/SKILL.md

Lines changed: 51 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,11 @@ sources:
2020

2121
This is a step-by-step migration checklist. Each check covers one conversion task. Complete them in order.
2222

23-
> **CRITICAL**: If your UI is blank after migration, open the console. Errors like "cannot use useNavigate outside of context" mean React Router imports remain alongside TanStack Router imports. Uninstall `react-router` to surface them as TypeScript errors.
23+
> **CRITICAL**: If your UI is blank after migration, open the console. Errors like "cannot use useNavigate outside of context" mean React Router imports remain alongside TanStack Router imports. Uninstall `react-router` (and `react-router-dom` if present) to surface them as TypeScript errors.
24+
>
2425
> **CRITICAL**: TanStack Router uses `to` + `params` for navigation, NOT template literal paths. Never interpolate params into the `to` string.
26+
>
27+
> **NOTE**: React Router v7 recommends importing from `react-router` (not `react-router-dom`). The `react-router-dom` package still exists but just re-exports from `react-router`. Check for imports from both.
2528
2629
## Pre-Migration
2730

@@ -34,8 +37,8 @@ git checkout -b migrate-to-tanstack-router
3437
- [ ] **Install TanStack Router alongside React Router temporarily**
3538

3639
```bash
37-
npm install @tanstack/react-router
38-
npm install -D @tanstack/router-plugin @tanstack/react-router-devtools
40+
npm install @tanstack/react-router @tanstack/react-router-devtools
41+
npm install -D @tanstack/router-plugin
3942
```
4043

4144
- [ ] **Configure bundler plugin (Vite example)**
@@ -197,7 +200,7 @@ Key differences:
197200

198201
- `to` is a route path pattern, NOT an interpolated string
199202
- `params` is a separate prop with typed values
200-
- Active class: `className="[&.active]:font-bold"` (automatic `active` data attribute)
203+
- Active styling: use `activeProps={{ className: 'font-bold' }}` or `data-status="active"` attribute for CSS
201204

202205
- [ ] **Convert all `useNavigate` calls**
203206

@@ -294,6 +297,43 @@ Or from within the route component:
294297
const { postId } = Route.useParams()
295298
```
296299

300+
## `useLocation` — Common Pitfall
301+
302+
- [ ] **Replace `useLocation` with specific hooks**
303+
304+
React Router's `useLocation` is heavily used, and TanStack Router has a hook with the same name — but they are NOT equivalent. TanStack Router's `useLocation()` returns the router's current location, which during pending navigations may differ from what's currently rendered. Most React Router `useLocation` usage should be replaced with more specific hooks. See [#3110](https://github.com/TanStack/router/issues/3110).
305+
306+
Replace based on what you actually need:
307+
308+
```tsx
309+
// React Router
310+
import { useLocation } from 'react-router'
311+
const location = useLocation()
312+
313+
// ❌ DON'T just swap to TanStack Router's useLocation — it's the "live" URL
314+
import { useLocation } from '@tanstack/react-router'
315+
316+
// ✅ DO use the specific hook for what you need:
317+
import {
318+
useMatch,
319+
useMatches,
320+
useParams,
321+
useSearch,
322+
} from '@tanstack/react-router'
323+
324+
// Current route match (replaces most useLocation().pathname usage)
325+
const match = useMatch({ from: '/posts/$postId' })
326+
327+
// All active matches (replaces useLocation for breadcrumbs/analytics)
328+
const matches = useMatches()
329+
330+
// Path params (replaces useLocation + manual parsing)
331+
const { postId } = useParams({ from: '/posts/$postId' })
332+
333+
// Search params (replaces useLocation().search parsing)
334+
const { page } = useSearch({ from: '/posts' })
335+
```
336+
297337
## Outlet
298338

299339
- [ ] **Replace React Router `Outlet` with TanStack Router `Outlet`**
@@ -352,62 +392,28 @@ Key differences:
352392

353393
## Code Splitting
354394

355-
- [ ] **Convert lazy route imports**
356-
357-
React Router:
358-
359-
```tsx
360-
const LazyPage = lazy(() => import('./pages/LazyPage'))
361-
{ path: '/lazy', element: <Suspense><LazyPage /></Suspense> }
362-
```
363-
364-
TanStack Router (with `autoCodeSplitting: true` in plugin config, this is automatic). For manual splitting:
395+
- [ ] **Convert lazy route imports** — with `autoCodeSplitting: true` in the plugin config, this is automatic. For manual splitting, use `.lazy.tsx` files:
365396

366397
```tsx
367398
// src/routes/lazy-page.lazy.tsx
368399
import { createLazyFileRoute } from '@tanstack/react-router'
369400

370401
export const Route = createLazyFileRoute('/lazy-page')({
371-
component: LazyPage,
402+
component: () => <div>Lazy loaded</div>,
372403
})
373-
374-
function LazyPage() {
375-
return <div>Lazy loaded</div>
376-
}
377404
```
378405

379406
## Cleanup
380407

381-
- [ ] **Remove React Router**
408+
- [ ] **Remove React Router and verify**
382409

383410
```bash
384411
npm uninstall react-router react-router-dom
412+
grep -r "from 'react-router" src/ # find stale imports
413+
npx tsc --noEmit # verify clean build
385414
```
386415

387-
- [ ] **Search for remaining React Router imports**
388-
389-
```bash
390-
grep -r "from 'react-router" src/
391-
grep -r 'from "react-router' src/
392-
```
393-
394-
Any remaining imports will now produce TypeScript errors after uninstalling.
395-
396-
- [ ] **Verify TypeScript compiles cleanly**
397-
398-
```bash
399-
npx tsc --noEmit
400-
```
401-
402-
- [ ] **Test all routes manually**
403-
404-
Verify:
405-
406-
- All routes render
407-
- Navigation works (including browser back/forward)
408-
- Search params persist and validate
409-
- Dynamic route params resolve
410-
- Loaders execute and data displays
416+
- [ ] **Test all routes** — verify rendering, navigation (incl. back/forward), search params, dynamic params, and loaders
411417

412418
## Common Mistakes
413419

@@ -477,6 +483,7 @@ File naming also uses `$`: `src/routes/posts/$postId.tsx`
477483
| `useParams()` | `useParams({ from: '/route/$param' })` |
478484
| `useSearchParams()` | `validateSearch` + `useSearch({ from })` |
479485
| `useLoaderData()` | `Route.useLoaderData()` |
486+
| `useLocation()` | `useMatch`, `useMatches`, `useParams`, `useSearch` |
480487
| `<Outlet />` | `<Outlet />` |
481488
| `loader({ params })` | `loader: ({ params }) => ...` (route option) |
482489
| `action({ request })` | Use mutations / form libraries |

packages/react-router/skills/react-router/SKILL.md

Lines changed: 32 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,23 @@ function Nav() {
381381
}
382382
```
383383

384+
### Reusable Components with Router Hooks
385+
386+
To create a component that uses router hooks across multiple routes, pass a union of route paths as the `from` prop:
387+
388+
```tsx
389+
function PostIdDisplay({ from }: { from: '/posts/$id' | '/drafts/$id' }) {
390+
const { id } = useParams({ from })
391+
return <span>ID: {id}</span>
392+
}
393+
394+
// Usage in different route components
395+
<PostIdDisplay from="/posts/$id" />
396+
<PostIdDisplay from="/drafts/$id" />
397+
```
398+
399+
This pattern avoids `strict: false` (which returns an imprecise union) while keeping the component reusable across specific known routes.
400+
384401
### Auth Provider Must Wrap RouterProvider
385402

386403
If routes use auth context (via `createRootRouteWithContext`), the auth provider must be an ancestor of `RouterProvider`:
@@ -420,7 +437,7 @@ const router = createRouter({
420437

421438
### 1. HIGH: Using React hooks in `beforeLoad` or `loader`
422439

423-
`beforeLoad` and `loader` are NOT React components — they are plain async functions called by the router. React hooks cannot be used in them.
440+
`beforeLoad` and `loader` are NOT React components — they are plain async functions. React hooks cannot be called in them. Pass auth state via router context instead.
424441

425442
```tsx
426443
// WRONG — useAuth is a React hook, cannot be called here
@@ -429,18 +446,7 @@ beforeLoad: () => {
429446
if (!auth.user) throw redirect({ to: '/login' })
430447
}
431448

432-
// CORRECT — pass auth state via router context
433-
const rootRoute = createRootRouteWithContext<{ auth: AuthState }>()({
434-
component: RootComponent,
435-
})
436-
437-
// In the component that creates the router:
438-
const router = createRouter({
439-
routeTree,
440-
context: { auth: getAuthState() },
441-
})
442-
443-
// Then in a route:
449+
// CORRECT — read auth from router context
444450
beforeLoad: ({ context }) => {
445451
if (!context.auth.isAuthenticated) {
446452
throw redirect({ to: '/login' })
@@ -450,25 +456,26 @@ beforeLoad: ({ context }) => {
450456

451457
### 2. HIGH: Wrapping RouterProvider inside an auth provider incorrectly
452458

453-
If you use `createRootRouteWithContext<{ auth: AuthState }>()`, the auth state must be available when the router is created — not injected after.
459+
Create the router once with an `undefined!` placeholder, then inject live auth via `RouterProvider`'s `context` prop. Do NOT recreate the router on auth changes — this resets caches and rebuilds the tree.
454460

455461
```tsx
456-
// WRONG — router created before auth is available
457-
const router = createRouter({ routeTree, context: {} })
462+
// CORRECT — create router once, inject live auth via context prop
463+
const router = createRouter({
464+
routeTree,
465+
context: { auth: undefined! }, // placeholder, filled by RouterProvider
466+
})
458467

459-
function App() {
460-
const auth = useAuth() // too late
461-
return <RouterProvider router={router} />
468+
function InnerApp() {
469+
const auth = useAuth()
470+
return <RouterProvider router={router} context={{ auth }} />
462471
}
463472

464-
// CORRECT — provide context at creation time
465473
function App() {
466-
const auth = useAuth()
467-
const router = useMemo(
468-
() => createRouter({ routeTree, context: { auth } }),
469-
[auth],
474+
return (
475+
<AuthProvider>
476+
<InnerApp />
477+
</AuthProvider>
470478
)
471-
return <RouterProvider router={router} />
472479
}
473480
```
474481

packages/react-start/bin/intent.js

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,12 @@
66
try {
77
await import('@tanstack/intent/intent-library')
88
} catch (e) {
9-
if (e?.code === 'ERR_MODULE_NOT_FOUND' || e?.code === 'MODULE_NOT_FOUND') {
9+
const isModuleNotFound =
10+
e?.code === 'ERR_MODULE_NOT_FOUND' || e?.code === 'MODULE_NOT_FOUND'
11+
const missingIntentLibrary =
12+
typeof e?.message === 'string' && e.message.includes('@tanstack/intent')
13+
14+
if (isModuleNotFound && missingIntentLibrary) {
1015
console.error('@tanstack/intent is not installed.')
1116
console.error('')
1217
console.error('Install it as a dev dependency:')

0 commit comments

Comments
 (0)