Fix redirect guard behaviour

.changeset/fix-fs-routes-guard-loader-chain.md

@@ -0,0 +1,5 @@
+---
+"@tailor-platform/app-shell": patch
+---
+
+Fixed file-based routing (vite plugin) not chaining guards with user-provided loaders. When a page module defines both guards and a `loader` export, guards are now evaluated first via `withGuardsLoader`, and the user loader runs only after guards pass. Previously, guards could overwrite the user loader.

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

@@ -0,0 +1,41 @@
+---
+"@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.
+
+Fixed a component-less module with a mixed guard (one that sometimes returns `pass()`) rendering a blank page. When guards pass on a route without a component, a 404 error is now thrown instead of rendering an empty page.
+
+### 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/fs-routes/converter.test.tsx

@@ -102,6 +102,45 @@
     expect(modules[0].resources[0].guards).not.toContain(parentGuard);
   });
 
+  it("generates guardLoader when guards and loader co-exist", async () => {
+    const pages = [
+      createMockPage("/dashboard", {
+        guards: [async () => ({ type: "pass" as const })],
+        loader: async () => ({ data: "test" }),
+      }),
+    ];
+    const modules = convertPagesToModules(pages);
+
+    // guardLoader should be generated (wraps guards + baseLoader)
+    expect(modules[0].guardLoader).toBeDefined();
+    expect(typeof modules[0].guardLoader).toBe("function");
+
+    // loader should be cleared since guardLoader takes over
+    expect(modules[0].loader).toBeUndefined();
+
+    // Execute the guardLoader — guard passes, so baseLoader should run
+    const result = await modules[0].guardLoader!({} as never);
+    expect(result).toEqual({ data: "test" });
+  });
+
+  it("generates guardLoader for resource when guards and loader co-exist", async () => {
+    const pages = [
+      createMockPage("/dashboard"),
+      createMockPage("/dashboard/orders", {
+        guards: [async () => ({ type: "pass" as const })],
+        loader: async () => ({ items: [1, 2, 3] }),
+      }),
+    ];
+    const modules = convertPagesToModules(pages);
+
+    const resource = modules[0].resources[0];
+    expect(resource.guardLoader).toBeDefined();
+    expect(resource.loader).toBeUndefined();
+
+    const result = await resource.guardLoader!({} as never);
+    expect(result).toEqual({ items: [1, 2, 3] });
+  });
+
   it("handles multiple top-level modules", () => {
     const pages = [
       createMockPage("/dashboard"),
@@ -233,7 +272,7 @@
     expect(modules).toHaveLength(1);
     expect(modules[0].path).toBe("");
     expect(modules[0].guards).toHaveLength(1);
     expect(modules[0].loader).toBeDefined();
     expect(typeof modules[0].loader).toBe("function");
   });
 
@@ -244,7 +283,7 @@
     expect(modules).toHaveLength(1);
     expect(modules[0].path).toBe("dashboard");
     expect(modules[0].guards).toHaveLength(1);
     expect(modules[0].loader).toBeDefined();
     expect(typeof modules[0].loader).toBe("function");
   });
 
@@ -259,7 +298,7 @@
     const resource = modules[0].resources[0];
     expect(resource.path).toBe("orders");
     expect(resource.guards).toHaveLength(1);
     expect(resource.loader).toBeDefined();
     expect(typeof resource.loader).toBe("function");
   });
 

packages/core/src/fs-routes/converter.tsx

@@ -79,17 +79,19 @@ function nodeToResource(node: PageNode): Resource {
   const title = getTitle(Component, node.path);
   const icon = Component?.appShellPageProps?.meta?.icon;
   const breadcrumbTitle = Component?.appShellPageProps?.meta?.breadcrumbTitle;
-  const explicitLoader = Component?.appShellPageProps?.loader;
-  const loader =
-    explicitLoader ??
-    (node.guards && node.guards.length > 0 ? withGuardsLoader(node.guards) : undefined);
+  const loader = Component?.appShellPageProps?.loader;
 
   // Recursively convert children to subResources
   const subResources: Resource[] = [];
   for (const child of node.children.values()) {
     subResources.push(nodeToResource(child));
   }
 
+  const hasGuards = node.guards.length > 0;
+  const guardLoader = hasGuards
+    ? withGuardsLoader(node.guards, { hasComponent: !!Component, baseLoader: loader })
+    : undefined;
+
   return {
     // oxlint-disable-next-line no-underscore-dangle
     _type: "resource",
@@ -104,7 +106,8 @@ function nodeToResource(node: PageNode): Resource {
     subResources: subResources.length > 0 ? subResources : undefined,
     errorBoundary: <DefaultErrorBoundary />,
     guards: node.guards,
-    loader,
+    loader: hasGuards ? undefined : loader,
+    guardLoader,
   };
 }
 
@@ -116,17 +119,19 @@ function nodeToModule(node: PageNode): Module {
   const title = getTitle(Component, node.path);
   const icon = Component?.appShellPageProps?.meta?.icon;
   const breadcrumbTitle = Component?.appShellPageProps?.meta?.breadcrumbTitle;
-  const explicitLoader = Component?.appShellPageProps?.loader;
-  const loader =
-    explicitLoader ??
-    (node.guards && node.guards.length > 0 ? withGuardsLoader(node.guards) : undefined);
+  const loader = Component?.appShellPageProps?.loader;
 
   // Convert children to resources
   const resources: Resource[] = [];
   for (const child of node.children.values()) {
     resources.push(nodeToResource(child));
   }
 
+  const hasGuards = node.guards.length > 0;
+  const guardLoader = hasGuards
+    ? withGuardsLoader(node.guards, { hasComponent: !!Component, baseLoader: loader })
+    : undefined;
+
   return {
     // oxlint-disable-next-line no-underscore-dangle
     _type: "module",
@@ -142,7 +147,8 @@ function nodeToModule(node: PageNode): Module {
     resources,
     errorBoundary: <DefaultErrorBoundary />,
     guards: node.guards,
-    loader,
+    loader: hasGuards ? undefined : loader,
+    guardLoader,
   };
 }
 

packages/core/src/resource.tsx

@@ -158,8 +158,18 @@ 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.
  */
-export const withGuardsLoader = (guards: Guard[] | undefined, baseLoader?: LoaderHandler) => {
+export 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) {
@@ -168,7 +178,9 @@ export const withGuardsLoader = (guards: Guard[] | undefined, baseLoader?: Loade
       case "redirect":
         return redirect(result.to);
       case "pass":
-        return baseLoader ? baseLoader(args) : null;
+        if (baseLoader) return baseLoader(args);
+        if (!hasComponent) throw createNotFoundError();
+        return null;
     }
   };
 };
@@ -209,6 +221,7 @@ export type Module = Omit<CommonPageResource, "meta"> & {
   errorBoundary: ErrorBoundaryComponent;
   guards?: Guard[];
   loader?: LoaderHandler;
+  guardLoader?: LoaderHandler;
 };
 
 /**
@@ -225,6 +238,7 @@ export type Resource = CommonPageResource & {
   errorBoundary: ErrorBoundaryComponent;
   guards?: Guard[];
   loader?: LoaderHandler;
+  guardLoader?: LoaderHandler;
 };
 
 export type Modules = Array<Module>;
@@ -373,7 +387,10 @@ export function defineModule(props: DefineModuleProps): Module {
   const { path, meta, component, resources, errorBoundary, guards } = props;
   const metaTitle: LocalizedString = meta?.title ?? titleFromPath(path);
   const fallbackTitle = titleFromPath(path);
-  const loader = guards && guards.length > 0 ? withGuardsLoader(guards) : undefined;
+  const guardLoader =
+    guards && guards.length > 0
+      ? withGuardsLoader(guards, { hasComponent: !!component })
+      : undefined;
   const wrappedComponent = component
     ? makeComponent({ metaTitle, fallbackTitle }, (title) => component({ title, resources }))
     : undefined;
@@ -384,7 +401,7 @@ export function defineModule(props: DefineModuleProps): Module {
     // oxlint-disable-next-line no-underscore-dangle
     _type: "module" as const,
     component: wrappedComponent,
-    loader,
+    guardLoader,
     meta: {
       title: metaTitle,
       ...(meta?.breadcrumbTitle !== undefined ? { breadcrumbTitle: meta.breadcrumbTitle } : {}),
@@ -473,7 +490,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 guardLoader =
+    guards && guards.length > 0 ? withGuardsLoader(guards, { hasComponent: true }) : undefined;
   const wrappedComponent = component
     ? makeComponent({ metaTitle, fallbackTitle }, (title) =>
         component({ title, resources: subResources }),
@@ -494,6 +512,6 @@ export function defineResource(props: DefineResourceProps): Resource {
     subResources,
     errorBoundary: errorBoundary ?? <DefaultErrorBoundary />,
     guards,
-    loader,
+    guardLoader,
   };
 }

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,
@@ -99,7 +106,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);