diff --git a/docs/decisions/0002-do-not-clone-request.md b/docs/decisions/0002-do-not-clone-request.md index 0b691a56eb5..30f599f7f0f 100644 --- a/docs/decisions/0002-do-not-clone-request.md +++ b/docs/decisions/0002-do-not-clone-request.md @@ -6,14 +6,14 @@ Status: accepted ## Context -To allow multiple loaders / actions to read the body of a request, we have been cloning the request before forwarding it to user-code. This is not the best thing to do as some runtimes will begin buffering the body to allow for multiple consumers. It is also goes against "the platform" that states a request body should only be consumed once. +To allow multiple loaders / actions to read the body of a request, we have been cloning the request before forwarding it to user-code. This is not the best thing to do as some runtimes will begin buffering the body to allow for multiple consumers. It also goes against "the platform" that states a request body should only be consumed once. ## Decision -Do not clone requests before they are passed to user-code (loaders, actions, handleDocumentRequest, handleDataRequest, etc.). +Do not clone requests before they are passed to user-code (actions, handleDocumentRequest, handleDataRequest), and remove body from request passed to loaders. Loaders should be thought of as a "GET" / "HEAD" request handler. These request methods are not allowed to have a body, therefore you should not be reading it in your Remix loader function. ## Consequences -If you are reading the request body in both an action and a loader this will now fail. Loaders should be thought of as a "GET" / "HEAD" request handler. These request methods are not allowed to have a body, therefore you should not be reading it in your Remix loader function. +Loaders always receive a null body for the request. -If you wish to continue reading the request body in multiple places for a single request against recommendations, consider using `.clone()` before reading it; just know this comes with tradeoffs. +If you are reading the request body in both an action and handleDocumentRequest or handleDataRequest this will now fail as the body will have already been read. If you wish to continue reading the request body in multiple places for a single request against recommendations, consider using `.clone()` before reading it; just know this comes with tradeoffs. diff --git a/packages/remix-server-runtime/responses.ts b/packages/remix-server-runtime/responses.ts index d50965ad8a3..b571d587415 100644 --- a/packages/remix-server-runtime/responses.ts +++ b/packages/remix-server-runtime/responses.ts @@ -69,19 +69,3 @@ export function isRedirectResponse(response: Response): boolean { export function isCatchResponse(response: Response) { return response.headers.get("X-Remix-Catch") != null; } - -export function extractData(response: Response): Promise { - let contentType = response.headers.get("Content-Type"); - - if (contentType && /\bapplication\/json\b/.test(contentType)) { - return response.json(); - } - - // What other data types do we need to handle here? What other kinds of - // responses are people going to be returning from their loaders? - // - application/x-www-form-urlencoded ? - // - multipart/form-data ? - // - binary (audio/video) ? - - return response.text(); -} diff --git a/packages/remix-server-runtime/server.ts b/packages/remix-server-runtime/server.ts index 35bdc2593dc..aa6dc89a057 100644 --- a/packages/remix-server-runtime/server.ts +++ b/packages/remix-server-runtime/server.ts @@ -293,13 +293,18 @@ async function handleDocumentRequest({ ); } + let loaderRequest = new Request(request.url, { + ...request, + body: null, + }); + let routeLoaderResults = await Promise.allSettled( matchesToLoad.map((match) => match.route.module.loader ? callRouteLoader({ loadContext, match, - request, + request: loaderRequest, }) : Promise.resolve(undefined) )