Skip to content

Conversation

Christopher96u
Copy link
Contributor

@Christopher96u Christopher96u commented Oct 9, 2025

Fixes #3876

Summary by CodeRabbit

  • Documentation
    • Clarified blocker semantics: returning true blocks navigation; false allows it, improving understanding for React usage.
  • Refactor
    • Introduced a clearer public type for navigation blocking and deprecated the previous allow-based alias. This is non-breaking and preserves existing behavior.
  • Chores
    • Added inline comments to clarify that a truthy blocker response prevents navigation in both history creation paths.

@github-actions github-actions bot added documentation Everything documentation related package: history labels Oct 9, 2025
Copy link
Contributor

coderabbitai bot commented Oct 9, 2025

Walkthrough

Clarifies navigation blocker semantics: documentation now states that returning true blocks navigation and false allows it. In code, introduces ShouldBlockNavigation type, deprecates ShouldAllowNavigation as an alias, and updates BlockerFn to return boolean|Promise representing “should block” without changing runtime behavior.

Changes

Cohort / File(s) Summary of modifications
Docs: React Router useBlockerHook
docs/router/framework/react/api/router/useBlockerHook.md
Updated wording to state that returning true blocks navigation and false allows it, aligning docs with actual behavior.
History types and comments
packages/history/src/index.ts
Added ShouldBlockNavigation type; deprecated ShouldAllowNavigation as alias; updated BlockerFn return type to use block semantics; added inline comments clarifying truthy means “block” across history creation paths.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I twitch my ears at types that mock,
Now truth is clear: true means block.
Old names hop off, new names hop in,
Docs align—no spin, just win.
Onward I bound through changelog dew,
Booleans sorted, carrots too. 🥕🐇

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ 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 clearly indicates that the documentation and associated code in the history module are being updated to clarify the boolean contract of the blockerFn API, matching the primary change in the PR.
Linked Issues Check ✅ Passed The PR directly addresses issue #3876 by clarifying the blockerFn boolean behavior in documentation and renaming the type to ShouldBlockNavigation to align with its actual semantics, satisfying the issue’s proposed resolutions.
Out of Scope Changes Check ✅ Passed All modifications in the PR are focused on updating documentation and types related to the blockerFn boolean contract in the history package, with no unrelated or extraneous changes introduced.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 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 3097080 and 7c2f75b.

📒 Files selected for processing (2)
  • docs/router/framework/react/api/router/useBlockerHook.md (1 hunks)
  • packages/history/src/index.ts (4 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
docs/**/*.{md,mdx}

📄 CodeRabbit inference engine (AGENTS.md)

Use internal docs links relative to the docs/ folder (e.g., ./guide/data-loading)

Files:

  • docs/router/framework/react/api/router/useBlockerHook.md
docs/{router,start}/**

📄 CodeRabbit inference engine (AGENTS.md)

Place router docs under docs/router/ and start framework docs under docs/start/

Files:

  • docs/router/framework/react/api/router/useBlockerHook.md
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

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

Files:

  • packages/history/src/index.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). (2)
  • GitHub Check: Preview
  • GitHub Check: Test
🔇 Additional comments (5)
packages/history/src/index.ts (4)

76-78: LGTM!

The BlockerFn return type is correctly updated to use ShouldBlockNavigation, maintaining consistency with the type rename.


157-157: LGTM!

The clarifying comment accurately documents the blocking semantics and improves code maintainability.


446-446: LGTM!

The clarifying comment is consistent with the one at line 157 and accurately documents the blocking semantics for the browser history code path.


61-66: Excellent type rename with backward compatibility!

No references to ShouldAllowNavigation outside its alias; the deprecation alias ensures backward compatibility.

docs/router/framework/react/api/router/useBlockerHook.md (1)

60-60: LGTM!

The documentation update clearly states the blocking semantics, eliminating the confusion reported in issue #3876. The explicit parenthetical clarification (true blocks, false allows) leaves no room for misinterpretation.


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.

Copy link

nx-cloud bot commented Oct 9, 2025

View your CI Pipeline Execution ↗ for commit 7c2f75b

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

☁️ Nx Cloud last updated this comment at 2025-10-09 06:12:57 UTC

Copy link

pkg-pr-new bot commented Oct 9, 2025

More templates

@tanstack/arktype-adapter

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

@tanstack/directive-functions-plugin

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

@tanstack/eslint-plugin-router

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

@tanstack/history

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

@tanstack/nitro-v2-vite-plugin

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

@tanstack/react-router

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

@tanstack/react-router-devtools

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

@tanstack/react-router-ssr-query

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

@tanstack/react-start

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

@tanstack/react-start-client

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

@tanstack/react-start-server

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

@tanstack/router-cli

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

@tanstack/router-core

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

@tanstack/router-devtools

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

@tanstack/router-devtools-core

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

@tanstack/router-generator

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

@tanstack/router-plugin

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

@tanstack/router-ssr-query-core

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

@tanstack/router-utils

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

@tanstack/router-vite-plugin

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

@tanstack/server-functions-plugin

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

@tanstack/solid-router

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

@tanstack/solid-router-devtools

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

@tanstack/solid-start

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

@tanstack/solid-start-client

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

@tanstack/solid-start-server

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

@tanstack/start-client-core

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

@tanstack/start-plugin-core

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

@tanstack/start-server-core

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

@tanstack/start-static-server-functions

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

@tanstack/start-storage-context

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

@tanstack/valibot-adapter

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

@tanstack/virtual-file-routes

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

@tanstack/zod-adapter

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

commit: 7c2f75b

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

Labels

documentation Everything documentation related package: history

Projects

None yet

Development

Successfully merging this pull request may close these issues.

blockerFn in router.history.block() returns true to block navigation — somewhat confusing

1 participant