refactor: move skip logic from define-router to create-pages (#1192)

I was skeptical about it. Actually, we tried kind of both in our iteration. Having `skip` logic in defineRouter can't support actually skipping rendering component. So, this moves the logic and the responsibility from define-router to create-pages. cc @tylersayshi
dai-shi · Jan 27, 2025 · 7c572ca · 7c572ca
1 parent 39d522e
commit 7c572ca
Show file tree

Hide file tree

Showing 2 changed files with 86 additions and 38 deletions.
diff --git a/packages/waku/src/router/create-pages.ts b/packages/waku/src/router/create-pages.ts
@@ -567,7 +567,7 @@ export const createPages = <
       }
       return paths;
     },
-    handleRoute: async (path, { query }) => {
+    handleRoute: async (path, { skip, query }) => {
       await configure();
 
       // path without slugs
@@ -587,13 +587,15 @@ export const createPages = <
 
       const pathSpec = parsePathWithSlug(routePath);
       const mapping = getPathMapping(pathSpec, path);
-      const result: Record<string, ReactNode> = {
-        [`page:${routePath}`]: createElement(
+      const elements: Record<string, ReactNode> = {};
+      const pageId = `page:${routePath}`;
+      if (!skip?.elements[pageId]) {
+        elements[pageId] = createElement(
           pageComponent,
           { ...mapping, ...(query ? { query } : {}), path },
           createElement(Children),
-        ),
-      };
+        );
+      }
 
       const layoutPaths = getLayouts(
         getOriginalStaticPathSpec(path) ?? pathSpec,
@@ -609,32 +611,43 @@ export const createPages = <
         // always true
         if (layout) {
           const id = 'layout:' + segment;
-          result[id] = createElement(
-            layout,
-            isDynamic ? { path } : null,
-            createElement(Children),
-          );
+          if (!skip?.elements[id]) {
+            elements[id] = createElement(
+              layout,
+              isDynamic ? { path } : null,
+              createElement(Children),
+            );
+          }
         }
       }
 
-      // loop over all layouts for path
-      const routeChildren = [
-        ...layoutPaths.map((lPath) => ({
-          component: Slot,
-          props: { id: `layout:${lPath}` },
-        })),
-        { component: Slot, props: { id: `page:${routePath}` } },
-      ];
+      const result: {
+        rootElement?: ReactNode;
+        routeElement?: ReactNode;
+        elements: Record<string, ReactNode>;
+      } = { elements };
 
-      return {
-        elements: result,
-        rootElement: createElement(
+      if (!skip?.rootElement) {
+        result.rootElement = createElement(
           rootItem ? rootItem.component : DefaultRoot,
           null,
           createElement(Children),
-        ),
-        routeElement: createNestedElements(routeChildren),
-      };
+        );
+      }
+
+      if (!skip?.routeElement) {
+        // loop over all layouts for path
+        const routeChildren = [
+          ...layoutPaths.map((lPath) => ({
+            component: Slot,
+            props: { id: `layout:${lPath}` },
+          })),
+          { component: Slot, props: { id: `page:${routePath}` } },
+        ];
+        result.routeElement = createNestedElements(routeChildren);
+      }
+
+      return result;
     },
     getApiConfig: async () => {
       await configure();

diff --git a/packages/waku/src/router/define-router.ts b/packages/waku/src/router/define-router.ts
@@ -87,11 +87,16 @@ export function unstable_defineRouter(fns: {
   handleRoute: (
     path: string,
     options: {
+      skip?: {
+        rootElement: boolean;
+        routeElement: boolean;
+        elements: Record<SlotId, boolean>;
+      };
       query?: string;
     },
   ) => Promise<{
-    rootElement: ReactNode;
-    routeElement: ReactNode;
+    rootElement?: ReactNode;
+    routeElement?: ReactNode;
     elements: Record<SlotId, ReactNode>;
   }>;
   getApiConfig?: () => Promise<
@@ -118,8 +123,10 @@ export function unstable_defineRouter(fns: {
     pathname: string | undefined;
     pattern: string;
     specs: {
-      isStatic?: true;
+      rootElementIsStatic?: true;
+      routeElementIsStatic?: true;
       staticElementIds?: SlotId[];
+      isStatic?: true;
       noSsr?: true;
       is404?: true;
       isApi?: true;
@@ -149,6 +156,12 @@ export function unstable_defineRouter(fns: {
             pathname: pathSpec2pathname(item.path),
             pattern: path2regexp(item.pathPattern || item.path),
             specs: {
+              ...(item.rootElement.isStatic
+                ? { rootElementIsStatic: true as const }
+                : {}),
+              ...(item.routeElement.isStatic
+                ? { routeElementIsStatic: true as const }
+                : {}),
               staticElementIds: Object.entries(item.elements).flatMap(
                 ([id, { isStatic }]) => (isStatic ? [id] : []),
               ),
@@ -189,11 +202,16 @@ export function unstable_defineRouter(fns: {
     skip: string[],
   ): Promise<string[]> => {
     const pathConfig = await getMyPathConfig();
+    const routeId = ROUTE_SLOT_ID_PREFIX + pathname;
     return skip.filter((slotId) => {
       const found = pathConfig.find(({ pathSpec }) =>
         getPathMapping(pathSpec, pathname),
       );
-      return found?.specs.staticElementIds?.includes(slotId);
+      return (
+        (found?.specs.rootElementIsStatic && slotId === 'root') ||
+        (found?.specs.routeElementIsStatic && slotId === routeId) ||
+        found?.specs.staticElementIds?.includes(slotId)
+      );
     });
   };
   const getEntries = async (
@@ -212,24 +230,41 @@ export function unstable_defineRouter(fns: {
     } catch {
       // ignore
     }
-    const skip = isStringArray(skipParam) ? skipParam : [];
+    const skipIds = isStringArray(skipParam) ? skipParam : [];
+    const effectiveSkipIds = await filterEffectiveSkip(pathname, skipIds);
+    const routeId = ROUTE_SLOT_ID_PREFIX + pathname;
+    const skip = {
+      rootElement: effectiveSkipIds.includes('root'),
+      routeElement: effectiveSkipIds.includes(routeId),
+      elements: Object.fromEntries(
+        effectiveSkipIds.flatMap((id) => {
+          if (['root', routeId].includes(id)) {
+            return [];
+          }
+          return [[id, true]];
+        }),
+      ),
+    };
     const { query } = parseRscParams(rscParams);
-    const { rootElement, routeElement, elements } = await fns.handleRoute(
+    const result = await fns.handleRoute(
       pathname,
-      pathConfigItem.specs.isStatic ? {} : { query },
+      pathConfigItem.specs.isStatic ? { skip } : { skip, query },
     );
     if (
-      Object.keys(elements).some((id) => id.startsWith(ROUTE_SLOT_ID_PREFIX))
+      Object.keys(result.elements).some((id) =>
+        id.startsWith(ROUTE_SLOT_ID_PREFIX),
+      )
     ) {
       throw new Error('Element ID cannot start with "route:"');
     }
-    const entries = {
-      ...elements,
-      root: rootElement,
-      [ROUTE_SLOT_ID_PREFIX + pathname]: routeElement,
+    const entries: Record<SlotId, ReactNode> = {
+      ...result.elements,
     };
-    for (const skipId of await filterEffectiveSkip(pathname, skip)) {
-      delete entries[skipId];
+    if ('rootElement' in result) {
+      entries.root = result.rootElement;
+    }
+    if ('routeElement' in result) {
+      entries[routeId] = result.routeElement;
     }
     entries[ROUTE_ID] = [pathname, query];
     entries[IS_STATIC_ID] = !!pathConfigItem.specs.isStatic;