fix(router-core): strip internal SSR globals from declarations - #8207
fix(router-core): strip internal SSR globals from declarations#8207Sheraff wants to merge 4 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
View your CI Pipeline Execution ↗ for commit 94e3bf0
☁️ Nx Cloud last updated this comment at |
🚀 Changeset Version Preview4 package(s) bumped directly, 10 bumped as dependents. 🟩 Patch bumps
|
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueNo actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughThe change adds an ChangesSSR hydration declarations
Estimated code review effort: 1 (Trivial) | ~2 minutes Merge Risk: ⚪ Minimal · up to This localized declaration-only change hides internal SSR globals from published types without changing runtime behavior, and no actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files. Full details: Description checkExplanation The description explains the change and testing status, but it does not use the required Changes, Checklist, and Release Impact sections. It also omits the required checklist items and release-impact selection. Resolution Rewrite the description to include the required ## 🎯 Changes, ## ✅ Checklist, and ## 🚀 Release Impact sections. Describe the change under Changes, complete each checklist item, and select the appropriate release-impact option. If this change affects published code, add a changeset. ✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 88621b3cd8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| // --- SSR hydration (client entry via @tanstack/router-core/ssr/client) --- | ||
|
|
||
| /** @internal */ |
There was a problem hiding this comment.
This changes the published declaration output of @tanstack/router-core, but the commit adds no .changeset entry. Because the release workflow versions and publishes packages selected by changesets, this fix will not independently trigger a router-core release or appear in its changelog; add a patch changeset for the package as required by the repository's contribution workflow.
AGENTS.md reference: AGENTS.md:L3-L5
Useful? React with 👍 / 👎.
Sheraff
changed the title
This reverts commit 2bc6b55.
Sheraff
changed the title
![nx-cloud[bot]](https://avatars.githubusercontent.com/in/80458?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Important
At least one additional CI pipeline execution has run since the conclusion below was written and it may no longer be applicable.
Nx Cloud is proposing a fix for your failed CI:
We add a local /** @internal */ declare global { interface Window { $_TSR?: TsrSsrGlobal } } block to each framework's RouterClient.tsx and to solid-router's setupTests.tsx to fix the downstream TS2339 build failures. The PR marked the Window augmentation in router-core as @internal, which caused it to be stripped from the published declarations by stripInternal, removing $_TSR from the Window type for all consumers. By re-declaring the augmentation locally—also marked @internal—the internal SSR files retain correct types without exposing the global in any package's published declarations.
Tip
✅ We verified this fix by re-running @tanstack/vue-router:build, @tanstack/react-router:build, @tanstack/solid-router:build and 1 more.
Suggested Fix changes
diff --git a/packages/react-router/src/ssr/RouterClient.tsx b/packages/react-router/src/ssr/RouterClient.tsx
index 69abc97f..7d8a3800 100644
--- a/packages/react-router/src/ssr/RouterClient.tsx
+++ b/packages/react-router/src/ssr/RouterClient.tsx
@@ -1,8 +1,15 @@
-import { hydrate } from '@tanstack/router-core/ssr/client'
+import { hydrate, type TsrSsrGlobal } from '@tanstack/router-core/ssr/client'
import { Await } from '../awaited'
import { RouterProvider } from '../RouterProvider'
import type { AnyRouter } from '@tanstack/router-core'
+/** @internal */
+declare global {
+ interface Window {
+ $_TSR?: TsrSsrGlobal
+ }
+}
+
let hydrationPromise: Promise<void> | undefined
export function RouterClient(props: { router: AnyRouter }) {
diff --git a/packages/solid-router/src/ssr/RouterClient.tsx b/packages/solid-router/src/ssr/RouterClient.tsx
index c2389495..a8028e23 100644
--- a/packages/solid-router/src/ssr/RouterClient.tsx
+++ b/packages/solid-router/src/ssr/RouterClient.tsx
@@ -1,10 +1,17 @@
-import { hydrate } from '@tanstack/router-core/ssr/client'
+import { hydrate, type TsrSsrGlobal } from '@tanstack/router-core/ssr/client'
import { Await } from '../awaited'
import { HeadContent } from '../HeadContent'
import { RouterProvider } from '../RouterProvider'
import type { AnyRouter } from '@tanstack/router-core'
import type { JSXElement } from 'solid-js'
+/** @internal */
+declare global {
+ interface Window {
+ $_TSR?: TsrSsrGlobal
+ }
+}
+
let hydrationPromise: Promise<void> | undefined
const Dummy = (props: { children?: JSXElement }) => <>{props.children}</>
diff --git a/packages/solid-router/tests/setupTests.tsx b/packages/solid-router/tests/setupTests.tsx
index c3d7ec5c..4bee6624 100644
--- a/packages/solid-router/tests/setupTests.tsx
+++ b/packages/solid-router/tests/setupTests.tsx
@@ -1,5 +1,12 @@
import '@testing-library/jest-dom/vitest'
import { vi } from 'vitest'
+import type { TsrSsrGlobal } from '@tanstack/router-core/ssr/client'
+
+declare global {
+ interface Window {
+ $_TSR?: TsrSsrGlobal
+ }
+}
// @ts-expect-error
global.IS_REACT_ACT_ENVIRONMENT = true
diff --git a/packages/vue-router/src/ssr/RouterClient.tsx b/packages/vue-router/src/ssr/RouterClient.tsx
index b21f0fc9..d8c48f18 100644
--- a/packages/vue-router/src/ssr/RouterClient.tsx
+++ b/packages/vue-router/src/ssr/RouterClient.tsx
@@ -3,6 +3,14 @@ import { hydrate } from '@tanstack/router-core/ssr/client'
import { HeadContent } from '../HeadContent'
import { RouterProvider } from '../RouterProvider'
import type { AnyRouter } from '@tanstack/router-core'
+import type { TsrSsrGlobal } from '@tanstack/router-core/ssr/client'
+
+/** @internal */
+declare global {
+ interface Window {
+ $_TSR?: TsrSsrGlobal
+ }
+}
let hydrationPromise: Promise<void> | undefined
Or Apply changes locally with:
npx nx-cloud apply-locally 3eNU-m32g
Apply fix locally with your editor ↗ View interactive diff ↗
🎓 Learn more about Self-Healing CI on nx.dev
Merging this PR will degrade performance by 5.17%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Memory | mem server error-paths redirect (solid) |
368.7 KB | 916.8 KB | -59.79% |
| ❌ | Memory | mem client unique-location-churn (vue) |
475.7 KB | 971.8 KB | -51.05% |
| ❌ | Memory | mem server serialization-payload (react) |
4.2 MB | 6 MB | -30.34% |
| ❌ | Memory | mem server aborted-requests (solid) |
1.2 MB | 1.3 MB | -5.61% |
| ❌ | Simulation | client-async-pipeline navigation loop (react) |
61.9 ms | 65 ms | -4.84% |
| ❌ | Memory | mem server error-paths unmatched (react) |
441.5 KB | 460.5 KB | -4.12% |
| ❌ | Memory | mem server error-paths unmatched (vue) |
577.5 KB | 600.2 KB | -3.79% |
| ❌ | Memory | mem server request-churn (react) |
639.3 KB | 660.4 KB | -3.19% |
| ⚡ | Memory | mem server error-paths redirect (vue) |
891.9 KB | 501.6 KB | +77.83% |
| ⚡ | Memory | mem server error-paths not-found (solid) |
698.7 KB | 555.3 KB | +25.83% |
| ⚡ | Memory | mem client preload-churn (vue) |
919.3 KB | 760.5 KB | +20.89% |
| ⚡ | Memory | mem client navigation-churn (vue) |
1.7 MB | 1.5 MB | +11.11% |
| ⚡ | Simulation | client-nested-params navigation loop (react) |
151.2 ms | 137 ms | +10.37% |
| ⚡ | Memory | mem server error-paths redirect (react) |
315.9 KB | 292.3 KB | +8.07% |
| ⚡ | Simulation | ssr global-mw document (solid) |
366.2 ms | 349.3 ms | +4.83% |
| ⚡ | Simulation | ssr server-fn multipart (solid) |
144.3 ms | 139.6 ms | +3.4% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing chore/internal-ssr-window-globals (94e3bf0) with main (37877da)
Bundle Size BenchmarksThis pull request does not affect bundle size in any measured scenario. |
1 participant
Summary
Windowaugmentation as internalTesting
Not run; CI will validate the change.