Conversation
✅ Deploy Preview for tanstack ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughThis PR migrates the project from Oxlint to ESLint as the primary linter, establishes a new ESLint flat configuration, and removes the docs and blog post redirect resolution system. The majority of changes are comment directives replacing Changes
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/routes/$libraryId/$version.docs.framework.$framework.$.tsx (1)
25-44:⚠️ Potential issue | 🟠 MajorLegacy framework doc links now bounce to the section root.
This loader no longer resolves renamed pages before
loadDocs()misses, so old deep links end up in the catch block and redirect to/docs/framework/$frameworkinstead of their canonical replacement. Users lose the specific page they asked for.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/`$libraryId/$version.docs.framework.$framework.$.tsx around lines 25 - 44, The catch block currently treats any missing doc as a framework-root redirect which loses legacy deep links; before throwing the generic redirect, call a function that resolves renamed/relocated pages (e.g., getRenamedDoc or resolveCanonicalDoc) using the same inputs used for loadDocs (library, version via getBranch(library, version), framework and docsPath constructed from library.docsRoot) and, if it returns a canonical replacement path, throw redirect to that canonical path; only if no replacement is found keep the existing redirect behavior, and continue to use isNotFound(error) to gate this logic.
🧹 Nitpick comments (2)
eslint.config.mjs (1)
52-67: Consider adding explicit React rules.The
reactplugin is imported but no React-specific rules (likereact/jsx-key,react/no-unescaped-entities) are applied. If you only need hooks linting, the import can be removed. Otherwise, consider spreadingreact.configs.recommended.rulesor selecting specific rules.Option 1: Remove unused import
-import react from 'eslint-plugin-react'And update the plugins object:
plugins: { - react, 'react-hooks': reactHooks, 'jsx-a11y': jsxA11y, },Option 2: Add React rules
rules: { ...reactHooks.configs.recommended.rules, ...jsxA11y.configs.recommended.rules, + ...react.configs.recommended.rules, 'react-hooks/set-state-in-effect': 'warn', },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eslint.config.mjs` around lines 52 - 67, The ESLint config currently registers the react plugin but doesn't apply any React rules; either remove the unused import/entry for react in the plugins block or add explicit React rules — e.g., spread react.configs.recommended.rules into the rules object (alongside ...reactHooks.configs.recommended.rules and ...jsxA11y.configs.recommended.rules) or selectively add rules such as 'react/jsx-key' and 'react/no-unescaped-entities'; update the plugins and rules entries (the react identifier and the rules object where ...reactHooks.configs.recommended.rules and ...jsxA11y.configs.recommended.rules are used) accordingly so the react plugin is either removed or its recommended rules are applied.src/hooks/useMutation.ts (1)
61-63: Drop unusedvariablesfromhandleSubmitdependencies.At Line 62,
variablesis not used in the callback body, so this causes unnecessary callback recreation.Proposed fix
- // eslint-disable-next-line react-hooks/exhaustive-deps - [mutate, variables], + [mutate],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useMutation.ts` around lines 61 - 63, The dependency array for the useCallback that defines handleSubmit currently includes variables even though variables is not referenced in the callback body; update the useCallback dependencies to only include mutate (i.e., change [mutate, variables] to [mutate]) and remove the now-unnecessary eslint-disable-next-line react-hooks/exhaustive-deps comment. Ensure you edit the useCallback that defines handleSubmit in useMutation.ts so the hook no longer recreates the callback unnecessarily.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/Doc.tsx`:
- Line 132: The anchor element currently uses aria-hidden="true" while remaining
focusable and unconditionally renders `${pagePath}.md`, which can produce
"undefined.md"; remove the aria-hidden attribute from the <a> and change the JSX
to only render the anchor when pagePath is defined (e.g., wrap the anchor in a
conditional that checks pagePath or use a guard like pagePath && ...) so the
link is not present/focusable when there is no pagePath and the href is only
created when pagePath exists.
- Line 4: The hook useWidthToggle is being called inside a try/catch which
violates React Hooks rules; replace this pattern by adding a non-throwing
wrapper (e.g., create useOptionalWidthToggle in the same module that currently
defines useWidthToggle, such as src/components/DocsLayout.tsx) that returns the
width toggle value or undefined instead of throwing when the context is absent,
then update Doc.tsx to import and call useOptionalWidthToggle (or the new
variant) at the top level unconditionally and remove the try/catch around the
hook call; ensure useWidthToggle keeps throwing behavior if you want a strict
version, while useOptionalWidthToggle uses useContext safely and returns
undefined for absent context.
In `@src/hooks/useMutation.ts`:
- Around line 52-53: The mutate callback currently only lists [opts.fn] as its
dependency which can cause it to close over stale handlers; update the
dependency array for the mutate callback (the function created in useMutation)
to include opts.onSuccess, opts.onError, and opts.onSettled in addition to
opts.fn so that the latest handler functions are used when mutate runs (or
alternatively stabilize opts by memoizing it upstream and then depend on that
stable reference).
In `@src/routes/`$libraryId/$version.docs.$.tsx:
- Around line 25-36: The catch branch after calling loadDocs currently turns
legacy/moved docs into hard 404s; instead, before throwing notFound() check for
legacy redirects and return a redirect when found. Update the catch around
loadDocs(...) to call the existing redirect-resolution routine (e.g.,
resolveLegacyDocRedirect or resolveRedirects) using the same inputs (library,
version, docsPath, getBranch(library, version), library.docsRoot) and if it
returns a canonical URL issue a redirect response; only call throw notFound() if
no redirect is found. Ensure you reference the same symbols used in the diff:
loadDocs, getBranch, library.docsRoot, docsPath, and notFound.
In `@src/utils/documents.server.ts`:
- Around line 343-349: The returned object currently spreads ...result.data
which still exposes frontmatter keys like redirect_from and redirectFrom; update
the return to explicitly omit those fields by creating a sanitizedData copy
(e.g., destructure or shallow clone of result.data while excluding redirect_from
and redirectFrom) and then return { ...result, data: { ...sanitizedData,
description: createExcerpt(result.content) } } so the redirect fields are not
present; reference result, result.data, createExcerpt, redirect_from and
redirectFrom when making the change.
---
Outside diff comments:
In `@src/routes/`$libraryId/$version.docs.framework.$framework.$.tsx:
- Around line 25-44: The catch block currently treats any missing doc as a
framework-root redirect which loses legacy deep links; before throwing the
generic redirect, call a function that resolves renamed/relocated pages (e.g.,
getRenamedDoc or resolveCanonicalDoc) using the same inputs used for loadDocs
(library, version via getBranch(library, version), framework and docsPath
constructed from library.docsRoot) and, if it returns a canonical replacement
path, throw redirect to that canonical path; only if no replacement is found
keep the existing redirect behavior, and continue to use isNotFound(error) to
gate this logic.
---
Nitpick comments:
In `@eslint.config.mjs`:
- Around line 52-67: The ESLint config currently registers the react plugin but
doesn't apply any React rules; either remove the unused import/entry for react
in the plugins block or add explicit React rules — e.g., spread
react.configs.recommended.rules into the rules object (alongside
...reactHooks.configs.recommended.rules and
...jsxA11y.configs.recommended.rules) or selectively add rules such as
'react/jsx-key' and 'react/no-unescaped-entities'; update the plugins and rules
entries (the react identifier and the rules object where
...reactHooks.configs.recommended.rules and ...jsxA11y.configs.recommended.rules
are used) accordingly so the react plugin is either removed or its recommended
rules are applied.
In `@src/hooks/useMutation.ts`:
- Around line 61-63: The dependency array for the useCallback that defines
handleSubmit currently includes variables even though variables is not
referenced in the callback body; update the useCallback dependencies to only
include mutate (i.e., change [mutate, variables] to [mutate]) and remove the
now-unnecessary eslint-disable-next-line react-hooks/exhaustive-deps comment.
Ensure you edit the useCallback that defines handleSubmit in useMutation.ts so
the hook no longer recreates the callback unnecessarily.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f71203d3-8790-40af-9e14-0942ad99e2e8
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (57)
.oxlintrc.jsoncontent-collections.tseslint.config.mjspackage.jsonsrc/components/Doc.tsxsrc/components/DocFeedbackFloatingButton.tsxsrc/components/DocFeedbackNote.tsxsrc/components/DocsLayout.tsxsrc/components/ExampleDeployDialog.tsxsrc/components/FileExplorer.tsxsrc/components/FilterComponents.tsxsrc/components/ImageUpload.client.tsxsrc/components/Navbar.tsxsrc/components/SearchModal.tsxsrc/components/ShowcaseSubmitForm.tsxsrc/components/StackBlitzEmbed.tsxsrc/components/ThemeProvider.tsxsrc/components/Tooltip.tsxsrc/components/builder/BuilderProvider.tsxsrc/components/builder/DeployDialog.tsxsrc/components/builder/useBuilderUrl.tssrc/components/builder/webcontainer/PreviewPanel.tsxsrc/components/game/engine/GameEngine.tssrc/components/game/machines/GameMachineProvider.tsxsrc/components/game/scene/Cannonballs.tsxsrc/components/game/scene/GameScene.tsxsrc/components/game/scene/Ocean.tsxsrc/components/game/ui/BadgeOverlay.tsxsrc/components/game/ui/CompassIndicator.tsxsrc/components/game/ui/DebugPanel.tsxsrc/components/game/ui/GameHUD.tsxsrc/components/game/ui/GameOverOverlay.tsxsrc/components/game/ui/IslandIndicator.tsxsrc/components/game/ui/UpgradeOverlay.tsxsrc/components/game/ui/WinOverlay.tsxsrc/components/markdown/MarkdownLink.tsxsrc/components/npm-stats/PackageSearch.tsxsrc/hooks/useAILibraryHeroAnimation.tssrc/hooks/useMutation.tssrc/hooks/useNpmDownloadCounter.tssrc/hooks/useScript.tssrc/routes/$libraryId/$version.docs.$.tsxsrc/routes/$libraryId/$version.docs.framework.$framework.$.tsxsrc/routes/admin/audit.tsxsrc/routes/admin/github-stats.tsxsrc/routes/admin/logins.tsxsrc/routes/admin/npm-stats.tsxsrc/routes/admin/roles.$roleId.tsxsrc/routes/admin/roles.index.tsxsrc/routes/admin/users.tsxsrc/routes/blog.$.tsxsrc/routes/dashboard.tsxsrc/routes/stats/npm/index.tsxsrc/utils/docs.tssrc/utils/documents.server.tssrc/utils/redirects.tssrc/utils/useLocalStorage.ts
💤 Files with no reviewable changes (4)
- src/utils/docs.ts
- src/routes/blog.$.tsx
- .oxlintrc.json
- src/utils/redirects.ts
| import { FoldHorizontal, UnfoldHorizontal } from 'lucide-react' | ||
| import { twMerge } from 'tailwind-merge' | ||
| import { useOptionalWidthToggle, DocNavigation } from '~/components/DocsLayout' | ||
| import { useWidthToggle, DocNavigation } from '~/components/DocsLayout' |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Do React Rules of Hooks (eslint-plugin-react-hooks) allow calling a hook inside a try/catch block?
💡 Result:
No. Rules of Hooks (and eslint-plugin-react-hooks) forbid calling any Hook inside a try/catch/finally block—Hooks must be called unconditionally at the top level so React can preserve the same call order on every render. [1][2]
Also, wrapping render-time Hook calls in try/catch is not a valid way to handle render errors; React recommends using Error Boundaries instead, and the plugin even has a lint warning for try/catch around certain Hook usage to prevent confusion. [3]
Sources: [1] React “Rules of Hooks” docs, [2] eslint-plugin-react-hooks rules-of-hooks lint docs, [3] eslint-plugin-react-hooks “error-boundaries” lint docs.
🏁 Script executed:
# First, check the Doc.tsx file to see the actual code
cat -n src/components/Doc.tsx | head -150Repository: TanStack/tanstack.com
Length of output: 5589
🏁 Script executed:
# Check the DocsLayout.tsx to understand the context
cd src/components && grep -n "useWidthToggle\|export" DocsLayout.tsx | head -30Repository: TanStack/tanstack.com
Length of output: 305
Avoid calling useWidthToggle inside try/catch; this violates React's Rules of Hooks and will fail eslint-plugin-react-hooks linting.
At Line 85, the hook is invoked inside a try/catch block. React's Rules of Hooks forbid calling any hook inside try/catch/finally blocks—hooks must be called unconditionally at the top level so React can preserve the same call order on every render. After the ESLint migration, this will trigger a lint error.
The idiomatic solution is to make useWidthToggle non-throwing and return undefined when context is unavailable, or create a separate useOptionalWidthToggle variant.
Suggested fix
- import { useWidthToggle, DocNavigation } from '~/components/DocsLayout'
+ import { useOptionalWidthToggle, DocNavigation } from '~/components/DocsLayout'
...
- let isFullWidth = false
- let setIsFullWidth: ((isFullWidth: boolean) => void) | undefined
-
- try {
- const context = useWidthToggle()
- isFullWidth = context.isFullWidth
- setIsFullWidth = context.setIsFullWidth
- } catch {
- // Context not available, that's okay
- }
+ const widthToggle = useOptionalWidthToggle()
+ const isFullWidth = widthToggle?.isFullWidth ?? false
+ const setIsFullWidth = widthToggle?.setIsFullWidthCreate useOptionalWidthToggle in src/components/DocsLayout.tsx that wraps useContext without throwing.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/Doc.tsx` at line 4, The hook useWidthToggle is being called
inside a try/catch which violates React Hooks rules; replace this pattern by
adding a non-throwing wrapper (e.g., create useOptionalWidthToggle in the same
module that currently defines useWidthToggle, such as
src/components/DocsLayout.tsx) that returns the width toggle value or undefined
instead of throwing when the context is absent, then update Doc.tsx to import
and call useOptionalWidthToggle (or the new variant) at the top level
unconditionally and remove the try/catch around the hook call; ensure
useWidthToggle keeps throwing behavior if you want a strict version, while
useOptionalWidthToggle uses useContext safely and returns undefined for absent
context.
| return ( | ||
| <div className="flex-1 min-h-0 flex flex-col pt-4 lg:pt-6 xl:pt-8"> | ||
| <a href={`${pagePath}.md`} className="sr-only"> | ||
| <a href={`${pagePath}.md`} className="sr-only" aria-hidden="true"> |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
According to WAI-ARIA guidance, is it valid to apply aria-hidden="true" to a focusable anchor element?
💡 Result:
No. WAI-ARIA guidance says you must not apply aria-hidden="true" to a focusable element (including a focusable <a>), because it can result in users tabbing to something assistive technologies can’t perceive/announce. WAI’s “Using ARIA” explicitly states: “Do not use … aria-hidden="true" on a focusable element.” [2]. The W3C ARIA in HTML spec likewise says authors may use aria-hidden on HTML elements except focusable elements (and body) [1].
Sources: [1] [2]
🏁 Script executed:
cd src/components && head -150 Doc.tsx | tail -50Repository: TanStack/tanstack.com
Length of output: 1578
🏁 Script executed:
wc -l src/components/Doc.tsxRepository: TanStack/tanstack.com
Length of output: 91
🏁 Script executed:
cat -n src/components/Doc.tsx | sed -n '1,50p'Repository: TanStack/tanstack.com
Length of output: 1675
🏁 Script executed:
cat -n src/components/Doc.tsx | sed -n '120,150p'Repository: TanStack/tanstack.com
Length of output: 1251
Remove aria-hidden="true" and add conditional rendering for optional pagePath.
At line 132, the anchor is focusable while hidden from assistive technology, violating WAI-ARIA guidance. Additionally, pagePath is optional, so this renders undefined.md when undefined.
Suggested fix
- <a href={`${pagePath}.md`} className="sr-only" aria-hidden="true">
- AI/LLM: This documentation page is available in plain markdown format at
- {pagePath}.md
- </a>
+ {pagePath ? (
+ <a href={`${pagePath}.md`} className="sr-only">
+ AI/LLM: This documentation page is available in plain markdown format at
+ {pagePath}.md
+ </a>
+ ) : null}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/Doc.tsx` at line 132, The anchor element currently uses
aria-hidden="true" while remaining focusable and unconditionally renders
`${pagePath}.md`, which can produce "undefined.md"; remove the aria-hidden
attribute from the <a> and change the JSX to only render the anchor when
pagePath is defined (e.g., wrap the anchor in a conditional that checks pagePath
or use a guard like pagePath && ...) so the link is not present/focusable when
there is no pagePath and the href is only created when pagePath exists.
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| [opts.fn], |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify captured `opts.*` members vs useCallback dependency list
rg -n -C2 "const mutate = React\.useCallback|opts\.onSuccess|opts\.onError|opts\.onSettled|\[opts\.fn\]" src/hooks/useMutation.tsRepository: TanStack/tanstack.com
Length of output: 969
🏁 Script executed:
cat -n src/hooks/useMutation.tsRepository: TanStack/tanstack.com
Length of output: 3620
🏁 Script executed:
rg -n "useMutation|useBaseMutation" --type ts --type tsx -B2 -A5 | head -100Repository: TanStack/tanstack.com
Length of output: 92
🏁 Script executed:
fd -e ts -e tsx | xargs rg "useMutation|useBaseMutation" -B2 -A5 | head -150Repository: TanStack/tanstack.com
Length of output: 8404
🏁 Script executed:
rg "from '~/hooks/useMutation'" -A20Repository: TanStack/tanstack.com
Length of output: 1632
🏁 Script executed:
rg "bytesSignupMutation|useMutation" src/routes/index.tsx -B3 -A15Repository: TanStack/tanstack.com
Length of output: 4826
Add missing dependencies to mutate callback.
At lines 52–53, the mutate callback captures opts.onSuccess, opts.onError, and opts.onSettled (lines 37, 44, 45) but only depends on opts.fn. When these handlers change between renders without being included in the dependency array, the callback will invoke stale versions.
Proposed fix
- // eslint-disable-next-line react-hooks/exhaustive-deps
- [opts.fn],
+ [opts.fn, opts.onSuccess, opts.onError, opts.onSettled],📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // eslint-disable-next-line react-hooks/exhaustive-deps | |
| [opts.fn], | |
| [opts.fn, opts.onSuccess, opts.onError, opts.onSettled], |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/hooks/useMutation.ts` around lines 52 - 53, The mutate callback currently
only lists [opts.fn] as its dependency which can cause it to close over stale
handlers; update the dependency array for the mutate callback (the function
created in useMutation) to include opts.onSuccess, opts.onError, and
opts.onSettled in addition to opts.fn so that the latest handler functions are
used when mutate runs (or alternatively stabilize opts by memoizing it upstream
and then depend on that stable reference).
| try { | ||
| return await loadDocs({ | ||
| repo: library.repo, | ||
| branch, | ||
| docsPath: `${docsRoot}/${docsPath ?? ''}`, | ||
| branch: getBranch(library, version), | ||
| docsPath: `${library.docsRoot || 'docs'}/${docsPath}`, | ||
| }) | ||
| } catch (error) { | ||
| const isNotFoundError = | ||
| isNotFound(error) || | ||
| (error && typeof error === 'object' && 'isNotFound' in error) | ||
|
|
||
| if (isNotFoundError) { | ||
| throw notFound() |
There was a problem hiding this comment.
This turns legacy docs aliases into hard 404s.
With the redirect resolution removed ahead of loadDocs(), any renamed or moved docs page now falls into the notFound() branch instead of redirecting to its canonical URL. That will break existing bookmarks and inbound links.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routes/`$libraryId/$version.docs.$.tsx around lines 25 - 36, The catch
branch after calling loadDocs currently turns legacy/moved docs into hard 404s;
instead, before throwing notFound() check for legacy redirects and return a
redirect when found. Update the catch around loadDocs(...) to call the existing
redirect-resolution routine (e.g., resolveLegacyDocRedirect or resolveRedirects)
using the same inputs (library, version, docsPath, getBranch(library, version),
library.docsRoot) and if it returns a canonical URL issue a redirect response;
only call throw notFound() if no redirect is found. Ensure you reference the
same symbols used in the diff: loadDocs, getBranch, library.docsRoot, docsPath,
and notFound.
| return { | ||
| ...result, | ||
| data: { | ||
| ...result.data, | ||
| description: createExcerpt(result.content), | ||
| redirect_from: redirectFrom, | ||
| redirectFrom, | ||
| } as { [key: string]: any } & { | ||
| description: string | ||
| redirect_from?: Array<string> | ||
| redirectFrom?: Array<string> | ||
| }, | ||
| } as { [key: string]: any } & { description: string }, | ||
| } |
There was a problem hiding this comment.
Explicitly omit redirect fields before returning data.
...result.data still copies every frontmatter key through unchanged. If this helper is supposed to stop exposing redirect_from / redirectFrom, those properties are still present until you remove them from the object you return.
Suggested fix
export function extractFrontMatter(content: string) {
const result = graymatter.default(content, {
excerpt: (file: any) => (file.excerpt = createRichExcerpt(file.content)),
})
+ const data = { ...(result.data as Record<string, unknown>) }
+ delete data.redirect_from
+ delete data.redirectFrom
return {
...result,
data: {
- ...result.data,
+ ...data,
description: createExcerpt(result.content),
- } as { [key: string]: any } & { description: string },
+ } as Record<string, unknown> & { description: string },
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/utils/documents.server.ts` around lines 343 - 349, The returned object
currently spreads ...result.data which still exposes frontmatter keys like
redirect_from and redirectFrom; update the return to explicitly omit those
fields by creating a sanitizedData copy (e.g., destructure or shallow clone of
result.data while excluding redirect_from and redirectFrom) and then return {
...result, data: { ...sanitizedData, description: createExcerpt(result.content)
} } so the redirect fields are not present; reference result, result.data,
createExcerpt, redirect_from and redirectFrom when making the change.
2 participants
Summary by CodeRabbit
Release Notes