Uh oh!
There was an error while loading. Please reload this page.
fix: restore InferStructuralSharing and handleHashScroll in published .d.ts files - #5134
Merged
Merged
Conversation
Contributor
WalkthroughDocumentation annotations were updated from @internal to @Private in two TypeScript files. No code, types, signatures, or control flow changed. The updates affect visibility in generated docs only and address missing published type metadata without altering runtime or type behavior. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Pre-merge checks (4 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"mode: "warning"instructions: | Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing touches
🧪 Generate unit tests
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 |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/router-core/src/scroll-restoration.ts (1)
311-314: Throttle handler capture: avoid creating a new throttled function on each setup.Minor: storing the throttled listener in a const avoids accidental multiple registrations with different closures if
setupScrollRestorationis called again.- if (typeof document !== 'undefined') {- document.addEventListener('scroll', throttle(onScroll, 100), true)- }+ if (typeof document !== 'undefined') {+ const onScrollThrottled = throttle(onScroll, 100)+ document.addEventListener('scroll', onScrollThrottled, true)+ // Consider returning a disposer to remove the listener on teardown.+ }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/react-router/src/typePrimitives.ts(1 hunks)packages/router-core/src/scroll-restoration.ts(1 hunks)
🔇 Additional comments (4)
packages/router-core/src/scroll-restoration.ts (2)
349-355: Switching @internal → @Private correctly prevents stripInternal from removing this export.Good call—TypeScript only strips
@internal, so@privatepreserves the declaration in published .d.ts files without changing runtime behavior.
361-364: No change needed — router.state.location.hash is already normalized (no leading '#')router.parseLocation normalizes HistoryLocation.hash with hash.split('#').reverse()[0] (packages/router-core/src/router.ts), and restoreScroll already strips window.location.hash; handleHashScroll uses the parsed location.hash (no leading '#'), so the suggested change is unnecessary.
Likely an incorrect or invalid review comment.
packages/react-router/src/typePrimitives.ts (2)
34-36: @internal → @Private restores InferStructuralSharing in published .d.ts.This aligns with
stripInternal: truewhile keeping the type available to consumers. No API or type changes—LGTM.
34-41: Add CI guard to ensure exported types are present in published .d.tsFail CI if key exports are missing after
pnpm -w build. Location: packages/react-router/src/typePrimitives.ts (lines 34–41).#!/bin/bashset -euo pipefail pnpm -w build # ensure .d.ts files existif! find . -type f -name '*.d.ts' -not -path './node_modules/*' -print -quit | grep -q .;thenecho"No .d.ts files found after build"exit 1 fi# verify exported symbols are present in built d.tsif! find . -type f -name '*.d.ts' -not -path './node_modules/*' -exec grep -En "export[^;]*InferStructuralSharing" {} + >/dev/null 2>&1;thenecho"InferStructuralSharing missing in published d.ts"exit 1 fiif! find . -type f -name '*.d.ts' -not -path './node_modules/*' -exec grep -En "export[^;]*handleHashScroll" {} + >/dev/null 2>&1;thenecho"handleHashScroll missing in published d.ts"exit 1 fi# catch accidental @internal annotations in sourceif find packages/router-core/src packages/react-router/src -type f -exec grep -En '@internal' {} + >/dev/null 2>&1;thenecho"Found @internal in source; ensure stripInternal is enabled for the build"exit 1 fi
Contributor
View your CI Pipeline Execution ↗ for commit b495d1a
☁️ Nx Cloud last updated this comment at |
More templates
@tanstack/arktype-adapter@tanstack/directive-functions-plugin@tanstack/eslint-plugin-router@tanstack/history@tanstack/react-router@tanstack/react-router-devtools@tanstack/react-router-ssr-query@tanstack/react-start@tanstack/react-start-client@tanstack/react-start-plugin@tanstack/react-start-server@tanstack/router-cli@tanstack/router-core@tanstack/router-devtools@tanstack/router-devtools-core@tanstack/router-generator@tanstack/router-plugin@tanstack/router-ssr-query-core@tanstack/router-utils@tanstack/router-vite-plugin@tanstack/server-functions-plugin@tanstack/solid-router@tanstack/solid-router-devtools@tanstack/solid-start@tanstack/solid-start-client@tanstack/solid-start-plugin@tanstack/solid-start-server@tanstack/start-client-core@tanstack/start-plugin-core@tanstack/start-server-core@tanstack/start-server-functions-client@tanstack/start-server-functions-fetcher@tanstack/start-server-functions-server@tanstack/start-storage-context@tanstack/valibot-adapter@tanstack/virtual-file-routes@tanstack/zod-adaptercommit: |
Sheraff
commented
Sep 15, 2025
Collaborator
@vedant416 out of curiosity, what is your use-case for |
Uh oh!
There was an error while loading. Please reload this page.
vedant416
commented
Sep 15, 2025
ContributorAuthor
@Sheraff the original reporter of issue #5116, @perbergland, might be able to share more about their use case. |
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
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Fixes#5116
Root Cause
In PR #4907, the TypeScript compiler option
stripInternalwas enabled in tsconfig.json, which causes TypeScript to remove any declarations marked with@internalfrom the published.d.tsfiles.This resulted in TypeScript compilation errors for library users who have set the TypeScript compiler option
skipLibChecktofalse, because the following members were missing:InferStructuralSharingtype inreact-routerhandleHashScrollfunction inrouter-core > scrollRestorationFix
@internalannotation with the@privateannotation.Summary by CodeRabbit