Fix redirect guard behaviour

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

@@ -0,0 +1,39 @@
+---
+"@tailor-platform/app-shell": minor
+---
+
+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/resource.tsx

@@ -157,8 +157,17 @@ export const runGuards = async (guards: Guard[] | undefined): Promise<GuardResul
  *
  * If guards deny access, throws createNotFoundError or redirects.
  * Otherwise, runs the base loader if provided.
+ *
+ * @param guards - Guards to evaluate
+ * @param options.hasComponent - Whether the route has a component to render.
+ *   When false and guards pass, throws 404 instead of rendering a blank page.
+ * @param options.baseLoader - Optional base loader to run after guards pass.
  */
-const withGuardsLoader = (guards: Guard[] | undefined, baseLoader?: LoaderHandler) => {
+const withGuardsLoader = (
+  guards: Guard[] | undefined,
+  options?: { hasComponent?: boolean; baseLoader?: LoaderHandler },
+) => {
+  const { hasComponent, baseLoader } = options ?? {};
   return async (args: LoaderFunctionArgs) => {
     const result = await runGuards(guards);
     switch (result.type) {
@@ -167,7 +176,9 @@ const withGuardsLoader = (guards: Guard[] | undefined, baseLoader?: LoaderHandle
       case "redirect":
         return redirect(result.to);
       case "pass":
-        return baseLoader ? baseLoader(args) : null;
+        if (baseLoader) return baseLoader(args);
+        if (!hasComponent) throw createNotFoundError();
+        return null;
     }
   };
 };
@@ -356,7 +367,10 @@ export function defineModule(props: DefineModuleProps): Module {
     );
   }
 
-  const loader = guards && guards.length > 0 ? withGuardsLoader(guards) : undefined;
+  const loader =
+    guards && guards.length > 0
+      ? withGuardsLoader(guards, { hasComponent: !!component })
+      : undefined;
 
   const wrappedComponent = component
     ? makeComponent({ metaTitle, fallbackTitle }, (title) => component({ title, resources }))
@@ -433,7 +447,8 @@ export function defineResource(props: DefineResourceProps): Resource {
   const { path, component, subResources, meta, errorBoundary, guards } = props;
   const metaTitle: LocalizedString = meta?.title ?? capitalCase(path);
   const fallbackTitle = capitalCase(path);
-  const loader = guards && guards.length > 0 ? withGuardsLoader(guards) : undefined;
+  const loader =
+    guards && guards.length > 0 ? withGuardsLoader(guards, { hasComponent: true }) : undefined;
 
   return {
     _type: "resource" as const,

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,112 @@ 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",