Skip to content

Conversation

@birkskyum
Copy link
Member

@birkskyum birkskyum commented Nov 3, 2025

Summary by CodeRabbit

  • Tests
    • Enhanced link navigation tests by adding explicit waits for URL updates to complete before assertions. Tests now ensure search parameters are properly reflected before proceeding, improving reliability and preventing flaky test behavior.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 3, 2025

Walkthrough

Test files in React Router and Solid Router packages were modified to convert synchronous click events to awaited interactions with explicit waitFor assertions. These changes ensure tests wait for URL navigation and search parameter updates to complete before proceeding with subsequent assertions.

Changes

Cohort / File(s) Summary
Link navigation test synchronization
packages/react-router/tests/link.test.tsx, packages/solid-router/tests/link.test.tsx
Converted synchronous fireEvent click calls to awaited interactions followed by waitFor assertions that verify window.location.search reflects updated query parameters before proceeding with further assertions

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • The changes follow a consistent, repetitive pattern applied to both test files (synchronous → asynchronous with waitFor guards)
  • Both modifications are straightforward conversions without complex new logic
  • The homogeneous nature of changes across files reduces review complexity

Possibly related PRs

Suggested reviewers

  • schiller-manuel
  • SeanCassiere

Poem

🐰 Click, click, await—no race condition here!
Navigation guards that make intent clear,
Link tests now pause for the URL to bloom,
Deterministic dancers in the testing room! 🎭✨

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly describes the main change across both modified test files: adding explicit waits for navigation completion before asserting on URL updates.
✨ 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 test(router)--wait-for-nav-to-complete

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.

❤️ Share

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

@nx-cloud
Copy link

nx-cloud bot commented Nov 3, 2025

View your CI Pipeline Execution ↗ for commit 17be63c

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

☁️ Nx Cloud last updated this comment at 2025-11-03 19:15:19 UTC

@pkg-pr-new
Copy link

pkg-pr-new bot commented Nov 3, 2025

More templates

@tanstack/arktype-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/arktype-adapter@5744

@tanstack/directive-functions-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/directive-functions-plugin@5744

@tanstack/eslint-plugin-router

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

@tanstack/history

npm i https://pkg.pr.new/TanStack/router/@tanstack/history@5744

@tanstack/nitro-v2-vite-plugin

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

@tanstack/react-router

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

@tanstack/react-router-devtools

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

@tanstack/react-router-ssr-query

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

@tanstack/react-start

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start@5744

@tanstack/react-start-client

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start-client@5744

@tanstack/react-start-server

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start-server@5744

@tanstack/router-cli

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

@tanstack/router-core

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

@tanstack/router-devtools

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

@tanstack/router-devtools-core

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

@tanstack/router-generator

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

@tanstack/router-plugin

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

@tanstack/router-ssr-query-core

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

@tanstack/router-utils

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

@tanstack/router-vite-plugin

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

@tanstack/server-functions-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/server-functions-plugin@5744

@tanstack/solid-router

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

@tanstack/solid-router-devtools

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

@tanstack/solid-router-ssr-query

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

@tanstack/solid-start

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start@5744

@tanstack/solid-start-client

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start-client@5744

@tanstack/solid-start-server

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start-server@5744

@tanstack/start-client-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-client-core@5744

@tanstack/start-plugin-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-plugin-core@5744

@tanstack/start-server-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-server-core@5744

@tanstack/start-static-server-functions

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

@tanstack/start-storage-context

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-storage-context@5744

@tanstack/valibot-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/valibot-adapter@5744

@tanstack/virtual-file-routes

npm i https://pkg.pr.new/TanStack/router/@tanstack/virtual-file-routes@5744

@tanstack/zod-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/zod-adapter@5744

commit: 17be63c

Copy link
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: 0

🧹 Nitpick comments (1)
packages/solid-router/tests/link.test.tsx (1)

778-783: LGTM! Explicit navigation wait improves test reliability.

The addition of waitFor to check that window.location.search updates correctly ensures the navigation completes before subsequent assertions. This makes the test more robust and explicit about what it's waiting for.

Minor note: In @solidjs/testing-library, fireEvent methods are synchronous and return void, so the await on line 778 doesn't actually wait for anything. However, it's harmless and the waitFor block is doing the real work of ensuring navigation completes.

If you want to eliminate the redundancy, you could remove the duplicate check at line 790 since it's already verified in the waitFor block above.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 219b72c and 17be63c.

📒 Files selected for processing (2)
  • packages/react-router/tests/link.test.tsx (2 hunks)
  • packages/solid-router/tests/link.test.tsx (2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use TypeScript in strict mode with extensive type safety across the codebase

Files:

  • packages/react-router/tests/link.test.tsx
  • packages/solid-router/tests/link.test.tsx
packages/{react-router,solid-router}/**

📄 CodeRabbit inference engine (AGENTS.md)

Implement React and Solid bindings/components only in packages/react-router/ and packages/solid-router/

Files:

  • packages/react-router/tests/link.test.tsx
  • packages/solid-router/tests/link.test.tsx
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to packages/{react-router,solid-router}/** : Implement React and Solid bindings/components only in packages/react-router/ and packages/solid-router/
Learnt from: nlynzaad
Repo: TanStack/router PR: 5402
File: packages/router-generator/tests/generator/no-formatted-route-tree/routeTree.nonnested.snapshot.ts:19-21
Timestamp: 2025-10-08T08:11:47.088Z
Learning: Test snapshot files in the router-generator tests directory (e.g., files matching the pattern `packages/router-generator/tests/generator/**/routeTree*.snapshot.ts` or `routeTree*.snapshot.js`) should not be modified or have issues flagged, as they are fixtures used to verify the generator's output and are intentionally preserved as-is.
📚 Learning: 2025-10-08T08:11:47.088Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5402
File: packages/router-generator/tests/generator/no-formatted-route-tree/routeTree.nonnested.snapshot.ts:19-21
Timestamp: 2025-10-08T08:11:47.088Z
Learning: Test snapshot files in the router-generator tests directory (e.g., files matching the pattern `packages/router-generator/tests/generator/**/routeTree*.snapshot.ts` or `routeTree*.snapshot.js`) should not be modified or have issues flagged, as they are fixtures used to verify the generator's output and are intentionally preserved as-is.

Applied to files:

  • packages/react-router/tests/link.test.tsx
  • packages/solid-router/tests/link.test.tsx
📚 Learning: 2025-09-23T17:36:12.598Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to packages/{react-router,solid-router}/** : Implement React and Solid bindings/components only in packages/react-router/ and packages/solid-router/

Applied to files:

  • packages/solid-router/tests/link.test.tsx
📚 Learning: 2025-10-01T18:30:26.591Z
Learnt from: schiller-manuel
Repo: TanStack/router PR: 5330
File: packages/router-core/src/router.ts:2231-2245
Timestamp: 2025-10-01T18:30:26.591Z
Learning: In `packages/router-core/src/router.ts`, the `resolveRedirect` method intentionally strips the router's origin from redirect URLs when they match (e.g., `https://foo.com/bar` → `/bar` for same-origin redirects) while preserving the full URL for cross-origin redirects. This logic should not be removed or simplified to use `location.publicHref` directly.

Applied to files:

  • packages/solid-router/tests/link.test.tsx
⏰ 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). (2)
  • GitHub Check: Test
  • GitHub Check: Preview
🔇 Additional comments (3)
packages/react-router/tests/link.test.tsx (2)

751-754: LGTM! Proper synchronization for async navigation.

The waitFor block correctly ensures navigation completes and search parameters are updated before proceeding with assertions. This prevents potential race conditions and improves test reliability.


868-871: LGTM! Consistent navigation synchronization.

The waitFor block correctly ensures navigation completes before assertions run. This matches the pattern used in the previous test and maintains consistency across similar test scenarios.

packages/solid-router/tests/link.test.tsx (1)

896-901: LGTM! Consistent with the previous navigation wait.

This follows the same reliable pattern as the earlier change (lines 778-783), explicitly waiting for the search parameters to update after navigation. The consistency across both test cases is good.

Same minor notes apply: the await on fireEvent.click is unnecessary (but harmless), and there's a redundant check at line 906.

@birkskyum birkskyum merged commit 2d338e0 into main Nov 3, 2025
6 checks passed
@birkskyum birkskyum deleted the test(router)--wait-for-nav-to-complete branch November 3, 2025 19:24
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.

2 participants