Skip to content

Commit

Permalink
Remove instanceof Response checks in favor of isResponse (#4782)
Browse files Browse the repository at this point in the history
* Remove instanceof Response checks in favor of isResponse

* Add changeset

* Bump to @remix-run/router 1.0.5-pre.1
  • Loading branch information
brophdawg11 authored and kentcdodds committed Dec 15, 2022
1 parent ffd55a4 commit 152dca1
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 11 deletions.
5 changes: 5 additions & 0 deletions .changeset/rare-bikes-report.md
@@ -0,0 +1,5 @@
---
"@remix-run/server-runtime": patch
---

Remove `instanceof Response` checks in favor of isResponse"
30 changes: 29 additions & 1 deletion integration/loader-test.ts
Expand Up @@ -69,27 +69,48 @@ test.describe("loader in an app", () => {
let appFixture: AppFixture;

let HOME_PAGE_TEXT = "hello world";
let REDIRECT_TARGET_TEXT = "redirect target";
let FETCH_TARGET_TEXT = "fetch target";

test.beforeAll(async () => {
appFixture = await createAppFixture(
await createFixture({
files: {
"app/root.jsx": js`
import { Outlet } from '@remix-run/react'
export default function Root() {
return (
<html>
<body>
${HOME_PAGE_TEXT}
<Outlet />
</body>
</html>
);
}
`,
"app/routes/redirect.jsx": js`
import { redirect } from "@remix-run/node";
export const loader = () => redirect("/");
export const loader = () => redirect("/redirect-target");
export default () => <div>Yo</div>
`,
"app/routes/redirect-target.jsx": js`
export default () => <div>${REDIRECT_TARGET_TEXT}</div>
`,
"app/routes/fetch.jsx": js`
export function loader({ request }) {
return fetch(new URL(request.url).origin + '/fetch-target');
}
`,

"app/routes/fetch-target.jsx": js`
import { json } from "@remix-run/node";
export function loader() {
return json({ message: "${FETCH_TARGET_TEXT}" })
}
`,
},
})
);
Expand All @@ -103,5 +124,12 @@ test.describe("loader in an app", () => {
let app = new PlaywrightFixture(appFixture, page);
await app.goto("/redirect");
expect(await app.getHtml()).toMatch(HOME_PAGE_TEXT);
expect(await app.getHtml()).toMatch(REDIRECT_TARGET_TEXT);
});

test("handles raw fetch responses", async ({ page }) => {
let app = new PlaywrightFixture(appFixture, page);
let res = await app.goto(`/fetch`);
expect((await res.json()).message).toBe(FETCH_TARGET_TEXT);
});
});
2 changes: 1 addition & 1 deletion packages/remix-server-runtime/package.json
Expand Up @@ -16,7 +16,7 @@
"typings": "dist/index.d.ts",
"module": "dist/esm/index.js",
"dependencies": {
"@remix-run/router": "1.0.5-pre.0",
"@remix-run/router": "1.0.5-pre.1",
"@types/cookie": "^0.4.0",
"@web3-storage/multipart-parser": "^1.0.0",
"cookie": "^0.4.1",
Expand Down
10 changes: 5 additions & 5 deletions packages/remix-server-runtime/server.ts
Expand Up @@ -17,7 +17,7 @@ import type { RouteMatch } from "./routeMatching";
import { matchServerRoutes } from "./routeMatching";
import type { ServerRoute, ServerRouteManifest } from "./routes";
import { createStaticHandlerDataRoutes, createRoutes } from "./routes";
import { json, isRedirectResponse } from "./responses";
import { json, isRedirectResponse, isResponse } from "./responses";
import { createServerHandoffString } from "./serverHandoff";

export type RequestHandler = (
Expand Down Expand Up @@ -123,7 +123,7 @@ async function handleDataRequestRR(

return response;
} catch (error) {
if (error instanceof Response) {
if (isResponse(error)) {
error.headers.set("X-Remix-Catch", "yes");
return error;
}
Expand Down Expand Up @@ -217,7 +217,7 @@ async function handleDocumentRequestRR(
return new Response(null, { status: 500 });
}

if (context instanceof Response) {
if (isResponse(context)) {
return context;
}

Expand Down Expand Up @@ -373,12 +373,12 @@ async function handleResourceRequestRR(
let response = await staticHandler.queryRoute(request, routeId);
// callRouteLoader/callRouteAction always return responses
invariant(
response instanceof Response,
isResponse(response),
"Expected a Response to be returned from queryRoute"
);
return response;
} catch (error) {
if (error instanceof Response) {
if (isResponse(error)) {
// Note: Not functionally required but ensures that our response headers
// match identically to what Remix returns
error.headers.set("X-Remix-Catch", "yes");
Expand Down
8 changes: 4 additions & 4 deletions yarn.lock
Expand Up @@ -2069,10 +2069,10 @@
"@changesets/types" "^5.0.0"
dotenv "^8.1.0"

"@remix-run/router@1.0.5-pre.0":
version "1.0.5-pre.0"
resolved "https://registry.npmjs.org/@remix-run/router/-/router-1.0.5-pre.0.tgz#d5327cc606b444b9721dd0856ad18760e8d75d6d"
integrity sha512-7UWQ6HcuNHbKeCiOKD94WDz+FAeeJVzxVi+GNeLczzEdJtqKAvYMiXqJ/IF84T8479+bUrEm/zp7hr+itZs8Zg==
"@remix-run/router@1.0.5-pre.1":
version "1.0.5-pre.1"
resolved "https://registry.npmjs.org/@remix-run/router/-/router-1.0.5-pre.1.tgz#dd0939d1631ebd56faa2bc69141e94a96887c7e5"
integrity sha512-dDfVwp0ta99+7vMKMDb5eh1Pc9CWZ5/Q+679YojyHyrZqSWJmlZyCP7prs6UAaXaGg5MveP1pCav/YFCINlcFw==

"@remix-run/web-blob@^3.0.3", "@remix-run/web-blob@^3.0.4":
version "3.0.4"
Expand Down

0 comments on commit 152dca1

Please sign in to comment.