fix: page, endpoint, q-data GET request (#2265)

packages/qwik-city/buildtime/vite/dev-server.ts

@@ -5,7 +5,10 @@ import { join, resolve } from 'node:path';
 import type { BuildContext } from '../types';
 import type { RouteModule } from '../../runtime/src/types';
 import type { QwikViteDevResponse } from '../../../qwik/src/optimizer/src/plugins/vite';
-import { loadUserResponse, updateRequestCtx } from '../../middleware/request-handler/user-response';
+import {
+  getRouteMatchPathname,
+  loadUserResponse,
+} from '../../middleware/request-handler/user-response';
 import { getQwikCityEnvData, pageHandler } from '../../middleware/request-handler/page-handler';
 import { updateBuildContext } from '../build';
 import { endpointHandler } from '../../middleware/request-handler/endpoint-handler';
@@ -62,7 +65,6 @@ export function ssrDevMiddleware(ctx: BuildContext, server: ViteDevServer) {
       }
 
       const requestCtx = fromNodeHttp(url, req, res, 'dev');
-      updateRequestCtx(requestCtx, ctx.opts.trailingSlash);
 
       await updateBuildContext(ctx);
 
@@ -74,7 +76,8 @@ export function ssrDevMiddleware(ctx: BuildContext, server: ViteDevServer) {
         }
       }
 
-      const routeResult = matchRouteRequest(requestCtx.url.pathname);
+      const matchPathname = getRouteMatchPathname(url.pathname, ctx.opts.trailingSlash);
+      const routeResult = matchRouteRequest(matchPathname);
       if (routeResult) {
         // found a matching route
         const { route, params } = routeResult;

packages/qwik-city/middleware/request-handler/request-handler.ts

@@ -1,10 +1,10 @@
-import { loadRoute } from '../../runtime/src/routing';
+import type { QwikCityHandlerOptions, QwikCityRequestContext } from './types';
 import { endpointHandler } from './endpoint-handler';
 import { errorHandler, ErrorResponse, errorResponse } from './error-handler';
+import { getRouteMatchPathname, loadUserResponse } from './user-response';
+import { loadRoute } from '../../runtime/src/routing';
 import { pageHandler } from './page-handler';
 import { RedirectResponse, redirectResponse } from './redirect-handler';
-import type { QwikCityHandlerOptions, QwikCityRequestContext } from './types';
-import { loadUserResponse, updateRequestCtx } from './user-response';
 
 /**
  * @alpha
@@ -16,9 +16,9 @@ export async function requestHandler<T = any>(
   try {
     const { render, qwikCityPlan } = opts;
     const { routes, menus, cacheModules, trailingSlash, basePathname } = qwikCityPlan;
-    updateRequestCtx(requestCtx, trailingSlash);
 
-    const loadedRoute = await loadRoute(routes, menus, cacheModules, requestCtx.url.pathname);
+    const matchPathname = getRouteMatchPathname(requestCtx.url.pathname, trailingSlash);
+    const loadedRoute = await loadRoute(routes, menus, cacheModules, matchPathname);
     if (loadedRoute) {
       // found and loaded the route for this pathname
       const [params, mods, _, routeBundleNames] = loadedRoute;

packages/qwik-city/middleware/request-handler/user-response.ts

@@ -28,13 +28,14 @@ export async function loadUserResponse(
   const { pathname } = url;
   const { method, headers } = request;
   const isPageModule = isLastModulePageRoute(routeModules);
-  const isEndpointReq = isEndPointRequest(method, headers.get('Accept'));
-  const isPageDataReq = isPageModule && isEndpointReq;
+  const isPageDataReq = isPageModule && pathname.endsWith(QDATA_JSON);
+  const isEndpointReq =
+    !isPageDataReq && isEndPointRequest(method, headers.get('Accept'), headers.get('Content-Type'));
 
   const cookie = new Cookie(headers.get('cookie'));
 
   const userResponse: UserResponseContext = {
-    type: isPageDataReq ? 'pagedata' : isPageModule ? 'pagehtml' : 'endpoint',
+    type: isPageDataReq ? 'pagedata' : isPageModule && !isEndpointReq ? 'pagehtml' : 'endpoint',
     url,
     params,
     status: HttpStatus.Ok,
@@ -44,10 +45,9 @@ export async function loadUserResponse(
     cookie,
     aborted: false,
   };
-  userResponse.headers.set('Vary', 'content-type, accept');
   let hasRequestMethodHandler = false;
 
-  if (isPageModule && pathname !== basePathname && !pathname.endsWith('.html')) {
+  if (isPageModule && !isPageDataReq && pathname !== basePathname && !pathname.endsWith('.html')) {
     // only check for slash redirect on pages
     if (trailingSlash) {
       // must have a trailing slash
@@ -98,6 +98,10 @@ export async function loadUserResponse(
           reqHandler = endpointModule.onPatch;
           break;
         }
+        case 'DELETE': {
+          reqHandler = endpointModule.onDelete;
+          break;
+        }
         case 'OPTIONS': {
           reqHandler = endpointModule.onOptions;
           break;
@@ -106,21 +110,13 @@ export async function loadUserResponse(
           reqHandler = endpointModule.onHead;
           break;
         }
-        case 'DELETE': {
-          reqHandler = endpointModule.onDelete;
-          break;
-        }
       }
 
       reqHandler = reqHandler || endpointModule.onRequest;
 
       if (typeof reqHandler === 'function') {
         hasRequestMethodHandler = true;
 
-        if (isEndpointReq && method !== 'GET') {
-          userResponse.type = 'endpoint';
-        }
-
         const response = new ResponseContext(userResponse, requestCtx);
 
         // create user request event, which is a narrowed down request context
@@ -184,11 +180,23 @@ export async function loadUserResponse(
     );
   }
 
-  // this is only an endpoint, and not a page module
-  if (userResponse.type === 'endpoint' && !hasRequestMethodHandler) {
-    // didn't find any handlers
-    throw new ErrorResponse(HttpStatus.MethodNotAllowed, `Method Not Allowed`);
+  if (hasRequestMethodHandler) {
+    // this request/method has a handler
+    if (isPageModule && method === 'GET') {
+      if (!userResponse.headers.has('Vary')) {
+        // if a page also has a GET handler, then auto-add the Accept Vary header
+        userResponse.headers.set('Vary', 'Content-Type, Accept');
+      }
+    }
+  } else {
+    // this request/method does NOT have a handler
+    if ((isEndpointReq && !isPageDataReq) || !isPageModule) {
+      // didn't find any handlers
+      // endpoints should respond with 405 Method Not Allowed
+      throw new ErrorResponse(HttpStatus.MethodNotAllowed, `Method Not Allowed`);
+    }
   }
+
   return userResponse;
 }
 
@@ -226,10 +234,18 @@ class ResponseContext implements ResponseContextInterface {
   }
 }
 
-export function isEndPointRequest(method: string, acceptHeader: string | null) {
+export function isEndPointRequest(
+  method: string,
+  acceptHeader: string | null,
+  contentTypeHeader: string | null
+) {
   if (method === 'GET' || method === 'POST') {
     // further check if GET or POST is an endpoint request
     // check if there's an Accept request header
+    if (contentTypeHeader && contentTypeHeader.includes('application/json')) {
+      return true;
+    }
+
     if (acceptHeader) {
       const htmlIndex = acceptHeader.indexOf('text/html');
       if (htmlIndex === 0) {
@@ -282,24 +298,19 @@ function isLastModulePageRoute(routeModules: RouteModule[]) {
   return lastRouteModule && typeof (lastRouteModule as PageModule).default === 'function';
 }
 
-export function updateRequestCtx(
-  requestCtx: QwikCityRequestContext,
-  trailingSlash: boolean | undefined
-) {
-  let pathname = requestCtx.url.pathname;
-
+/**
+ * The pathname used to match in the route regex array.
+ * A pathname ending with /q-data.json should be treated as a pathname without it.
+ */
+export function getRouteMatchPathname(pathname: string, trailingSlash: boolean | undefined) {
   if (pathname.endsWith(QDATA_JSON)) {
-    requestCtx.request.headers.set('Accept', 'application/json');
-    requestCtx.request.headers.set('Vary', 'Accept');
-
     const trimEnd = pathname.length - QDATA_JSON_LEN + (trailingSlash ? 1 : 0);
-
     pathname = pathname.slice(0, trimEnd);
     if (pathname === '') {
       pathname = '/';
     }
-    requestCtx.url.pathname = pathname;
   }
+  return pathname;
 }
 
 const QDATA_JSON = '/q-data.json';