Fix redirect guard behaviour

.changeset/fix-redirect-guard-routing.md

@@ -0,0 +1,39 @@
+---
+"@tailor-platform/app-shell": patch
+---
+
+Fixed `redirectTo()` guard not working on modules/resources without a component. Index routes are now created for loader-only routes, enabling redirect guards to execute properly.
+
+Fixed auto-generated sidebar navigation hiding modules/resources that use `redirectTo()` guards. Navigation filtering now only excludes `hidden()` guards, so redirect-guarded items remain visible in the sidebar. This only affects the auto-generation mode of `SidebarLayout`; composition mode (where children are explicitly provided) is not affected.
+
+### Guard result types and their effect on navigation visibility
+
+| Guard result | Nav visibility | Routing behaviour |
+|---|---|---|
+| `pass()` | Visible | Renders the component |
+| `hidden()` | **Hidden** | Returns 404 |
+| `redirectTo(path)` | Visible | Redirects to the specified path |
+
+`redirectTo()` guards intentionally keep the item visible in the sidebar. This supports the common pattern where a module has no component of its own but should still appear as a sidebar item that redirects elsewhere (e.g. aliasing a legacy path to a new location). Additionally, if a module with `redirectTo()` were hidden from the sidebar, all of its child resources would also disappear from navigation, even though those children may have real pages that users need to access.
+
+```tsx
+// This pattern previously rendered a blank page — now correctly redirects:
+defineModule({
+  path: "old-dashboard",
+  guards: [() => redirectTo("/new-dashboard")],
+  resources: [...],
+});
+
+// Modules/resources with redirectTo guards remain visible in the sidebar.
+// Clicking "Legacy" navigates to /legacy, which then redirects to /modern.
+defineModule({
+  path: "legacy",
+  guards: [() => redirectTo("/modern")],
+  resources: [
+    defineResource({
+      path: "page",
+      component: () => <div>Page</div>,
+    }),
+  ],
+});
+```

packages/core/src/routing/navigation.test.tsx

@@ -2,7 +2,7 @@ import { renderHook, waitFor } from "@testing-library/react";
 import { describe, it, expect, beforeEach } from "vitest";
 import { AppShell } from "../components/appshell";
 import { useNavItems } from "./navigation";
-import { defineModule, defineResource, hidden } from "@/resource";
+import { defineModule, defineResource, hidden, redirectTo } from "@/resource";
 
 const renderNavItems = (
   modules: Array<ReturnType<typeof defineModule>>,
@@ -226,6 +226,115 @@ describe("useNavItems", () => {
     expect(resources.map((r) => r.url)).not.toContain("users/:id");
   });
 
+  it("keeps modules with redirectTo guards visible in navigation", async () => {
+    const modules = [
+      defineModule({
+        path: "redirected",
+        meta: { title: "Redirected Module" },
+        resources: [
+          defineResource({
+            path: "target",
+            component: () => <div>Target</div>,
+          }),
+        ],
+        guards: [() => redirectTo("/other")],
+      }),
+      defineModule({
+        path: "dashboard",
+        meta: { title: "Dashboard" },
+        component: () => <div>Dashboard Root</div>,
+        resources: [
+          defineResource({
+            path: "overview",
+            component: () => <div>Overview</div>,
+          }),
+        ],
+      }),
+    ];
+
+    const { result } = renderNavItems(modules, "/dashboard/overview");
+
+    await waitFor(async () => {
+      expect(await result.current!).toHaveLength(2);
+    });
+
+    const navItems = await result.current!;
+    expect(navItems.map((i) => i.title)).toEqual([
+      "Redirected Module",
+      "Dashboard",
+    ]);
+  });
+
+  it("keeps resources with redirectTo guards visible in navigation", async () => {
+    const modules = [
+      defineModule({
+        path: "workspace",
+        meta: { title: "Workspace" },
+        component: () => <div>Workspace Root</div>,
+        resources: [
+          defineResource({
+            path: "visible",
+            component: () => <div>Visible</div>,
+          }),
+          defineResource({
+            path: "redirected",
+            component: () => <div>Redirected</div>,
+            guards: [() => redirectTo("/other")],
+          }),
+        ],
+      }),
+    ];
+
+    const { result } = renderNavItems(modules, "/workspace/visible");
+
+    await waitFor(async () => {
+      expect(await result.current!).toHaveLength(1);
+    });
+
+    const navItems = await result.current!;
+    const resources = navItems[0].items;
+    expect(resources).toHaveLength(2);
+    expect(resources.map((r) => r.title)).toEqual(["Visible", "Redirected"]);
+  });
+
+  it("keeps subResources with redirectTo guards visible in navigation", async () => {
+    const modules = [
+      defineModule({
+        path: "admin",
+        meta: { title: "Admin" },
+        component: () => <div>Admin Root</div>,
+        resources: [
+          defineResource({
+            path: "panel",
+            component: () => <div>Panel</div>,
+            subResources: [
+              defineResource({
+                path: "visible",
+                component: () => <div>Visible</div>,
+              }),
+              defineResource({
+                path: "redirected",
+                component: () => <div>Redirected</div>,
+                guards: [() => redirectTo("/other")],
+              }),
+            ],
+          }),
+        ],
+      }),
+    ];
+
+    const { result } = renderNavItems(modules, "/admin/panel");
+
+    await waitFor(async () => {
+      expect(await result.current!).toHaveLength(1);
+    });
+
+    const navItems = await result.current!;
+    const subItems = navItems[0].items[0].items!;
+    expect(subItems).toHaveLength(2);
+    expect(subItems.map((s) => s.title)).toEqual(["Visible", "Redirected"]);
+  });
+
   it("filters out hidden subResources by guards", async () => {
     const modules = [
       defineModule({

packages/core/src/routing/navigation.tsx

@@ -61,8 +61,15 @@ const buildNavItems = async (props: BuildNavItemsProps) => {
       // Skip param routes at module level
       if (module.path.startsWith(":")) return null;
 
+      // Only `hidden` guards are excluded from navigation.
+      // `redirect` guards intentionally remain visible — this supports the
+      // pattern where a module has no component but should still appear as
+      // a sidebar item (e.g. aliasing an old path to a new one).
+      // Additionally, hiding a module would also hide all of its child
+      // resources from the sidebar, which is undesirable when the module
+      // itself simply redirects but its children have real pages.
       const guardResult = await runGuards(module.guards);
-      if (guardResult.type !== "pass") return null;
+      if (guardResult.type === "hidden") return null;
 
       const visibleResources = await filterVisibleResources(
         module.resources,
@@ -98,7 +105,7 @@ const filterVisibleResources = async (
       if (resource.path.startsWith(":")) return null;
 
       const guardResult = await runGuards(resource.guards);
-      if (guardResult.type !== "pass") return null;
+      if (guardResult.type === "hidden") return null;
 
       const resourcePath = `${basePath}/${resource.path}`;
       const resourceTitle = resolveTitle(resource.meta.title, resource.path);

packages/core/src/routing/router.test.tsx

@@ -249,6 +249,43 @@ describe("RouterContainer (memory)", () => {
     expect(await screen.findByText("Overview")).toBeDefined();
   });
 
+  it("redirects when module has no component and only redirectTo guard", async () => {
+    const dashboardModule = defineModule({
+      path: "dashboard",
+      component: () => <div>Dashboard</div>,
+      meta: { title: "Dashboard" },
+      resources: [
+        defineResource({
+          path: "overview",
+          component: () => <div>Overview</div>,
+          meta: { title: "Overview" },
+        }),
+      ],
+    });
+
+    // Module without component — only redirectTo guard
+    const redirectModule = defineModule({
+      path: "redirect-only",
+      meta: { title: "Redirect Only" },
+      guards: [() => redirectTo("/dashboard/overview")],
+      resources: [
+        defineResource({
+          path: "child",
+          component: () => <div>Child</div>,
+          meta: { title: "Child" },
+        }),
+      ],
+    });
+
+    renderWithConfig({
+      modules: [dashboardModule, redirectModule],
+      initialEntries: ["/redirect-only"],
+    });
+
+    // Should redirect to dashboard/overview, not render a blank page
+    expect(await screen.findByText("Overview")).toBeDefined();
+  });
+
   it("redirects resource when guard returns redirectTo", async () => {
     const dashboardModule = defineModule({
       path: "dashboard",

packages/core/src/routing/routes.test.tsx

@@ -1,7 +1,7 @@
 import { describe, expect, it, assert } from "vitest";
 import { createContentRoutes } from "./routes";
 import { EmptyOutlet, SettingsWrapper } from "@/components/content";
-import { defineModule, defineResource, pass } from "@/resource";
+import { defineModule, defineResource, pass, redirectTo } from "@/resource";
 
 const createMockResource = (path: string) =>
   defineResource({
@@ -156,6 +156,85 @@ describe("createContentRoutes", () => {
     expect(childResource?.loader).toBeUndefined();
   });
 
+  it("creates index route with loader and dummy component for module without component but with guards", () => {
+    const module = defineModule({
+      path: "redirect-module",
+      meta: { title: "Redirect Module" },
+      guards: [() => redirectTo("/dashboard")],
+      resources: [createMockResource("child")],
+    });
+
+    const routes = createContentRoutes({
+      modules: [module],
+      settingsResources: [],
+    });
+
+    const moduleContainer = routes[1];
+    const moduleRoute = moduleContainer.children?.[0];
+    expect(moduleRoute?.path).toBe("redirect-module");
+
+    // Index route should exist with loader and dummy component
+    const indexRoute = moduleRoute?.children?.[0];
+    expect(indexRoute?.index).toBe(true);
+    expect(indexRoute?.loader).toBeDefined();
+    expect(typeof indexRoute?.loader).toBe("function");
+    expect(typeof indexRoute?.Component).toBe("function");
+
+    // Child resource should still be present
+    const childRoute = moduleRoute?.children?.[1];
+    expect(childRoute?.path).toBe("child");
+  });
+
+  it("does not create index route for module without component and without guards", () => {
+    // This case should throw in defineModule, but we test createRoute's behavior
+    // by using a module with component (which produces index route)
+    const module = defineModule({
+      path: "no-comp-no-guard",
+      meta: { title: "Test" },
+      component: () => <div>Test</div>,
+      resources: [createMockResource("child")],
+    });
+
+    const routes = createContentRoutes({
+      modules: [module],
+      settingsResources: [],
+    });
+
+    const moduleContainer = routes[1];
+    const moduleRoute = moduleContainer.children?.[0];
+    const indexRoute = moduleRoute?.children?.[0];
+    expect(indexRoute?.index).toBe(true);
+    expect(indexRoute?.Component).toBeDefined();
+    // No guards = no loader
+    expect(indexRoute?.loader).toBeUndefined();
+  });
+
+  it("redirect guard loader returns a redirect response", async () => {
+    const module = defineModule({
+      path: "redirect-module",
+      meta: { title: "Redirect Module" },
+      guards: [() => redirectTo("/dashboard")],
+      resources: [createMockResource("child")],
+    });
+
+    const routes = createContentRoutes({
+      modules: [module],
+      settingsResources: [],
+    });
+
+    const moduleContainer = routes[1];
+    const moduleRoute = moduleContainer.children?.[0];
+    const indexRoute = moduleRoute?.children?.[0];
+    const loader = indexRoute?.loader;
+    expect(typeof loader).toBe("function");
+
+    // Execute the loader - it should return a redirect
+    const result = await (loader as (...args: unknown[]) => unknown)({} as never);
+    expect(result).toBeInstanceOf(Response);
+    expect((result as Response).status).toBe(302);
+    expect((result as Response).headers.get("Location")).toBe("/dashboard");
+  });
+
   it("attaches loader to index route only for resource (no cascade to sub-resources)", () => {
     const module = defineModule({
       path: "dashboard",

packages/core/src/routing/routes.tsx

@@ -30,14 +30,28 @@ const createRoute = (
 ): RouteObject => {
   const effectiveErrorBoundary = source.errorBoundary || parentErrorBoundary;
 
-  // Guards are applied only to this route's index, not cascading to children
+  // Guards are applied only to this route's index, not cascading to children.
+  //
+  // When `source.loader` exists without a component, it means guards were attached
+  // via `withGuardsLoader()`. In that case we still need an index route so the loader
+  // can run (e.g. to redirect or throw 404). `source.loader` is currently only set
+  // by `withGuardsLoader()` when guards are present — if that coupling changes in the
+  // future this condition should be revisited.
   const indexRoute: RouteObject | undefined = source.component
     ? {
         index: true,
         Component: source.component,
         ...(source.loader && { loader: source.loader }),
       }
-    : undefined;
+    : source.loader
+      ? {
+          index: true,
+          loader: source.loader,
+          // Component is required to suppress React Router's warning about empty leaf routes,
+          // even though the loader always redirects/throws and this component will never render.
+          Component: () => null,
+        }
+      : undefined;
 
   return {
     path: source.path,