Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(remix-server-runtime): RRR 2 - Deploy @remix-run/server-runtime changes #4612

Merged
merged 2 commits into from
Nov 16, 2022
Merged

feat(remix-server-runtime): RRR 2 - Deploy @remix-run/server-runtime changes #4612

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
035f4b4
feat(remix-server-runtime): RRR 2 - Deploy `@remix-run/server-runtime…
jacob-ebey Nov 16, 2022
5fc0f66
Create beige-planes-type.md
jacob-ebey Nov 16, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/beige-planes-type.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@remix-run/server-runtime": minor
---

We have been busy at work [Layering Remix on top of React Router 6.4](https://github.com/remix-run/remix/blob/main/decisions/0007-remix-on-react-router-6-4-0.md) and are excited to be releasing the step 1 in this process that consists of running the server flow through a local copy the new framework agnostic `@remix-run/router`.
79 changes: 0 additions & 79 deletions packages/remix-server-runtime/__tests__/data-test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
import type { ServerBuild } from "../build";
import { createRequestHandler } from "../server";
import { callRouteAction, callRouteLoader } from "../data";
import type { RouteMatch } from "../routeMatching";
import type { ServerRoute } from "../routes";

describe("loaders", () => {
// so that HTML/Fetch requests are the same, and so redirects don't hang on to
Expand Down Expand Up @@ -146,80 +143,4 @@ describe("loaders", () => {
let res = await handler(request);
expect(await res.json()).toMatchInlineSnapshot(`"?foo=bar&index=test"`);
});

it("throws the right error message when `loader` returns undefined", async () => {
let loader = async () => {};

let routeId = "routes/random";

let request = new Request("http://example.com/random?_data=routes/random");

let match = {
params: {},
pathname: "random",
route: {
id: routeId,
module: {
loader,
},
},
} as unknown as RouteMatch<ServerRoute>;

let possibleError: any;
try {
possibleError = await callRouteLoader({
request,
loader: match.route.module.loader,
routeId: match.route.id,
params: match.params,
loadContext: {},
});
} catch (error) {
possibleError = error;
}

expect(possibleError).toBeInstanceOf(Error);
expect(possibleError.message).toMatchInlineSnapshot(
'"You defined a loader for route \\"routes/random\\" but didn\'t return anything from your `loader` function. Please return a value or `null`."'
);
});
});

describe("actions", () => {
it("throws the right error message when `action` returns undefined", async () => {
let action = async () => {};

let routeId = "routes/random";

let request = new Request("http://example.com/random?_data=routes/random");

let match = {
params: {},
pathname: "random",
route: {
id: routeId,
module: {
action,
},
},
} as unknown as RouteMatch<ServerRoute>;

let possibleError: any;
try {
possibleError = await callRouteAction({
request,
action: match.route.module.action,
routeId: match.route.id,
params: match.params,
loadContext: {},
});
} catch (error) {
possibleError = error;
}

expect(possibleError).toBeInstanceOf(Error);
expect(possibleError.message).toMatchInlineSnapshot(
'"You defined an action for route \\"routes/random\\" but didn\'t return anything from your `action` function. Please return a value or `null`."'
);
});
});
122 changes: 6 additions & 116 deletions packages/remix-server-runtime/data.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import { json, isResponse, isRedirectResponse } from "./responses";
import { json, isResponse } from "./responses";
import type {
ActionFunction,
DataFunctionArgs,
LoaderFunction,
} from "./routeModules";
import { ErrorResponse } from "./router";

/**
* An object of unknown type for route loaders and actions provided by the
Expand All @@ -19,104 +18,6 @@ export interface AppLoadContext {
*/
export type AppData = any;

export async function callRouteAction({
loadContext,
routeId,
action,
params,
request,
}: {
loadContext: AppLoadContext;
routeId: string;
action?: ActionFunction;
params: DataFunctionArgs["params"];
request: Request;
}) {
if (!action) {
let pathname = new URL(request.url).pathname;
let msg =
`You made a ${request.method} request to "${pathname}" but did not provide ` +
`an \`action\` for route "${routeId}", so there is no way to handle the request.`;
throw new ErrorResponse(405, "Method Not Allowed", new Error(msg), true);
}

let result;
try {
result = await action({
request: stripDataParam(stripIndexParam(request)),
context: loadContext,
params,
});
} catch (error: unknown) {
if (!isResponse(error)) {
throw error;
}

if (!isRedirectResponse(error)) {
error.headers.set("X-Remix-Catch", "yes");
}
result = error;
}

if (result === undefined) {
throw new Error(
`You defined an action for route "${routeId}" but didn't return ` +
`anything from your \`action\` function. Please return a value or \`null\`.`
);
}

return isResponse(result) ? result : json(result);
}

export async function callRouteLoader({
loadContext,
routeId,
loader,
params,
request,
}: {
request: Request;
routeId: string;
loader?: LoaderFunction;
params: DataFunctionArgs["params"];
loadContext: AppLoadContext;
}) {
if (!loader) {
let pathname = new URL(request.url).pathname;
let msg =
`You made a ${request.method} request to "${pathname}" but did not provide ` +
`a \`loader\` for route "${routeId}", so there is no way to handle the request.`;
throw new ErrorResponse(400, "Bad Request", new Error(msg), true);
}

let result;
try {
result = await loader({
request: stripDataParam(stripIndexParam(request)),
context: loadContext,
params,
});
} catch (error: unknown) {
if (!isResponse(error)) {
throw error;
}

if (!isRedirectResponse(error)) {
error.headers.set("X-Remix-Catch", "yes");
}
result = error;
}

if (result === undefined) {
throw new Error(
`You defined a loader for route "${routeId}" but didn't return ` +
`anything from your \`loader\` function. Please return a value or \`null\`.`
);
}

return isResponse(result) ? result : json(result);
}

export async function callRouteActionRR({
loadContext,
action,
Expand Down Expand Up @@ -175,6 +76,11 @@ export async function callRouteLoaderRR({
return isResponse(result) ? result : json(result);
}

// TODO: Document these search params better
// and stop stripping these in V2. These break
// support for running in a SW and also expose
// valuable info to data funcs that is being asked
// for such as "is this a data request?".
function stripIndexParam(request: Request) {
let url = new URL(request.url);
let indexValues = url.searchParams.getAll("index");
Expand All @@ -197,19 +103,3 @@ function stripDataParam(request: Request) {
url.searchParams.delete("_data");
return new Request(url.href, request);
}

export function extractData(response: Response): Promise<unknown> {
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();
}
31 changes: 0 additions & 31 deletions packages/remix-server-runtime/headers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,37 +5,6 @@ import type { ServerRoute } from "./routes";
import type { RouteMatch } from "./routeMatching";
import type { StaticHandlerContext } from "./router";

export function getDocumentHeaders(
build: ServerBuild,
matches: RouteMatch<ServerRoute>[],
routeLoaderResponses: Record<string, Response>,
actionResponse?: Response
): Headers {
return matches.reduce((parentHeaders, match, index) => {
let routeModule = build.routes[match.route.id].module;
let routeLoaderResponse = routeLoaderResponses[match.route.id];
let loaderHeaders = routeLoaderResponse
? routeLoaderResponse.headers
: new Headers();
let actionHeaders = actionResponse ? actionResponse.headers : new Headers();
let headers = new Headers(
routeModule.headers
? typeof routeModule.headers === "function"
? routeModule.headers({ loaderHeaders, parentHeaders, actionHeaders })
: routeModule.headers
: undefined
);

// Automatically preserve Set-Cookie headers that were set either by the
// loader or by a parent route.
prependCookies(actionHeaders, headers);
prependCookies(loaderHeaders, headers);
prependCookies(parentHeaders, headers);

return headers;
}, new Headers());
}

export function getDocumentHeadersRR(
build: ServerBuild,
context: StaticHandlerContext,
Expand Down
4 changes: 0 additions & 4 deletions packages/remix-server-runtime/responses.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,3 @@ const redirectStatusCodes = new Set([301, 302, 303, 307, 308]);
export function isRedirectResponse(response: Response): boolean {
return redirectStatusCodes.has(response.status);
}

export function isCatchResponse(response: Response) {
return response.headers.get("X-Remix-Catch") != null;
}
Loading