Skip to content

fix(navigation): preserve redirecting pages and descendants - #660

Draft
IzumiSy wants to merge 2 commits into
mainfrom
fix/redirect-guard-navigation
Draft

IzumiSy wants to merge 2 commits into
mainfrom
fix/redirect-guard-navigation

Conversation

@IzumiSy

@IzumiSy IzumiSy commented Oct 29, 2026

Copy link
Copy Markdown
Contributor

Motivation

A parent page that returns redirectTo() currently removes its entire subtree from Command Palette entries and automatic sidebar navigation. This makes reachable child pages disappear even though page guards apply only to their own index routes, not to descendants.

Redirect-only pages without a component are also valid navigation entry points, but were treated as non-navigable namespaces. Redirecting to a default child should not require a placeholder component or hide the parent link.

Design Decision

Keep redirect behavior separate from visibility

Only hidden() prunes a navigation subtree. A redirect result makes the page navigable even without a component, while componentless pages returning pass() remain non-clickable. Links retain their original paths so the route guard runs on navigation rather than baking its redirect destination into the menu.

This applies to both shared static navigation and current-path-aware dynamic Palette entries. AutoSidebar's display depth and page-guard inheritance remain unchanged.

Summary

  • Preserve redirecting parents and visible descendants in AutoSidebar and Command Palette entries, including redirect-only root pages, modules, and resources.
  • Add regression coverage for componentful/componentless parents, redirect-only leaves, dynamic routes, pass()/hidden() behavior, and AutoSidebar/Palette integration.
  • Include a patch changeset.
  • Type checking, lint, the full test suite (2,675 core tests), formatting, and the pre-commit documentation check passed. Published-declaration validation timed out during fixture installation. Browser style tests passed (382 tests) before the redirect-only follow-up and were not rerun afterward.

@github-actions

github-actions Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Code Metrics Report

main (1cae270) #660 (6d0bdab) +/-
Coverage 89.5% 89.5% +0.0%
Test Execution Time 2m40s 3m37s +57s
Details
  |                     | main (1cae270) | #660 (6d0bdab) |  +/-  |
  |---------------------|----------------|----------------|-------|
+ | Coverage            |          89.5% |          89.5% | +0.0% |
  |   Files             |            265 |            265 |     0 |
  |   Lines             |           7590 |           7591 |    +1 |
+ |   Covered           |           6799 |           6801 |    +2 |
- | Test Execution Time |          2m40s |          3m37s |  +57s |

Code coverage of files in pull request scope (94.4% → 94.7%, patch 100.0%)

Files Coverage +/- Patch Coverage Status
packages/core/src/components/command-palette/command-palette.tsx 92.8% 0.0% - modified
packages/core/src/components/internals/breadcrumb/breadcrumb.tsx 87.5% +12.5% - affected
packages/core/src/routing/navigation.tsx 100.0% 0.0% 100.0% modified

Reported by octocov

Copilot AI left a comment

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.

🟡 Changes recommended

Static navigation exposes redirecting paths with embedded dynamic parameters and their descendants as sidebar and Palette links containing literal placeholders such as :id.

1 open issue
What changed in this PR

This PR separates redirect behavior from navigation visibility so AppShell’s automatic sidebar and Command Palette retain redirecting pages and their visible descendants.

Changes:

  • Keep redirect-only pages navigable at their original paths, including current-path-aware dynamic entries.
  • Add regression tests for redirecting parents, componentless pages, and sidebar/Palette integration.
  • Include a patch changeset describing the behavior change.
File Description
packages/​core/​src/​routing/​navigation.tsx Preserve redirecting entries and descendants in navigation builders.
packages/​core/​src/​routing/​navigation.test.tsx Cover redirects, dynamic paths, and componentless guard outcomes.
packages/​core/​src/​components/​sidebar/​default-sidebar.test.tsx Verify redirecting links and descendant Palette entries together.
packages/​core/​src/​components/​command-palette/​command-palette.tsx Update a comment to reflect componentless navigable pages; no functional change.
.changeset/​tidy-guards-navigate.md Document the navigation fix for a patch release.

Review effort: Balanced


const guardResult = await runGuards(module.guards);
if (guardResult.type !== "pass") return null;
if (guardResult.type === "hidden") return null;
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.

2 participants