Fix redirect guard behaviour

[IzumiSy]

See changeset for detail

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/tailor-platform/app-shell/@tailor-platform/app-shell@33

npm i https://pkg.pr.new/tailor-platform/app-shell/@tailor-platform/app-shell-vite-plugin@33

commit: 5888a43

[IzumiSy]

/review

[github-actions]

Generated by API Design Review for issue #33

[github-actions]

[1/2 — Medium] Loader-only index route semantics unclear

The new behavior creates index routes for loader-only modules/resources when source.loader exists but source.component doesn't. However, looking at the codebase, source.loader is only populated by withGuardsLoader() when guards are present.

Issue: The condition source.loader is being used as a proxy for "has guards", which is implicit and fragile. If a module/resource could have a loader without guards in the future, this would incorrectly create an index route with Component: () => null.

Recommendation: Make the intent explicit by checking for guards directly, or document this coupling clearly. Consider:

: (source.loader && hasGuards) // or add a flag like `source.hasGuards`
  ? {
      index: true,
      loader: source.loader,
      Component: () => null,
    }
  : undefined;

Alternatively, add a JSDoc comment explaining that source.loader only exists when guards are present.

[IzumiSy]

Added a JSDoc comment explaining that source.loader is currently only set by withGuardsLoader() when guards are present, and that this condition should be revisited if that coupling changes in the future.

Commit: a9d566c

[github-actions]

[2/2 — High] Navigation filtering change breaks API contract

Breaking change: The navigation filtering logic changed from "only show items where guards pass" (!== "pass") to "show everything except explicitly hidden items" (=== "hidden").

API consistency issue: This means modules/resources with redirectTo() guards now appear in navigation even though they never render their component — they always redirect away. This creates a confusing UX where users see a nav item that, when clicked, immediately redirects elsewhere.

Example of problematic behavior:

defineModule({
  path: "old-dashboard",
  guards: [() => redirectTo("/new-dashboard")],
  resources: [/* ... */],
});
// "Old Dashboard" appears in sidebar, but clicking it redirects to "/new-dashboard"

Recommendation: Consider one of these alternatives:

1. Add a new guard result type like { type: "silent-redirect" } that redirects but hides from navigation
2. Add an explicit meta.hiddenInNav flag so developers can control visibility independently
3. Document this new behavior clearly as intentional UX (nav items can be "redirect aliases")

The current approach conflates routing behavior (redirect) with UI visibility (sidebar), which may surprise developers.

[IzumiSy]

This is intentional — keeping redirectTo() items visible in the sidebar is a deliberate design choice for two reasons:

1. Redirect-only modules: A module may have no component but should still appear as a sidebar item that redirects elsewhere (e.g. aliasing a legacy path).
2. Child resource visibility: In auto-generated sidebar mode, hiding a module also hides all of its child resources from navigation. A module that redirects at its own path may still have child resources with real pages that users need to access.

If a developer wants to hide a module/resource from the sidebar entirely, they should use hidden() instead of redirectTo().

Added inline comments in navigation.tsx clarifying this design intent, and updated the changeset with a guard result → nav visibility table documenting the behavior.

Commit: 5888a43