Skip to content

Commit

Permalink
Lift redirects to the root for single fetch loaders
Browse files Browse the repository at this point in the history
  • Loading branch information
@brophdawg11
brophdawg11 committed Mar 11, 2024
1 parent 395078d commit faf262e
Show file tree
Hide file tree
Showing 8 changed files with 134 additions and 109 deletions.
6 changes: 4 additions & 2 deletions integration/client-data-test.ts
View file
Expand Up @@ -831,13 +831,15 @@ test.describe("Client Data", () => {
);
let app = new PlaywrightFixture(appFixture, page);

await app.goto("/parent/child");
await app.goto("/parent/child", false);
let html = await app.getHtml("main");
expect(html).toMatch("Parent Server Loader</p>");
expect(html).toMatch("Child Server Error");
expect(html).not.toMatch("Should not see me");
// Ensure we hydrate and remain on the boundary
await new Promise((r) => setTimeout(r, 100));
await page.waitForSelector(
":has-text('Parent Server Loader (mutated by client)')"
);
html = await app.getHtml("main");
expect(html).toMatch("Parent Server Loader (mutated by client)</p>");
expect(html).toMatch("Child Server Error");
Expand Down
5 changes: 3 additions & 2 deletions integration/defer-test.ts
View file
Expand Up @@ -2304,6 +2304,7 @@ test.describe("single fetch", () => {
// Exported for use by the server runtime so we can abort the
// turbo-stream encode() call
export const streamTimeout = 250;
const renderTimeout = streamTimeout + 250;
export default function handleRequest(
request: Request,
Expand Down Expand Up @@ -2365,7 +2366,7 @@ test.describe("single fetch", () => {
}
);
setTimeout(abort, streamTimeout);
setTimeout(abort, renderTimeout);
});
}
Expand Down Expand Up @@ -2407,7 +2408,7 @@ test.describe("single fetch", () => {
}
);
setTimeout(abort, streamTimeout);
setTimeout(abort, renderTimeout);
});
}
`,
Expand Down
26 changes: 15 additions & 11 deletions integration/error-data-request-test.ts
View file
Expand Up @@ -292,11 +292,13 @@ test.describe("single fetch", () => {
expect(status).toBe(200);
expect(headers.has("X-Remix-Error")).toBe(false);
expect(data).toEqual({
root: {
data: null,
},
"routes/_index": {
data: null,
results: {
root: {
data: null,
},
"routes/_index": {
data: null,
},
},
});
});
Expand Down Expand Up @@ -339,12 +341,14 @@ test.describe("single fetch", () => {
expect(status).toBe(404);
expect(headers.has("X-Remix-Error")).toBe(false);
expect(data).toEqual({
root: {
error: new ErrorResponseImpl(
404,
"Not Found",
'Error: No route matches URL "/i/match/nothing"'
),
results: {
root: {
error: new ErrorResponseImpl(
404,
"Not Found",
'Error: No route matches URL "/i/match/nothing"'
),
},
},
});
assertLoggedErrorInstance('No route matches URL "/i/match/nothing"');
Expand Down
124 changes: 73 additions & 51 deletions integration/error-sanitization-test.ts
View file
Expand Up @@ -757,11 +757,13 @@ test.describe("single fetch", () => {
test("returns data without errors", async () => {
let { data } = await fixture.requestSingleFetchData("/_root.data");
expect(data).toEqual({
root: {
data: null,
},
"routes/_index": {
data: "LOADER",
results: {
root: {
data: null,
},
"routes/_index": {
data: "LOADER",
},
},
});
});
Expand All @@ -771,11 +773,13 @@ test.describe("single fetch", () => {
"/_root.data?loader"
);
expect(data).toEqual({
root: {
data: null,
},
"routes/_index": {
error: new Error("Unexpected Server Error"),
results: {
root: {
data: null,
},
"routes/_index": {
error: new Error("Unexpected Server Error"),
},
},
});
expect(errorLogs.length).toBe(1);
Expand All @@ -786,7 +790,9 @@ test.describe("single fetch", () => {
test("returns deferred data without errors", async () => {
let { data } = await fixture.requestSingleFetchData("/defer.data");
// @ts-expect-error
expect(await data["routes/defer"].data.lazy).toEqual("RESOLVED");
expect(await data.results["routes/defer"].data.lazy).toEqual(
"RESOLVED"
);
});

test("sanitizes loader errors in deferred data requests", async () => {
Expand All @@ -795,7 +801,7 @@ test.describe("single fetch", () => {
);
try {
// @ts-expect-error
await data["routes/defer"].data.lazy;
await data.results["routes/defer"].data.lazy;
expect(true).toBe(false);
} catch (e) {
expect((e as Error).message).toBe("Unexpected Server Error");
Expand All @@ -820,12 +826,14 @@ test.describe("single fetch", () => {
"/not-a-route.data"
);
expect(data).toEqual({
root: {
error: new ErrorResponseImpl(
404,
"Not Found",
'Error: No route matches URL "/not-a-route"'
),
results: {
root: {
error: new ErrorResponseImpl(
404,
"Not Found",
'Error: No route matches URL "/not-a-route"'
),
},
},
});
expect(errorLogs).toEqual([
Expand Down Expand Up @@ -930,11 +938,13 @@ test.describe("single fetch", () => {
test("returns data without errors", async () => {
let { data } = await fixture.requestSingleFetchData("/_root.data");
expect(data).toEqual({
root: {
data: null,
},
"routes/_index": {
data: "LOADER",
results: {
root: {
data: null,
},
"routes/_index": {
data: "LOADER",
},
},
});
});
Expand All @@ -944,11 +954,13 @@ test.describe("single fetch", () => {
"/_root.data?loader"
);
expect(data).toEqual({
root: {
data: null,
},
"routes/_index": {
error: new Error("Loader Error"),
results: {
root: {
data: null,
},
"routes/_index": {
error: new Error("Loader Error"),
},
},
});
expect(errorLogs.length).toBe(1);
Expand All @@ -959,7 +971,9 @@ test.describe("single fetch", () => {
test("returns deferred data without errors", async () => {
let { data } = await fixture.requestSingleFetchData("/defer.data");
// @ts-expect-error
expect(await data["routes/defer"].data.lazy).toEqual("RESOLVED");
expect(await data.results["routes/defer"].data.lazy).toEqual(
"RESOLVED"
);
});

test("does not sanitize loader errors in deferred data requests", async () => {
Expand All @@ -968,7 +982,7 @@ test.describe("single fetch", () => {
);
try {
// @ts-expect-error
await data["routes/defer"].data.lazy;
await data.results["routes/defer"].data.lazy;
expect(true).toBe(false);
} catch (e) {
expect((e as Error).message).toBe("REJECTED");
Expand All @@ -994,12 +1008,14 @@ test.describe("single fetch", () => {
"/not-a-route.data"
);
expect(data).toEqual({
root: {
error: new ErrorResponseImpl(
404,
"Not Found",
'Error: No route matches URL "/not-a-route"'
),
results: {
root: {
error: new ErrorResponseImpl(
404,
"Not Found",
'Error: No route matches URL "/not-a-route"'
),
},
},
});
expect(errorLogs).toEqual([
Expand Down Expand Up @@ -1198,11 +1214,13 @@ test.describe("single fetch", () => {
test("returns data without errors", async () => {
let { data } = await fixture.requestSingleFetchData("/_root.data");
expect(data).toEqual({
root: {
data: null,
},
"routes/_index": {
data: "LOADER",
results: {
root: {
data: null,
},
"routes/_index": {
data: "LOADER",
},
},
});
});
Expand All @@ -1212,8 +1230,10 @@ test.describe("single fetch", () => {
"/_root.data?loader"
);
expect(data).toEqual({
root: { data: null },
"routes/_index": { error: new Error("Unexpected Server Error") },
results: {
root: { data: null },
"routes/_index": { error: new Error("Unexpected Server Error") },
},
});
expect(errorLogs[0][0]).toEqual("App Specific Error Logging:");
expect(errorLogs[1][0]).toEqual(
Expand All @@ -1227,7 +1247,7 @@ test.describe("single fetch", () => {
test("returns deferred data without errors", async () => {
let { data } = await fixture.requestSingleFetchData("/defer.data");
// @ts-expect-error
expect(await data["routes/defer"].data.lazy).toBe("RESOLVED");
expect(await data.results["routes/defer"].data.lazy).toBe("RESOLVED");
});

test("sanitizes loader errors in deferred data requests", async () => {
Expand All @@ -1236,7 +1256,7 @@ test.describe("single fetch", () => {
);
try {
// @ts-expect-error
await data["routes/defer"].data.lazy;
await data.results["routes/defer"].data.lazy;
expect(true).toBe(false);
} catch (e) {
expect((e as Error).message).toBe("Unexpected Server Error");
Expand Down Expand Up @@ -1266,12 +1286,14 @@ test.describe("single fetch", () => {
"/not-a-route.data"
);
expect(data).toEqual({
root: {
error: new ErrorResponseImpl(
404,
"Not Found",
'Error: No route matches URL "/not-a-route"'
),
results: {
root: {
error: new ErrorResponseImpl(
404,
"Not Found",
'Error: No route matches URL "/not-a-route"'
),
},
},
});
expect(errorLogs[0][0]).toEqual("App Specific Error Logging:");
Expand Down
6 changes: 4 additions & 2 deletions integration/loader-test.ts
View file
Expand Up @@ -195,8 +195,10 @@ test.describe("single fetch", () => {
test("returns responses for single fetch routes", async () => {
let { data } = await fixture.requestSingleFetchData("/_root.data");
expect(data).toEqual({
root: { data: ROOT_DATA },
"routes/_index": { data: INDEX_DATA },
results: {
root: { data: ROOT_DATA },
"routes/_index": { data: INDEX_DATA },
},
});
});
});
Expand Down
23 changes: 11 additions & 12 deletions packages/remix-react/single-fetch.tsx
View file
Expand Up @@ -7,6 +7,10 @@ import {
UNSAFE_ErrorResponseImpl as ErrorResponseImpl,
redirect,
} from "@remix-run/router";
import type {
UNSAFE_SingleFetchResult as SingleFetchResult,
UNSAFE_SingleFetchResults as SingleFetchResults,
} from "@remix-run/server-runtime";
import type {
DataRouteObject,
unstable_DataStrategyFunctionArgs as DataStrategyFunctionArgs,
Expand All @@ -18,15 +22,6 @@ import type { AssetsManifest, EntryContext } from "./entry";
import type { RouteModules } from "./routeModules";
import invariant from "./invariant";

// IMPORTANT! Keep in sync with the types in @remix-run/server-runtime
type SingleFetchResult =
| { data: unknown }
| { error: unknown }
| { redirect: string; status: number; revalidate: boolean; reload: boolean };
type SingleFetchResults = {
[key: string]: SingleFetchResult;
};

interface StreamTransferProps {
context: EntryContext;
identifier: number;
Expand Down Expand Up @@ -158,9 +153,13 @@ export function getSingleFetchDataStrategy(
singleFetchPromise = makeSingleFetchCall();
}
let results = await singleFetchPromise;
return results[routeId] !== undefined
? unwrapSingleFetchResult(results[routeId], routeId)
: null;
if ("redirect" in results) {
return unwrapSingleFetchResult(results, routeId);
} else {
return results.results[routeId] !== undefined
? unwrapSingleFetchResult(results.results[routeId], routeId)
: null;
}
};
}

Expand Down
4 changes: 4 additions & 0 deletions packages/remix-server-runtime/index.ts
View file
Expand Up @@ -5,6 +5,10 @@ export {
parseMultipartFormData as unstable_parseMultipartFormData,
} from "./formData";
export { defer, json, redirect, redirectDocument } from "./responses";
export type {
SingleFetchResult as UNSAFE_SingleFetchResult,
SingleFetchResults as UNSAFE_SingleFetchResults,
} from "./server";
export { createRequestHandler } from "./server";
export {
createSession,
Expand Down

0 comments on commit faf262e

Please sign in to comment.