feat: improve error message for duplicated loaders (#4619)

fixes #4603
QwikDev · Jun 26, 2023 · b567025 · b567025
1 parent d1912ed
commit b567025
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 12 deletions.
diff --git a/packages/qwik-city/buildtime/vite/dev-server.ts b/packages/qwik-city/buildtime/vite/dev-server.ts
@@ -2,8 +2,10 @@ import type { ViteDevServer, Connect } from 'vite';
 import type { ServerResponse } from 'node:http';
 import type { BuildContext, BuildRoute } from '../types';
 import type {
+  ActionInternal,
   ContentMenu,
   LoadedRoute,
+  LoaderInternal,
   MenuData,
   MenuModule,
   MenuModuleLoader,
@@ -24,7 +26,10 @@ import { updateBuildContext } from '../build';
 import { getExtension, normalizePath } from '../../utils/fs';
 import { getMenuLoader, getPathParams } from '../../runtime/src/routing';
 import { fromNodeHttp, getUrl } from '../../middleware/node/http';
-import { resolveRequestHandlers } from '../../middleware/request-handler/resolve-request-handlers';
+import {
+  checkBrand,
+  resolveRequestHandlers,
+} from '../../middleware/request-handler/resolve-request-handlers';
 import { formatError } from './format-error';
 
 export function ssrDevMiddleware(ctx: BuildContext, server: ViteDevServer) {
@@ -81,12 +86,13 @@ export function ssrDevMiddleware(ctx: BuildContext, server: ViteDevServer) {
         const qwikSerializer = { _deserializeData, _serializeData, _verifySerializable };
 
         // use vite to dynamically load each layout/page module in this route's hierarchy
-
+        const loaderMap = new Map<string, string>();
         const serverPlugins: RouteModule[] = [];
         for (const file of ctx.serverPlugins) {
           const layoutModule = await server.ssrLoadModule(file.filePath);
           serverPlugins.push(layoutModule);
           routeModulePaths.set(layoutModule, file.filePath);
+          checkModule(loaderMap, layoutModule, file.filePath);
         }
 
         const matchPathname = getRouteMatchPathname(url.pathname, ctx.opts.trailingSlash);
@@ -103,10 +109,12 @@ export function ssrDevMiddleware(ctx: BuildContext, server: ViteDevServer) {
             const layoutModule = await server.ssrLoadModule(layout.filePath);
             routeModules.push(layoutModule);
             routeModulePaths.set(layoutModule, layout.filePath);
+            checkModule(loaderMap, layoutModule, layout.filePath);
           }
           const endpointModule = await server.ssrLoadModule(route.filePath);
           routeModules.push(endpointModule);
           routeModulePaths.set(endpointModule, route.filePath);
+          checkModule(loaderMap, endpointModule, route.filePath);
         }
 
         const renderFn = async (requestEv: RequestEvent) => {
@@ -261,6 +269,31 @@ export function ssrDevMiddleware(ctx: BuildContext, server: ViteDevServer) {
   };
 }
 
+const checkModule = (loaderMap: Map<string, string>, routeModule: any, filePath: string) => {
+  for (const loader of Object.values(routeModule)) {
+    if (checkBrand(loader, 'server_action') || checkBrand(loader, 'server_loader')) {
+      checkUniqueLoader(loaderMap, loader as any, filePath);
+    }
+  }
+};
+
+const checkUniqueLoader = (
+  loaderMap: Map<string, string>,
+  loader: LoaderInternal | ActionInternal,
+  filePath: string
+) => {
+  const prev = loaderMap.get(loader.__id);
+  if (prev) {
+    const type = loader.__brand === 'server_loader' ? 'routeLoader$' : 'routeAction$';
+    throw new Error(
+      `The same ${type} (${loader.__qrl.getSymbol()}) was exported in multiple modules:
+      - ${prev}
+      - ${filePath}`
+    );
+  }
+  loaderMap.set(loader.__id, filePath);
+};
+
 export function getUnmatchedRouteHtml(url: URL, ctx: BuildContext): string {
   const blue = '#006ce9';
   const routesAndDistance = sortRoutesByDistance(ctx.routes, url);

diff --git a/packages/qwik-city/middleware/request-handler/resolve-request-handlers.ts b/packages/qwik-city/middleware/request-handler/resolve-request-handlers.ts
@@ -73,7 +73,7 @@ export const resolveRequestHandlers = (
       method
     );
     if (isPageRoute) {
-      if (routeLoaders.length + actionsMiddleware.length > 0) {
+      if (routeLoaders.length + routeActions.length > 0) {
         requestHandlers.push(actionsMiddleware(routeLoaders, routeActions) as any);
       }
       requestHandlers.push(renderHandler);
@@ -138,12 +138,12 @@ const _resolveRequestHandlers = (
     if (collectActions) {
       const loaders = Object.values(routeModule).filter((e) =>
         checkBrand(e, 'server_loader')
-      ) as any[];
+      ) as LoaderInternal[];
       routeLoaders.push(...loaders);
 
       const actions = Object.values(routeModule).filter((e) =>
         checkBrand(e, 'server_action')
-      ) as any[];
+      ) as ActionInternal[];
       routeActions.push(...actions);
     }
   }
@@ -207,13 +207,6 @@ export function actionsMiddleware(routeLoaders: LoaderInternal[], routeActions:
       await Promise.all(
         routeLoaders.map((loader) => {
           const loaderId = loader.__id;
-          if (isDev) {
-            if (loaders[loaderId]) {
-              throw new Error(
-                `Duplicate loader id "${loaderId}" detected. Please ensure that all loader ids are unique.`
-              );
-            }
-          }
           return (loaders[loaderId] = runValidators(
             requestEv,
             loader.__validators,