Skip to content

Conversation

@tannerlinsley
Copy link
Collaborator

@tannerlinsley tannerlinsley commented Nov 22, 2025

Summary by CodeRabbit

  • Refactor

    • Removed redundant documentation comments across router packages; no runtime behavior changes.
  • Breaking Changes

    • Removed deprecated overloads for blocking/navigation APIs; migrate to the current parameter form.
    • Several previously-exported internal utilities and a public session-type were removed, reducing the public API surface — update integrations to rely on remaining public APIs.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 22, 2025

Walkthrough

This PR removes numerous JSDoc/comment blocks and deletes several exported types, interfaces, functions, and a class across react-router, router-core, solid-router, and start-server-core. No runtime logic changes were made; changes primarily reduce public API surface and clean up documentation.

Changes

Cohort / File(s) Summary
JSDoc Removal (react-router)
packages/react-router/src/ClientOnly.tsx, packages/react-router/src/HeadContent.tsx, packages/react-router/src/Matches.tsx, packages/react-router/src/RouterProvider.tsx, packages/react-router/src/Scripts.tsx, packages/react-router/src/awaited.tsx, packages/react-router/src/fileRoute.ts, packages/react-router/src/router.ts, packages/react-router/src/useLoaderData.tsx, packages/react-router/src/useLoaderDeps.tsx, packages/react-router/src/useLocation.tsx, packages/react-router/src/useParams.tsx, packages/react-router/src/useRouter.tsx, packages/react-router/src/useRouterState.tsx, packages/react-router/src/useSearch.tsx
Removed JSDoc / docblock comments above exported functions/components; implementations and signatures unchanged.
API Change (react-router)
packages/react-router/src/useBlocker.tsx
Removed deprecated positional overload `useBlocker(blockerFn?: LegacyBlockerFn, condition?: boolean
JSDoc Removal (router-core)
packages/router-core/src/qss.ts, packages/router-core/src/redirect.ts, packages/router-core/src/scroll-restoration.ts, packages/router-core/src/searchMiddleware.ts, packages/router-core/src/searchParams.ts
Removed explanatory JSDoc blocks; no behavioral or signature changes.
API Removal (router-core - types)
packages/router-core/src/Matches.ts
Deleted exported interface MatchRouteOptions.
API Removal (router-core - route processing)
packages/router-core/src/new-process-route-tree.ts
Removed exported helpers: parseSegment, parseSegments, createDynamicNode, findFlatMatch, findSingleMatch, and processRouteTree.
API Removal (router-core - path)
packages/router-core/src/path.ts
Removed exported function exactPathTest.
API Removal (router-core - core router)
packages/router-core/src/router.ts
Removed exported getLocationChangeInfo, RouterCore class, lazyFn helper, and getMatchedRoutes function.
API Consolidation (solid-router)
packages/solid-router/src/useBlocker.tsx
Collapsed two deprecated useBlocker overloads into a single unified overload accepting `UseBlockerOpts
Type Removal (start-server-core)
packages/start-server-core/src/session.ts
Removed exported type SealOptions (SealOptionsSub remains).

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

  • Areas needing extra attention:
    • packages/router-core/src/router.ts — removal of RouterCore, getLocationChangeInfo, getMatchedRoutes, and lazyFn.
    • packages/router-core/src/new-process-route-tree.ts — removal of multiple route-processing helpers.
    • Type/API compatibility for useBlocker changes in both react- and solid-router.
    • packages/start-server-core/src/session.ts — verify downstream type usage after SealOptions removal.

Possibly related PRs

Suggested reviewers

  • schiller-manuel
  • SeanCassiere

Poem

🐰 With nimble paws I cleared the lea,
Docblocks swept and types set free.
Old overloads hopped off to rest,
The router trims its vest — and best,
It bounds ahead, all spry and bright. ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'remove extra comments' accurately reflects the primary change across all files—removal of duplicate/redundant JSDoc blocks and documentation comments throughout multiple packages.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/remove-extra-comments

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6ac4af7 and acf36f0.

📒 Files selected for processing (2)
  • packages/react-router/src/fileRoute.ts (0 hunks)
  • packages/router-core/src/scroll-restoration.ts (0 hunks)
💤 Files with no reviewable changes (2)
  • packages/react-router/src/fileRoute.ts
  • packages/router-core/src/scroll-restoration.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Test

Comment @coderabbitai help to get the list of available commands and usage tips.

@nx-cloud
Copy link

nx-cloud bot commented Nov 22, 2025

🤖 Nx Cloud AI Fix Eligible

An automatically generated fix could have helped fix failing tasks for this run, but Self-healing CI is disabled for this workspace. Visit workspace settings to enable it and get automatic fixes in future runs.

To disable these notifications, a workspace admin can disable them in workspace settings.


View your CI Pipeline Execution ↗ for commit acf36f0

Command Status Duration Result
nx affected --targets=test:eslint,test:unit,tes... ❌ Failed 1m 59s View ↗
nx run-many --target=build --exclude=examples/*... ❌ Failed 4s View ↗

☁️ Nx Cloud last updated this comment at 2025-11-22 20:40:08 UTC

* @returns A function that accepts Route options and returns a Route instance.
* @link https://tanstack.com/router/latest/docs/framework/react/api/router/createFileRouteFunction
*/
/**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this still has duplicated jsdocs

@@ -292,18 +276,6 @@
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove

@@ -1,4 +1,3 @@
/* eslint-disable react-hooks/rules-of-hooks */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dont remove this

@@ -247,4 +243,4 @@
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still duplicate

5: number
}

/**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dont remove that!

return output as ParsedSegment
}

/**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dont remove that!

}
}

/**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dont remove that

>(1000)
}

/**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dont remove that

return result
}

/**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dont remove that

return path === '/' ? path : path.replace(/\/{1,}$/, '')
}

/**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dont remove that

cache?: LRUCache<string, string>
}

/**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dont remove that

export type TrailingSlashOption =
(typeof trailingSlashOptions)[keyof typeof trailingSlashOptions]

/**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dont remove that

TDehydrated
>

/**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dont remove that

return normalize(a) === normalize(b)
}

/**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dont remove that

return {}
}

/**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dont remove that

type IntegrityAlgorithm = 'sha256'
/** @internal */
type _Algorithm = EncryptionAlgorithm | IntegrityAlgorithm
/**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dont remove that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants