Skip to content

chore: stabilize tests#7503

Merged
schiller-manuel merged 2 commits into
mainfrom
stabilize-tests-4
May 29, 2026
Merged

chore: stabilize tests#7503
schiller-manuel merged 2 commits into
mainfrom
stabilize-tests-4

Conversation

@schiller-manuel
Copy link
Copy Markdown
Collaborator

@schiller-manuel schiller-manuel commented May 28, 2026

Summary by CodeRabbit

  • Tests
    • Improved end-to-end test synchronization across React, Solid, and Vue by using router "idle/status" indicators instead of network idle.
    • Added shared helpers to wait for router idle and for the posts index to be ready, making redirect and navigation tests more reliable and consistent.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 28, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2cdb7a33-b534-4cd4-a901-4c6dc9b88b6e

📥 Commits

Reviewing files that changed from the base of the PR and between 7e1c594 and 9b573d7.

📒 Files selected for processing (3)
  • e2e/react-start/basic/tests/redirect.spec.ts
  • e2e/solid-start/basic/tests/redirect.spec.ts
  • e2e/vue-start/basic/tests/redirect.spec.ts
🚧 Files skipped from review as they are similar to previous changes (3)
  • e2e/react-start/basic/tests/redirect.spec.ts
  • e2e/vue-start/basic/tests/redirect.spec.ts
  • e2e/solid-start/basic/tests/redirect.spec.ts

📝 Walkthrough

Walkthrough

Expose router {isLoading,status} in hidden ClientOnly DOM regions for React, Solid, and Vue roots; update redirect e2e tests to add POSTS_URL and waitForRouterIdle/waitForPostsIndex helpers and replace networkidle waits with router-state-based synchronization across redirect scenarios.

Changes

E2E Router State Exposure and Test Synchronization

Layer / File(s) Summary
React router state exposure
e2e/react-start/basic/src/routes/__root.tsx
React root imports useRouterState, selects { isLoading, status } with structuralSharing and renders them hidden with data-testid attributes.
React test router-idle synchronization
e2e/react-start/basic/tests/redirect.spec.ts
Adds POSTS_URL, waitForRouterIdle, and waitForPostsIndex helpers; replaces page.waitForLoadState('networkidle') with router-state waits across internal, external, serverFn, and useServerFn redirect tests.
Solid router state exposure
e2e/solid-start/basic/src/routes/__root.tsx
Solid root imports useRouterState, selects { isLoading, status }, and exposes them in a hidden ClientOnly block with test IDs.
Solid test router-idle synchronization
e2e/solid-start/basic/tests/redirect.spec.ts
Adds POSTS_URL and wait helpers; replaces networkidle waits with router-state assertions and adds conditional router-idle waits for internal posts navigation.
Vue router state exposure
e2e/vue-start/basic/src/routes/__root.tsx
Vue root adds ClientOnly and useRouterState, selects { isLoading, status }, and renders them in a hidden test-ID-labeled DOM region.
Vue test router-idle synchronization
e2e/vue-start/basic/tests/redirect.spec.ts
Adds POSTS_URL and shared helpers for router idle and posts index readiness; replaces networkidle waits with router-state-based synchronization across redirect scenarios.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested labels

package: react-start, package: solid-start, package: vue-start

Poem

A rabbit peeks where routers hide their beat,
Hops through React, Solid, Vue with nimble feet,
Adds tiny tests that read the loading sign,
Now redirects wait on the router's line—
🐰✨ quiet, precise, and sweet.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'chore: stabilize tests' accurately describes the main objective of the changeset, which implements router state synchronization helpers across multiple test files to improve test reliability.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch stabilize-tests-4

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

@nx-cloud
Copy link
Copy Markdown
Contributor

nx-cloud Bot commented May 28, 2026

View your CI Pipeline Execution ↗ for commit 7e1c594

Command Status Duration Result
nx affected --targets=test:eslint,test:unit,tes... ✅ Succeeded 6m 59s View ↗
nx run-many --target=build --exclude=examples/*... ✅ Succeeded 2m 5s View ↗

☁️ Nx Cloud last updated this comment at 2026-05-29 00:07:08 UTC

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 28, 2026

🚀 Changeset Version Preview

No changeset entries found. Merging this PR will not cause a version bump for any packages.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@e2e/vue-start/basic/src/routes/__root.tsx`:
- Around line 71-73: The select callback passed to useRouterState lacks an
explicit type for its parameter; update the callback to annotate the parameter
(e.g., change select: (state) => ... to select: (state: RouterState) => ...) and
import the correct RouterState (or equivalent) type from `@tanstack/vue-router`
(or replace RouterState with the actual exported type name) so the select
function is fully typed under TS strict mode.

In `@e2e/vue-start/basic/tests/redirect.spec.ts`:
- Around line 24-29: In function waitForPostsIndex, fix the inconsistent
indentation of the await waitForRouterIdle(page) statement (currently
over-indented) so it matches the other statements in the function body; adjust
whitespace to use the same 2-space indentation for all lines inside
waitForPostsIndex (keeping the existing await page.waitForURL, expect, and await
expect lines intact) to restore consistent formatting.
🪄 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: 0615f5f9-aa1e-4eae-a3e4-45344dfd3c4a

📥 Commits

Reviewing files that changed from the base of the PR and between c79b396 and 7e1c594.

📒 Files selected for processing (6)
  • e2e/react-start/basic/src/routes/__root.tsx
  • e2e/react-start/basic/tests/redirect.spec.ts
  • e2e/solid-start/basic/src/routes/__root.tsx
  • e2e/solid-start/basic/tests/redirect.spec.ts
  • e2e/vue-start/basic/src/routes/__root.tsx
  • e2e/vue-start/basic/tests/redirect.spec.ts

Comment on lines +71 to +73
const routerState = useRouterState({
select: (state) => ({ isLoading: state.isLoading, status: state.status }),
})
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 29, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add explicit type annotation to the select function parameter.

The state parameter in the select callback lacks an explicit type annotation. Under TypeScript strict mode, this may lead to type safety issues or compiler warnings. As per coding guidelines, TypeScript files should use strict mode with extensive type safety.

🔧 Proposed fix
-  const routerState = useRouterState({
-    select: (state) => ({ isLoading: state.isLoading, status: state.status }),
-  })
+  const routerState = useRouterState({
+    select: (state: RouterState) => ({ isLoading: state.isLoading, status: state.status }),
+  })

Note: Replace RouterState with the actual type exported by @tanstack/vue-router if it differs.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@e2e/vue-start/basic/src/routes/__root.tsx` around lines 71 - 73, The select
callback passed to useRouterState lacks an explicit type for its parameter;
update the callback to annotate the parameter (e.g., change select: (state) =>
... to select: (state: RouterState) => ...) and import the correct RouterState
(or equivalent) type from `@tanstack/vue-router` (or replace RouterState with the
actual exported type name) so the select function is fully typed under TS strict
mode.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

useRouterState is, as all other APIs from TanStack Router and Start fully typed. no need for any explicit types

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@schiller-manuel, you're right — useRouterState's select callback is fully typed via the library's generics, so TypeScript infers the state parameter type automatically. No explicit annotation is needed. I'll note this for future reviews.

(ᵕᴗᵕ)っ🐰


✏️ Learnings added
Learnt from: schiller-manuel
Repo: TanStack/router PR: 7503
File: e2e/vue-start/basic/src/routes/__root.tsx:71-73
Timestamp: 2026-05-29T00:17:44.833Z
Learning: In TanStack Router and TanStack Start, all APIs including `useRouterState` are fully typed. The `select` callback parameter (e.g., `select: (state) => ...`) has its type automatically inferred by TypeScript from the library's generic definitions — no explicit type annotation is needed. Avoid flagging missing type annotations on `useRouterState` select callbacks in TanStack Router/Start projects.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: nlynzaad
Repo: TanStack/router PR: 6389
File: e2e/solid-start/basic/src/routes/specialChars/malformed/route.tsx:11-24
Timestamp: 2026-01-18T17:36:17.282Z
Learning: In TanStack/router e2e tests, intentionally using href instead of to on Link components (e.g., in routes like e2e/solid-start/basic/src/routes/specialChars/malformed/route.tsx) can be used to validate encoding/decoding and edge-case behavior. This is a test pattern and should not appear in production code. When reviewing, verify that such usage is confined to test files, clearly documented as test-only, and that it exercises edge cases without impacting actual navigation behavior in the app.

Comment thread e2e/vue-start/basic/tests/redirect.spec.ts
@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented May 29, 2026

More templates

@tanstack/arktype-adapter

npm i https://pkg.pr.new/@tanstack/arktype-adapter@7503

@tanstack/eslint-plugin-router

npm i https://pkg.pr.new/@tanstack/eslint-plugin-router@7503

@tanstack/eslint-plugin-start

npm i https://pkg.pr.new/@tanstack/eslint-plugin-start@7503

@tanstack/history

npm i https://pkg.pr.new/@tanstack/history@7503

@tanstack/nitro-v2-vite-plugin

npm i https://pkg.pr.new/@tanstack/nitro-v2-vite-plugin@7503

@tanstack/react-router

npm i https://pkg.pr.new/@tanstack/react-router@7503

@tanstack/react-router-devtools

npm i https://pkg.pr.new/@tanstack/react-router-devtools@7503

@tanstack/react-router-ssr-query

npm i https://pkg.pr.new/@tanstack/react-router-ssr-query@7503

@tanstack/react-start

npm i https://pkg.pr.new/@tanstack/react-start@7503

@tanstack/react-start-client

npm i https://pkg.pr.new/@tanstack/react-start-client@7503

@tanstack/react-start-rsc

npm i https://pkg.pr.new/@tanstack/react-start-rsc@7503

@tanstack/react-start-server

npm i https://pkg.pr.new/@tanstack/react-start-server@7503

@tanstack/router-cli

npm i https://pkg.pr.new/@tanstack/router-cli@7503

@tanstack/router-core

npm i https://pkg.pr.new/@tanstack/router-core@7503

@tanstack/router-devtools

npm i https://pkg.pr.new/@tanstack/router-devtools@7503

@tanstack/router-devtools-core

npm i https://pkg.pr.new/@tanstack/router-devtools-core@7503

@tanstack/router-generator

npm i https://pkg.pr.new/@tanstack/router-generator@7503

@tanstack/router-plugin

npm i https://pkg.pr.new/@tanstack/router-plugin@7503

@tanstack/router-ssr-query-core

npm i https://pkg.pr.new/@tanstack/router-ssr-query-core@7503

@tanstack/router-utils

npm i https://pkg.pr.new/@tanstack/router-utils@7503

@tanstack/router-vite-plugin

npm i https://pkg.pr.new/@tanstack/router-vite-plugin@7503

@tanstack/solid-router

npm i https://pkg.pr.new/@tanstack/solid-router@7503

@tanstack/solid-router-devtools

npm i https://pkg.pr.new/@tanstack/solid-router-devtools@7503

@tanstack/solid-router-ssr-query

npm i https://pkg.pr.new/@tanstack/solid-router-ssr-query@7503

@tanstack/solid-start

npm i https://pkg.pr.new/@tanstack/solid-start@7503

@tanstack/solid-start-client

npm i https://pkg.pr.new/@tanstack/solid-start-client@7503

@tanstack/solid-start-server

npm i https://pkg.pr.new/@tanstack/solid-start-server@7503

@tanstack/start-client-core

npm i https://pkg.pr.new/@tanstack/start-client-core@7503

@tanstack/start-fn-stubs

npm i https://pkg.pr.new/@tanstack/start-fn-stubs@7503

@tanstack/start-plugin-core

npm i https://pkg.pr.new/@tanstack/start-plugin-core@7503

@tanstack/start-server-core

npm i https://pkg.pr.new/@tanstack/start-server-core@7503

@tanstack/start-static-server-functions

npm i https://pkg.pr.new/@tanstack/start-static-server-functions@7503

@tanstack/start-storage-context

npm i https://pkg.pr.new/@tanstack/start-storage-context@7503

@tanstack/valibot-adapter

npm i https://pkg.pr.new/@tanstack/valibot-adapter@7503

@tanstack/virtual-file-routes

npm i https://pkg.pr.new/@tanstack/virtual-file-routes@7503

@tanstack/vue-router

npm i https://pkg.pr.new/@tanstack/vue-router@7503

@tanstack/vue-router-devtools

npm i https://pkg.pr.new/@tanstack/vue-router-devtools@7503

@tanstack/vue-router-ssr-query

npm i https://pkg.pr.new/@tanstack/vue-router-ssr-query@7503

@tanstack/vue-start

npm i https://pkg.pr.new/@tanstack/vue-start@7503

@tanstack/vue-start-client

npm i https://pkg.pr.new/@tanstack/vue-start-client@7503

@tanstack/vue-start-server

npm i https://pkg.pr.new/@tanstack/vue-start-server@7503

@tanstack/zod-adapter

npm i https://pkg.pr.new/@tanstack/zod-adapter@7503

commit: 9b573d7

@schiller-manuel schiller-manuel merged commit 72805df into main May 29, 2026
10 checks passed
@schiller-manuel schiller-manuel deleted the stabilize-tests-4 branch May 29, 2026 00:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant