From 4e938cd21ace2c61e074b368f503dca114650797 Mon Sep 17 00:00:00 2001 From: Logan McAnsh Date: Tue, 10 Aug 2021 11:37:41 -0400 Subject: [PATCH 01/33] feat(express): add support for multiple 'Set-Cookie' and other multi headers x-ref #231 Signed-off-by: Logan McAnsh --- .../remix-express/__tests__/server-test.ts | 19 +++++++++++++++---- packages/remix-express/server.ts | 6 ++++-- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/packages/remix-express/__tests__/server-test.ts b/packages/remix-express/__tests__/server-test.ts index f55dd44ac33..ddf85204f74 100644 --- a/packages/remix-express/__tests__/server-test.ts +++ b/packages/remix-express/__tests__/server-test.ts @@ -3,7 +3,7 @@ import supertest from "supertest"; import { createRequestHandler } from "../server"; -import { Response } from "@remix-run/node"; +import { Response, Headers } from "@remix-run/node"; import { createRequestHandler as createRemixRequestHandler } from "@remix-run/node/server"; @@ -65,15 +65,26 @@ describe("express createRequestHandler", () => { it("sets headers", async () => { mockedCreateRequestHandler.mockImplementation(() => async () => { - return new Response("", { - headers: { "X-Time-Of-Year": "most wonderful" } - }); + const headers = new Headers({ "X-Time-Of-Year": "most wonderful" }); + headers.append( + "Set-Cookie", + "first=one; Expires=0; Path=/; HttpOnly; Secure; SameSite=Lax" + ); + headers.append( + "Set-Cookie", + "second=two; MaxAge=1209600; Path=/; HttpOnly; Secure; SameSite=Lax" + ); + return new Response("", { headers }); }); let request = supertest(createApp()); let res = await request.get("/"); expect(res.headers["x-time-of-year"]).toBe("most wonderful"); + expect(res.headers["set-cookie"]).toEqual([ + "first=one; Expires=0; Path=/; HttpOnly; Secure; SameSite=Lax", + "second=two; MaxAge=1209600; Path=/; HttpOnly; Secure; SameSite=Lax" + ]); }); }); }); diff --git a/packages/remix-express/server.ts b/packages/remix-express/server.ts index 3d7bd7b06b2..43d95241648 100644 --- a/packages/remix-express/server.ts +++ b/packages/remix-express/server.ts @@ -101,8 +101,10 @@ function createRemixRequest(req: express.Request): Request { function sendRemixResponse(res: express.Response, response: Response): void { res.status(response.status); - for (let [key, value] of response.headers.entries()) { - res.set(key, value); + for (let [key, values] of Object.entries(response.headers.raw())) { + for (const value of values) { + res.append(key, value); + } } if (Buffer.isBuffer(response.body)) { From 8c93d194cfcdeddebbb24c716cf3d83f5b541cf2 Mon Sep 17 00:00:00 2001 From: Logan McAnsh Date: Tue, 10 Aug 2021 12:01:26 -0400 Subject: [PATCH 02/33] test(express): add tests for createRemixHeaders Signed-off-by: Logan McAnsh --- .../remix-express/__tests__/server-test.ts | 94 ++++++++++++++++++- packages/remix-express/server.ts | 4 +- 2 files changed, 92 insertions(+), 6 deletions(-) diff --git a/packages/remix-express/__tests__/server-test.ts b/packages/remix-express/__tests__/server-test.ts index ddf85204f74..be5a75120ca 100644 --- a/packages/remix-express/__tests__/server-test.ts +++ b/packages/remix-express/__tests__/server-test.ts @@ -1,12 +1,10 @@ import express from "express"; import supertest from "supertest"; - -import { createRequestHandler } from "../server"; - import { Response, Headers } from "@remix-run/node"; - import { createRequestHandler as createRemixRequestHandler } from "@remix-run/node/server"; +import { createRemixHeaders, createRequestHandler } from "../server"; + // We don't want to test that the remix server works here (that's what the // puppetteer tests do), we just want to test the express adapter jest.mock("@remix-run/node/server"); @@ -88,3 +86,91 @@ describe("express createRequestHandler", () => { }); }); }); + +describe("express createRemixHeaders", () => { + describe("creates fetch headers from express headers", () => { + it("handles empty headers", () => { + expect(createRemixHeaders({})).toMatchInlineSnapshot(` + Headers { + Symbol(map): Object {}, + } + `); + }); + + it("handles simple headers", () => { + expect(createRemixHeaders({ "x-foo": "bar" })).toMatchInlineSnapshot(` + Headers { + Symbol(map): Object { + "x-foo": Array [ + "bar", + ], + }, + } + `); + }); + + it("handles multiple headers", () => { + expect(createRemixHeaders({ "x-foo": "bar", "x-bar": "baz" })) + .toMatchInlineSnapshot(` + Headers { + Symbol(map): Object { + "x-bar": Array [ + "baz", + ], + "x-foo": Array [ + "bar", + ], + }, + } + `); + }); + + it("handles headers with multiple values", () => { + expect(createRemixHeaders({ "x-foo": "bar, baz" })) + .toMatchInlineSnapshot(` + Headers { + Symbol(map): Object { + "x-foo": Array [ + "bar, baz", + ], + }, + } + `); + }); + + it("handles headers with multiple values and multiple headers", () => { + expect(createRemixHeaders({ "x-foo": "bar, baz", "x-bar": "baz" })) + .toMatchInlineSnapshot(` + Headers { + Symbol(map): Object { + "x-bar": Array [ + "baz", + ], + "x-foo": Array [ + "bar, baz", + ], + }, + } + `); + }); + + it("handles multiple set-cookie headers", () => { + expect( + createRemixHeaders({ + "set-cookie": [ + "__session=some_value; Path=/; Secure; HttpOnly; MaxAge=7200; SameSite=Lax", + "__other=some_other_value; Path=/; Secure; HttpOnly; MaxAge=3600; SameSite=Lax" + ] + }) + ).toMatchInlineSnapshot(` + Headers { + Symbol(map): Object { + "set-cookie": Array [ + "__session=some_value; Path=/; Secure; HttpOnly; MaxAge=7200; SameSite=Lax,__other=some_other_value; Path=/; Secure; HttpOnly; MaxAge=3600; SameSite=Lax", + ], + }, + } + `); + }); + }); +}); diff --git a/packages/remix-express/server.ts b/packages/remix-express/server.ts index 43d95241648..e2cfd162d53 100644 --- a/packages/remix-express/server.ts +++ b/packages/remix-express/server.ts @@ -64,7 +64,7 @@ export function createRequestHandler({ }; } -function createRemixHeaders( +export function createRemixHeaders( requestHeaders: express.Request["headers"] ): Headers { return new Headers( @@ -82,7 +82,7 @@ function createRemixHeaders( ); } -function createRemixRequest(req: express.Request): Request { +export function createRemixRequest(req: express.Request): Request { let origin = `${req.protocol}://${req.hostname}`; let url = new URL(req.url, origin); From 52d0209934caf17dbcb9a2111d49a237352d0d5f Mon Sep 17 00:00:00 2001 From: Logan McAnsh Date: Tue, 10 Aug 2021 12:08:07 -0400 Subject: [PATCH 03/33] test(express): add basic test for createRemixRequest Signed-off-by: Logan McAnsh --- .../remix-express/__tests__/server-test.ts | 63 ++++++++++++++++++- packages/remix-express/package.json | 1 + yarn.lock | 5 ++ 3 files changed, 68 insertions(+), 1 deletion(-) diff --git a/packages/remix-express/__tests__/server-test.ts b/packages/remix-express/__tests__/server-test.ts index be5a75120ca..fd91129d957 100644 --- a/packages/remix-express/__tests__/server-test.ts +++ b/packages/remix-express/__tests__/server-test.ts @@ -2,8 +2,13 @@ import express from "express"; import supertest from "supertest"; import { Response, Headers } from "@remix-run/node"; import { createRequestHandler as createRemixRequestHandler } from "@remix-run/node/server"; +import { getMockReq } from "@jest-mock/express"; -import { createRemixHeaders, createRequestHandler } from "../server"; +import { + createRemixHeaders, + createRemixRequest, + createRequestHandler +} from "../server"; // We don't want to test that the remix server works here (that's what the // puppetteer tests do), we just want to test the express adapter @@ -174,3 +179,59 @@ describe("express createRemixHeaders", () => { }); }); }); + +describe("express createRemixRequest", () => { + it("creates a request with the correct headers", async () => { + const expressRequest = getMockReq({ + url: "/foo/bar", + protocol: "http", + hostname: "localhost", + headers: { + "Cache-Control": "max-age=300, s-maxage=3600" + }, + pipe: jest.fn() + }); + + expect(createRemixRequest(expressRequest)).toMatchInlineSnapshot(` + Request { + "agent": undefined, + "compress": true, + "counter": 0, + "follow": 20, + "size": 0, + "timeout": 0, + Symbol(Body internals): Object { + "body": null, + "disturbed": false, + "error": null, + }, + Symbol(Request internals): Object { + "headers": Headers { + Symbol(map): Object { + "Cache-Control": Array [ + "max-age=300, s-maxage=3600", + ], + }, + }, + "method": "GET", + "parsedURL": Url { + "auth": null, + "hash": null, + "host": "localhost", + "hostname": "localhost", + "href": "http://localhost/foo/bar", + "path": "/foo/bar", + "pathname": "/foo/bar", + "port": null, + "protocol": "http:", + "query": null, + "search": null, + "slashes": true, + }, + "redirect": "follow", + "signal": null, + }, + } + `); + }); +}); diff --git a/packages/remix-express/package.json b/packages/remix-express/package.json index 83c887f6b5f..aa582ec1ff6 100644 --- a/packages/remix-express/package.json +++ b/packages/remix-express/package.json @@ -10,6 +10,7 @@ "express": "^4.17.1" }, "devDependencies": { + "@jest-mock/express": "^1.4.3", "@types/express": "^4.17.9", "@types/supertest": "^2.0.10", "supertest": "^6.0.1" diff --git a/yarn.lock b/yarn.lock index bbacede38fb..03ed19da3e9 100644 --- a/yarn.lock +++ b/yarn.lock @@ -954,6 +954,11 @@ resolved "https://registry.yarnpkg.com/@istanbuljs/schema/-/schema-0.1.3.tgz#e45e384e4b8ec16bce2fd903af78450f6bf7ec98" integrity sha512-ZXRY4jNvVgSVQ8DL3LTcakaAtXwTVUxE81hslsyD2AtoXW/wVob10HkOJ1X/pAlcI7D+2YoZKg5do8G/w6RYgA== +"@jest-mock/express@^1.4.3": + version "1.4.3" + resolved "https://registry.yarnpkg.com/@jest-mock/express/-/express-1.4.3.tgz#2849cbc82365d3bacf542f8ee58d5455d5478852" + integrity sha512-qf7BGFl0T+o1/PuBWaLjRp0fZIbuZOE/VHX6oBAez8gvYZzRmJAwafD9QBSOFQccLczn2d2SdMTzWFFUSTQ0lg== + "@jest/console@^26.6.2": version "26.6.2" resolved "https://registry.yarnpkg.com/@jest/console/-/console-26.6.2.tgz#4e04bc464014358b03ab4937805ee36a0aeb98f2" From ba964731dca1cee5b1440870013d526334a32e92 Mon Sep 17 00:00:00 2001 From: Logan McAnsh Date: Tue, 10 Aug 2021 14:18:03 -0400 Subject: [PATCH 04/33] test(architect): add tests for headers Signed-off-by: Logan McAnsh --- jest.config.js | 16 ++- .../remix-architect/__tests__/server-test.ts | 132 ++++++++++++++++++ packages/remix-architect/package.json | 1 + packages/remix-architect/server.ts | 62 +++++--- packages/remix-architect/tsconfig.json | 2 +- types/architect__functions.d.ts | 132 ------------------ yarn.lock | 5 + 7 files changed, 195 insertions(+), 155 deletions(-) create mode 100644 packages/remix-architect/__tests__/server-test.ts delete mode 100644 types/architect__functions.d.ts diff --git a/jest.config.js b/jest.config.js index 0358d50d9bc..3c00876dfc9 100644 --- a/jest.config.js +++ b/jest.config.js @@ -11,16 +11,28 @@ module.exports = { testEnvironment: "node", testMatch: ["/packages/remix-dev/**/*-test.[jt]s?(x)"] }, + { + displayName: "node", + testEnvironment: "node", + testMatch: ["/packages/remix-node/**/*-test.[jt]s?(x)"] + }, + // Node Adapters + { + displayName: "architect", + testEnvironment: "node", + testMatch: ["/packages/remix-architect/**/*-test.[jt]s?(x)"] + }, { displayName: "express", testEnvironment: "node", testMatch: ["/packages/remix-express/**/*-test.[jt]s?(x)"] }, { - displayName: "node", + displayName: "vercel", testEnvironment: "node", - testMatch: ["/packages/remix-node/**/*-test.[jt]s?(x)"] + testMatch: ["/packages/remix-vercel/**/*-test.[jt]s?(x)"] }, + // Fixture Apps { displayName: "gists-app", testEnvironment: "node", diff --git a/packages/remix-architect/__tests__/server-test.ts b/packages/remix-architect/__tests__/server-test.ts new file mode 100644 index 00000000000..7ad5ff01671 --- /dev/null +++ b/packages/remix-architect/__tests__/server-test.ts @@ -0,0 +1,132 @@ +import { Response, Headers } from "@remix-run/node"; +import { createRequestHandler as createRemixRequestHandler } from "@remix-run/node/server"; + +import { + createRemixHeaders, + createRemixRequest, + createRequestHandler +} from "../server"; + +// We don't want to test that the remix server works here (that's what the +// puppetteer tests do), we just want to test the express adapter +jest.mock("@remix-run/node/server"); +let mockedCreateRequestHandler = createRemixRequestHandler as jest.MockedFunction< + typeof createRemixRequestHandler +>; + +describe.skip("architect createRequestHandler", () => {}); + +describe("architect createRemixHeaders", () => { + describe("creates fetch headers from architect headers", () => { + it("handles empty headers", () => { + expect(createRemixHeaders({}, undefined)).toMatchInlineSnapshot(` + Headers { + Symbol(map): Object {}, + } + `); + }); + + it("handles simple headers", () => { + expect(createRemixHeaders({ "x-foo": "bar" }, undefined)) + .toMatchInlineSnapshot(` + Headers { + Symbol(map): Object { + "x-foo": Array [ + "bar", + ], + }, + } + `); + }); + + it("handles multiple headers", () => { + expect(createRemixHeaders({ "x-foo": "bar", "x-bar": "baz" }, undefined)) + .toMatchInlineSnapshot(` + Headers { + Symbol(map): Object { + "x-bar": Array [ + "baz", + ], + "x-foo": Array [ + "bar", + ], + }, + } + `); + }); + + it("handles headers with multiple values", () => { + expect(createRemixHeaders({ "x-foo": "bar, baz" }, undefined)) + .toMatchInlineSnapshot(` + Headers { + Symbol(map): Object { + "x-foo": Array [ + "bar, baz", + ], + }, + } + `); + }); + + it("handles headers with multiple values and multiple headers", () => { + expect( + createRemixHeaders({ "x-foo": "bar, baz", "x-bar": "baz" }, undefined) + ).toMatchInlineSnapshot(` + Headers { + Symbol(map): Object { + "x-bar": Array [ + "baz", + ], + "x-foo": Array [ + "bar, baz", + ], + }, + } + `); + }); + + // it.skip("handles multiple set-cookie headers", () => { + // expect( + // createRemixHeaders( + // { + // "set-cookie": [ + // "__session=some_value; Path=/; Secure; HttpOnly; MaxAge=7200; SameSite=Lax", + // "__other=some_other_value; Path=/; Secure; HttpOnly; MaxAge=3600; SameSite=Lax" + // ] + // }, + // undefined + // ) + // ).toMatchInlineSnapshot(` + // Headers { + // Symbol(map): Object { + // "set-cookie": Array [ + // "__session=some_value; Path=/; Secure; HttpOnly; MaxAge=7200; SameSite=Lax,__other=some_other_value; Path=/; Secure; HttpOnly; MaxAge=3600; SameSite=Lax", + // ], + // }, + // } + // `); + // }); + + it("handles cookies", () => { + expect( + createRemixHeaders({}, [ + "__session=some_value; Path=/; Secure; HttpOnly; MaxAge=7200; SameSite=Lax", + "__other=some_other_value; Path=/; Secure; HttpOnly; MaxAge=3600; SameSite=Lax" + ]) + ).toMatchInlineSnapshot(` + Headers { + Symbol(map): Object { + "Cookie": Array [ + "__session=some_value; Path=/; Secure; HttpOnly; MaxAge=7200; SameSite=Lax", + "__other=some_other_value; Path=/; Secure; HttpOnly; MaxAge=3600; SameSite=Lax", + ], + }, + } + `); + }); + }); +}); + +describe("architect createRemixRequest", () => { + it.todo("creates a request with the correct headers"); +}); diff --git a/packages/remix-architect/package.json b/packages/remix-architect/package.json index 261fbdff41b..d139d9aedd1 100644 --- a/packages/remix-architect/package.json +++ b/packages/remix-architect/package.json @@ -4,6 +4,7 @@ "version": "0.18.0-pre.0", "repository": "https://github.com/remix-run/packages", "dependencies": { + "@types/aws-lambda": "^8.10.82", "@remix-run/node": "0.18.0-pre.0" }, "peerDependencies": { diff --git a/packages/remix-architect/server.ts b/packages/remix-architect/server.ts index e5c87619f59..f1e407785c0 100644 --- a/packages/remix-architect/server.ts +++ b/packages/remix-architect/server.ts @@ -1,13 +1,15 @@ import { URL } from "url"; -import type { - Request as ArcRequest, - Response as ArcResponse -} from "@architect/functions"; import type { ServerBuild, AppLoadContext } from "@remix-run/node"; import { + Headers, Request, createRequestHandler as createRemixRequestHandler } from "@remix-run/node"; +import { + APIGatewayProxyEventHeaders, + APIGatewayProxyEventV2, + APIGatewayProxyHandlerV2 +} from "aws-lambda"; /** * A function that returns the value to use as `context` in route `loader` and @@ -17,7 +19,7 @@ import { * environment/platform-specific values through to your loader/action. */ export interface GetLoadContextFunction { - (req: ArcRequest): AppLoadContext; + (event: APIGatewayProxyEventV2): AppLoadContext; } export type RequestHandler = ReturnType; @@ -34,13 +36,13 @@ export function createRequestHandler({ build: ServerBuild; getLoadContext: GetLoadContextFunction; mode?: string; -}) { +}): APIGatewayProxyHandlerV2 { let handleRequest = createRemixRequestHandler(build, mode); - return async (req: ArcRequest): Promise => { - let request = createRemixRequest(req); + return async (event, _context) => { + let request = createRemixRequest(event); let loadContext = - typeof getLoadContext === "function" ? getLoadContext(req) : undefined; + typeof getLoadContext === "function" ? getLoadContext(event) : undefined; let response = await handleRequest(request, loadContext); @@ -52,19 +54,39 @@ export function createRequestHandler({ }; } -function createRemixRequest(req: ArcRequest): Request { - let host = req.headers["x-forwarded-host"] || req.headers.host; - let search = req.rawQueryString.length ? "?" + req.rawQueryString : ""; - let url = new URL(req.rawPath + search, `https://${host}`); +export function createRemixHeaders( + requestHeaders: APIGatewayProxyEventHeaders, + requestCookies?: string[] +): Headers { + let headers = new Headers(); + + for (let [header, value] of Object.entries(requestHeaders)) { + if (value) { + headers.append(header, value); + } + } + + if (requestCookies) { + for (const cookie of requestCookies) { + headers.append("Cookie", cookie); + } + } + + return headers; +} + +export function createRemixRequest(event: APIGatewayProxyEventV2): Request { + let host = event.headers["x-forwarded-host"] || event.headers.host; + let proto = event.headers["x-forwarded-proto"] || "https"; + let search = event.rawQueryString.length ? `?${event.rawQueryString}` : ""; + let url = new URL(event.rawPath + search, `${proto}://${host}`); return new Request(url.toString(), { - method: req.requestContext.http.method, - headers: req.cookies - ? { ...req.headers, Cookie: req.cookies.join(";") } - : req.headers, + method: event.requestContext.http.method, + headers: createRemixHeaders(event.headers, event.cookies), body: - req.body && req.isBase64Encoded - ? Buffer.from(req.body, "base64").toString() - : req.body + event.body && event.isBase64Encoded + ? Buffer.from(event.body, "base64").toString() + : event.body }); } diff --git a/packages/remix-architect/tsconfig.json b/packages/remix-architect/tsconfig.json index 72da2ef0c76..0051d59df43 100644 --- a/packages/remix-architect/tsconfig.json +++ b/packages/remix-architect/tsconfig.json @@ -1,5 +1,5 @@ { - "include": ["../../types/architect__functions.d.ts", "**/*"], + "include": ["**/*"], "compilerOptions": { "lib": ["ES2019"], "target": "ES2019", diff --git a/types/architect__functions.d.ts b/types/architect__functions.d.ts deleted file mode 100644 index afa214d28eb..00000000000 --- a/types/architect__functions.d.ts +++ /dev/null @@ -1,132 +0,0 @@ -declare module "@architect/functions" { - /** - * Requests are passed to your handler function in an object, and include the following parameters - */ - - export interface Request { - /** - * Payload version (e.g. 2.0) - */ - version: string; - - /** - * Tuple of HTTP method (GET, POST, PATCH, PUT, or DELETE) and path; URL - * params are surrounded in braces If path is not captured by a specific - * function, routeKey will be $default (and be handled by the get / function) - * - * Example: GET /, GET /shop/{product} - */ - routeKey: string; - - /** - * The absolute path of the request - * - * Example: /, /shop/chocolate-chip-cookies - */ - rawPath: string; - - /** - * Any URL params, if defined in your HTTP function's path (e.g. product in /shop/:product) - * - * Example: { product: 'chocolate-chip-cookies' } - */ - pathParameters?: { [param: string]: string }; - - /** - * String containing query string params of request, if any - * - * Example: ?someParam=someValue, '' (if none) - */ - rawQueryString: string; - - /** - * Any query params if present in the client request - * - * Example: { someParam: someValue } - */ - queryStringParameters?: { [param: string]: string }; - - /** - * Array containing all cookies, if present in client request - * - * Example: [ 'some_cookie_name=some_cookie_value' ] - */ - cookies?: string[]; - - /** - * All client request headers - * - * Example: { 'accept-encoding': 'gzip' } - */ - headers: { [header: string]: string }; - - /** - * Request metadata, including http object containing method and path (should - * you not want to parse the routeKey) - */ - requestContext: { - http: { - method: string; - path: string; - routeKey: string; - }; - }; - - /** - * Contains unparsed, base64-encoded request body - * We suggest parsing with a body parser helper - */ - body?: string; - - /** - * Indicates whether body is base64-encoded binary payload - */ - isBase64Encoded: boolean; - } - - export interface Response { - /** - * Sets the HTTP status code; usually to 200 - */ - statusCode: number; - - /** - * All response headers - */ - headers?: { [header: string]: string }; - - /** - * Contains response body, either as a plain string, or, if binary, a - * base64-encoded buffer - * - * Note: The maximum body payload size is 6MB; files being delivered - * non-dynamically should use the Begin CDN - */ - body?: string; - - /** - * Indicates whether body is base64-encoded binary payload; defaults to false - * - * Required to be set to true if emitting a binary payload - */ - isBase64Encoded?: boolean; - } - - // There's a lot more, but this is all we use - export interface Arc { - http: Http; - } - - type session = { [key: string]: string }; - - interface Http { - session: { - write: (session: session) => Promise; - read: (request: Request) => Promise; - }; - } - - const arc: Arc; - - export default arc; -} diff --git a/yarn.lock b/yarn.lock index 03ed19da3e9..26cd3f385ac 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1212,6 +1212,11 @@ dependencies: "@sinonjs/commons" "^1.7.0" +"@types/aws-lambda@^8.10.82": + version "8.10.82" + resolved "https://registry.yarnpkg.com/@types/aws-lambda/-/aws-lambda-8.10.82.tgz#336062d3270f52d2156eeb7d9e497fd63684572a" + integrity sha512-sJo8pz8hu+OzLRAj7Do2g66zYLizWtB3kGK6K45RWmGW+S54XXMoK3sNbvzKXfndBxYiSVExHoCNiSlt2gPmxw== + "@types/babel__core@^7.0.0", "@types/babel__core@^7.1.7": version "7.1.12" resolved "https://registry.yarnpkg.com/@types/babel__core/-/babel__core-7.1.12.tgz#4d8e9e51eb265552a7e4f1ff2219ab6133bdfb2d" From 97c515ce08c2f918600303be344a1d4a7af5db6f Mon Sep 17 00:00:00 2001 From: Logan McAnsh Date: Tue, 10 Aug 2021 14:50:43 -0400 Subject: [PATCH 05/33] test(vercel): extract headers conversion function, start adding tests around it Signed-off-by: Logan McAnsh --- .../remix-vercel/__tests__/server-test.ts | 147 ++++++++++++++++++ packages/remix-vercel/package.json | 4 +- packages/remix-vercel/server.ts | 24 +-- yarn.lock | 12 ++ 4 files changed, 177 insertions(+), 10 deletions(-) create mode 100644 packages/remix-vercel/__tests__/server-test.ts diff --git a/packages/remix-vercel/__tests__/server-test.ts b/packages/remix-vercel/__tests__/server-test.ts new file mode 100644 index 00000000000..8451103f19e --- /dev/null +++ b/packages/remix-vercel/__tests__/server-test.ts @@ -0,0 +1,147 @@ +import { Response, Headers, RequestInfo, RequestInit } from "@remix-run/node"; +import { createRequestHandler as createRemixRequestHandler } from "@remix-run/node/server"; +// @ts-ignore soon +import { createServerWithHelpers } from "@vercel/node/dist/helpers"; +import listen from "test-listen"; +import fetch from "node-fetch"; + +import { createRemixHeaders, createRequestHandler } from "../server"; + +// We don't want to test that the remix server works here (that's what the +// puppetteer tests do), we just want to test the express adapter +jest.mock("@remix-run/node/server"); +let mockedCreateRequestHandler = createRemixRequestHandler as jest.MockedFunction< + typeof createRemixRequestHandler +>; + +const consumeEventMock = jest.fn(); +const mockBridge = { consumeEvent: consumeEventMock }; + +async function fetchWithProxyReq(_url: RequestInfo, opts: RequestInit = {}) { + if (opts.body) { + // eslint-disable-next-line + // @ts-ignore look into + opts = { ...opts, body: Buffer.from(opts.body) }; + } + + consumeEventMock.mockImplementationOnce(() => opts); + + return fetch(_url, { + ...opts, + headers: { ...opts.headers, "x-now-bridge-request-id": "2" } + }); +} + +async function createApp() { + const server = createServerWithHelpers((req: any, res: any) => { + // We don't have a real app to test, but it doesn't matter. We + // won't ever call through to the real createRequestHandler + // @ts-expect-error + return createRequestHandler({ build: undefined })(req, res); + }, mockBridge); + + const url = listen(server); + + return url; +} + +describe("vercel createRequestHandler", () => { + afterEach(() => { + mockedCreateRequestHandler.mockReset(); + }); + + afterAll(() => { + jest.restoreAllMocks(); + }); + + it("handles a request", async () => { + mockedCreateRequestHandler.mockImplementation(() => async req => { + return new Response(`URL: ${new URL(req.url).pathname}`); + }); + + const url = await createApp(); + + const res = await fetchWithProxyReq(url + "/foo/bar"); + + expect(res.status).toBe(200); + expect(await res.text()).toBe("URL: /foo/bar"); + }); + + it("handles a request with multiple Set-Cookie headers", async () => { + mockedCreateRequestHandler.mockImplementation(() => async req => { + const headers = new Headers(req.headers); + headers.append("Cache-Control", "maxage=300"); + headers.append( + "Set-Cookie", + "__session=some_value; Path=/; Secure; HttpOnly; MaxAge=7200; SameSite=Lax" + ); + headers.append( + "Set-Cookie", + "__other=some_other_value; Path=/; Secure; HttpOnly; MaxAge=3600; SameSite=Lax" + ); + + return new Response("check out these headerssss", { headers }); + }); + + const url = await createApp(); + + const res = await fetchWithProxyReq(url + "/foo/bar"); + + expect(res.status).toBe(200); + expect(res.headers.get("Cache-Control")).toBe("maxage=300"); + expect(res.headers.get("Set-Cookie")).toMatchInlineSnapshot( + `"__session=some_value; Path=/; Secure; HttpOnly; MaxAge=7200; SameSite=Lax, __other=some_other_value; Path=/; Secure; HttpOnly; MaxAge=3600; SameSite=Lax"` + ); + }); +}); + +describe("vercel createRemixHeaders", () => { + afterEach(() => { + mockedCreateRequestHandler.mockReset(); + }); + + afterAll(() => { + jest.restoreAllMocks(); + }); + + it("creates fetch headers from vercel headers", async () => { + expect( + createRemixHeaders({ + "Cache-Control": "maxage=300" + }) + ).toMatchInlineSnapshot(` + Headers { + Symbol(map): Object { + "Cache-Control": Array [ + "maxage=300", + ], + }, + } + `); + }); + + it.todo("handles simple headers"); + it.todo("handles multiple headers"); + it.todo("handles headers with multiple values"); + it.todo("handles headers with multiple values and multiple headers"); + it("handles multiple set-cookie headers", () => { + expect( + createRemixHeaders({ + "Set-Cookie": ["foo=bar", "bar=baz"] + }) + ).toMatchInlineSnapshot(` + Headers { + Symbol(map): Object { + "Set-Cookie": Array [ + "foo=bar", + "bar=baz", + ], + }, + } + `); + }); +}); + +describe("vercel createRemixRequest", () => { + it.todo("creates a request with the correct headers"); +}); diff --git a/packages/remix-vercel/package.json b/packages/remix-vercel/package.json index 0f2c96ebfc6..648afb149eb 100644 --- a/packages/remix-vercel/package.json +++ b/packages/remix-vercel/package.json @@ -10,6 +10,8 @@ "@vercel/node": "^1.8.3" }, "devDependencies": { - "@vercel/node": "^1.8.3" + "@types/test-listen": "^1.1.0", + "@vercel/node": "^1.8.3", + "test-listen": "^1.1.0" } } diff --git a/packages/remix-vercel/server.ts b/packages/remix-vercel/server.ts index 4e6293e0b5b..51034f8afb5 100644 --- a/packages/remix-vercel/server.ts +++ b/packages/remix-vercel/server.ts @@ -53,15 +53,12 @@ export function createRequestHandler({ }; } -function createRemixRequest(req: NowRequest): Request { - let host = req.headers["x-forwarded-host"] || req.headers["host"]; - // doesn't seem to be available on their req object! - let protocol = req.headers["x-forwarded-proto"] || "https"; - let url = new URL(req.url!, `${protocol}://${host}`); - +export function createRemixHeaders( + requestHeaders: NowRequest["headers"] +): Headers { let headers = new Headers(); - for (let key in req.headers) { - let header = req.headers[key]!; + for (let key in requestHeaders) { + let header = requestHeaders[key]!; // set-cookie is an array (maybe others) if (Array.isArray(header)) { for (let value of header) { @@ -72,9 +69,18 @@ function createRemixRequest(req: NowRequest): Request { } } + return headers; +} + +export function createRemixRequest(req: NowRequest): Request { + let host = req.headers["x-forwarded-host"] || req.headers["host"]; + // doesn't seem to be available on their req object! + let protocol = req.headers["x-forwarded-proto"] || "https"; + let url = new URL(req.url!, `${protocol}://${host}`); + let init: RequestInit = { method: req.method, - headers: headers + headers: createRemixHeaders(req.headers) }; if (req.method !== "GET" && req.method !== "HEAD") { diff --git a/yarn.lock b/yarn.lock index 26cd3f385ac..8f2802352d9 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1574,6 +1574,13 @@ dependencies: "@types/superagent" "*" +"@types/test-listen@^1.1.0": + version "1.1.0" + resolved "https://registry.yarnpkg.com/@types/test-listen/-/test-listen-1.1.0.tgz#db7e5b0e277b4a3ee7b90770dae1a41b186458af" + integrity sha512-y6ZfbSzYHniCeY6ZAzsQjSAdJInNVoEz4Uhsb81W+RCoNYA59yoG/+XbqPqCPj2KCU3Wa6RFWSozutkGIHIsNQ== + dependencies: + "@types/node" "*" + "@types/through@*": version "0.0.30" resolved "https://registry.yarnpkg.com/@types/through/-/through-0.0.30.tgz#e0e42ce77e897bd6aead6f6ea62aeb135b8a3895" @@ -7381,6 +7388,11 @@ test-exclude@^6.0.0: glob "^7.1.4" minimatch "^3.0.4" +test-listen@^1.1.0: + version "1.1.0" + resolved "https://registry.yarnpkg.com/test-listen/-/test-listen-1.1.0.tgz#2ba614d96c3bc9157469003027b42a495dd83b6a" + integrity sha512-OyEVi981C1sb9NX1xayfgZls3p8QTDRwp06EcgxSgd1kktaENBW8dO15i8v/7Fi15j0IYQctJzk5J+hyEBId2w== + text-table@^0.2.0: version "0.2.0" resolved "https://registry.yarnpkg.com/text-table/-/text-table-0.2.0.tgz#7f5ee823ae805207c00af2df4a84ec3fcfa570b4" From e7e69fdc9c4f1393cc63a2177013828b536db988 Mon Sep 17 00:00:00 2001 From: Logan McAnsh Date: Tue, 10 Aug 2021 14:57:50 -0400 Subject: [PATCH 06/33] test(vercel): shutdown server after running test Signed-off-by: Logan McAnsh --- .../remix-vercel/__tests__/server-test.ts | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/packages/remix-vercel/__tests__/server-test.ts b/packages/remix-vercel/__tests__/server-test.ts index 8451103f19e..974d2c26326 100644 --- a/packages/remix-vercel/__tests__/server-test.ts +++ b/packages/remix-vercel/__tests__/server-test.ts @@ -14,8 +14,10 @@ let mockedCreateRequestHandler = createRemixRequestHandler as jest.MockedFunctio typeof createRemixRequestHandler >; -const consumeEventMock = jest.fn(); -const mockBridge = { consumeEvent: consumeEventMock }; +let consumeEventMock = jest.fn(); +let mockBridge = { consumeEvent: consumeEventMock }; +let server: any; +let url: string; async function fetchWithProxyReq(_url: RequestInfo, opts: RequestInit = {}) { if (opts.body) { @@ -33,21 +35,24 @@ async function fetchWithProxyReq(_url: RequestInfo, opts: RequestInit = {}) { } async function createApp() { - const server = createServerWithHelpers((req: any, res: any) => { + server = createServerWithHelpers((req: any, res: any) => { // We don't have a real app to test, but it doesn't matter. We // won't ever call through to the real createRequestHandler // @ts-expect-error return createRequestHandler({ build: undefined })(req, res); }, mockBridge); - const url = listen(server); - - return url; + url = await listen(server); } +beforeEach(() => { + consumeEventMock.mockClear(); +}); + describe("vercel createRequestHandler", () => { - afterEach(() => { + afterEach(async () => { mockedCreateRequestHandler.mockReset(); + await server.close(); }); afterAll(() => { @@ -59,7 +64,7 @@ describe("vercel createRequestHandler", () => { return new Response(`URL: ${new URL(req.url).pathname}`); }); - const url = await createApp(); + await createApp(); const res = await fetchWithProxyReq(url + "/foo/bar"); @@ -83,7 +88,7 @@ describe("vercel createRequestHandler", () => { return new Response("check out these headerssss", { headers }); }); - const url = await createApp(); + await createApp(); const res = await fetchWithProxyReq(url + "/foo/bar"); From bbb6cd4a63b8075551b33c1d87041b146ab493d8 Mon Sep 17 00:00:00 2001 From: Logan McAnsh Date: Tue, 10 Aug 2021 15:51:14 -0400 Subject: [PATCH 07/33] chore(architect): use requestContent protocol; add test for arc createRemixRequest Signed-off-by: Logan McAnsh --- .../remix-architect/__tests__/server-test.ts | 144 +++++++++++++++--- packages/remix-architect/server.ts | 2 +- .../remix-vercel/__tests__/server-test.ts | 2 +- 3 files changed, 123 insertions(+), 25 deletions(-) diff --git a/packages/remix-architect/__tests__/server-test.ts b/packages/remix-architect/__tests__/server-test.ts index 7ad5ff01671..e0127a6553b 100644 --- a/packages/remix-architect/__tests__/server-test.ts +++ b/packages/remix-architect/__tests__/server-test.ts @@ -8,7 +8,7 @@ import { } from "../server"; // We don't want to test that the remix server works here (that's what the -// puppetteer tests do), we just want to test the express adapter +// puppetteer tests do), we just want to test the architect adapter jest.mock("@remix-run/node/server"); let mockedCreateRequestHandler = createRemixRequestHandler as jest.MockedFunction< typeof createRemixRequestHandler @@ -85,27 +85,27 @@ describe("architect createRemixHeaders", () => { `); }); - // it.skip("handles multiple set-cookie headers", () => { - // expect( - // createRemixHeaders( - // { - // "set-cookie": [ - // "__session=some_value; Path=/; Secure; HttpOnly; MaxAge=7200; SameSite=Lax", - // "__other=some_other_value; Path=/; Secure; HttpOnly; MaxAge=3600; SameSite=Lax" - // ] - // }, - // undefined - // ) - // ).toMatchInlineSnapshot(` - // Headers { - // Symbol(map): Object { - // "set-cookie": Array [ - // "__session=some_value; Path=/; Secure; HttpOnly; MaxAge=7200; SameSite=Lax,__other=some_other_value; Path=/; Secure; HttpOnly; MaxAge=3600; SameSite=Lax", - // ], - // }, - // } - // `); - // }); + it("handles multiple set-cookie headers", () => { + expect( + createRemixHeaders( + { + "set-cookie": [ + "__session=some_value; Path=/; Secure; HttpOnly; MaxAge=7200; SameSite=Lax", + "__other=some_other_value; Path=/; Secure; HttpOnly; MaxAge=3600; SameSite=Lax" + ] + }, + undefined + ) + ).toMatchInlineSnapshot(` + Headers { + Symbol(map): Object { + "set-cookie": Array [ + "__session=some_value; Path=/; Secure; HttpOnly; MaxAge=7200; SameSite=Lax,__other=some_other_value; Path=/; Secure; HttpOnly; MaxAge=3600; SameSite=Lax", + ], + }, + } + `); + }); it("handles cookies", () => { expect( @@ -128,5 +128,103 @@ describe("architect createRemixHeaders", () => { }); describe("architect createRemixRequest", () => { - it.todo("creates a request with the correct headers"); + it("creates a request with the correct headers", () => { + expect( + createRemixRequest({ + headers: { + host: "localhost:3333", + accept: + "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8", + "upgrade-insecure-requests": "1", + "user-agent": + "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/15.0 Safari/605.1.15", + "accept-language": "en-US,en;q=0.9", + "accept-encoding": "gzip, deflate" + }, + isBase64Encoded: false, + rawPath: "/", + rawQueryString: "", + requestContext: { + http: { + method: "GET", + path: "/", + protocol: "http", + userAgent: + "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/15.0 Safari/605.1.15", + sourceIp: "127.0.0.1" + }, + routeKey: "ANY /{proxy+}", + accountId: "1234567890", + requestId: "abcdefghijklmnopqrstuvwxyz", + apiId: "1234567890abcdef", + domainName: "localhost:3333", + domainPrefix: "localhost:3333", + stage: "prod", + time: "2021-08-10T19:48:50.969Z", + timeEpoch: 1628624930969 + }, + routeKey: "foo", + version: "2.0", + cookies: ["__session=value"] + }) + ).toMatchInlineSnapshot(` + Request { + "agent": undefined, + "compress": true, + "counter": 0, + "follow": 20, + "size": 0, + "timeout": 0, + Symbol(Body internals): Object { + "body": null, + "disturbed": false, + "error": null, + }, + Symbol(Request internals): Object { + "headers": Headers { + Symbol(map): Object { + "Cookie": Array [ + "__session=value", + ], + "accept": Array [ + "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8", + ], + "accept-encoding": Array [ + "gzip, deflate", + ], + "accept-language": Array [ + "en-US,en;q=0.9", + ], + "host": Array [ + "localhost:3333", + ], + "upgrade-insecure-requests": Array [ + "1", + ], + "user-agent": Array [ + "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/15.0 Safari/605.1.15", + ], + }, + }, + "method": "GET", + "parsedURL": Url { + "auth": null, + "hash": null, + "host": "localhost:3333", + "hostname": "localhost", + "href": "http://localhost:3333/", + "path": "/", + "pathname": "/", + "port": "3333", + "protocol": "http:", + "query": null, + "search": null, + "slashes": true, + }, + "redirect": "follow", + "signal": null, + }, + } + `); + }); }); diff --git a/packages/remix-architect/server.ts b/packages/remix-architect/server.ts index f1e407785c0..35bba4f81a6 100644 --- a/packages/remix-architect/server.ts +++ b/packages/remix-architect/server.ts @@ -77,7 +77,7 @@ export function createRemixHeaders( export function createRemixRequest(event: APIGatewayProxyEventV2): Request { let host = event.headers["x-forwarded-host"] || event.headers.host; - let proto = event.headers["x-forwarded-proto"] || "https"; + let proto = event.requestContext.http.protocol || "https"; let search = event.rawQueryString.length ? `?${event.rawQueryString}` : ""; let url = new URL(event.rawPath + search, `${proto}://${host}`); diff --git a/packages/remix-vercel/__tests__/server-test.ts b/packages/remix-vercel/__tests__/server-test.ts index 974d2c26326..78939e1b092 100644 --- a/packages/remix-vercel/__tests__/server-test.ts +++ b/packages/remix-vercel/__tests__/server-test.ts @@ -8,7 +8,7 @@ import fetch from "node-fetch"; import { createRemixHeaders, createRequestHandler } from "../server"; // We don't want to test that the remix server works here (that's what the -// puppetteer tests do), we just want to test the express adapter +// puppetteer tests do), we just want to test the vercel adapter jest.mock("@remix-run/node/server"); let mockedCreateRequestHandler = createRemixRequestHandler as jest.MockedFunction< typeof createRemixRequestHandler From 682e82d3cfdba2cfdf0f2f986cccf8d53e48c305 Mon Sep 17 00:00:00 2001 From: Logan McAnsh Date: Tue, 10 Aug 2021 15:56:12 -0400 Subject: [PATCH 08/33] test(architect): remove tes as I was handling it in the wrong direction Signed-off-by: Logan McAnsh --- .../remix-architect/__tests__/server-test.ts | 22 ------------------- 1 file changed, 22 deletions(-) diff --git a/packages/remix-architect/__tests__/server-test.ts b/packages/remix-architect/__tests__/server-test.ts index e0127a6553b..ec1571b2521 100644 --- a/packages/remix-architect/__tests__/server-test.ts +++ b/packages/remix-architect/__tests__/server-test.ts @@ -85,28 +85,6 @@ describe("architect createRemixHeaders", () => { `); }); - it("handles multiple set-cookie headers", () => { - expect( - createRemixHeaders( - { - "set-cookie": [ - "__session=some_value; Path=/; Secure; HttpOnly; MaxAge=7200; SameSite=Lax", - "__other=some_other_value; Path=/; Secure; HttpOnly; MaxAge=3600; SameSite=Lax" - ] - }, - undefined - ) - ).toMatchInlineSnapshot(` - Headers { - Symbol(map): Object { - "set-cookie": Array [ - "__session=some_value; Path=/; Secure; HttpOnly; MaxAge=7200; SameSite=Lax,__other=some_other_value; Path=/; Secure; HttpOnly; MaxAge=3600; SameSite=Lax", - ], - }, - } - `); - }); - it("handles cookies", () => { expect( createRemixHeaders({}, [ From b97c453cb62441d01194542ec43079c4402e999b Mon Sep 17 00:00:00 2001 From: Logan McAnsh Date: Tue, 10 Aug 2021 16:16:47 -0400 Subject: [PATCH 09/33] chore: remove `Object.fromEntries(responseHeaders)` spreading Signed-off-by: Logan McAnsh --- docs/guides/styling.md | 7 +++---- fixtures/gists-app/app/entry.server.jsx | 7 +++---- fixtures/tutorial/app/entry.server.tsx | 7 +++---- .../remix-init/templates/_shared_js/app/entry.server.jsx | 7 +++---- .../remix-init/templates/_shared_ts/app/entry.server.tsx | 7 +++---- packages/remix-node/server.ts | 7 +++---- 6 files changed, 18 insertions(+), 24 deletions(-) diff --git a/docs/guides/styling.md b/docs/guides/styling.md index 96ef2e651fb..a0604257c89 100644 --- a/docs/guides/styling.md +++ b/docs/guides/styling.md @@ -222,12 +222,11 @@ Here's some sample code to show how you might use Styled Components with Remix: ); + responseHeaders.set("Content-Type","text/html") + return new Response("" + markup, { status: responseStatusCode, - headers: { - ...Object.fromEntries(responseHeaders), - "Content-Type": "text/html" - } + headers: responseHeaders }); } ``` diff --git a/fixtures/gists-app/app/entry.server.jsx b/fixtures/gists-app/app/entry.server.jsx index b79eedb9841..2a902ea1764 100644 --- a/fixtures/gists-app/app/entry.server.jsx +++ b/fixtures/gists-app/app/entry.server.jsx @@ -11,11 +11,10 @@ export default function handleRequest( ); + responseHeaders.set("Content-Type", "text/html"); + return new Response("" + markup, { status: responseStatusCode, - headers: { - ...Object.fromEntries(responseHeaders), - "Content-Type": "text/html" - } + headers: responseHeaders }); } diff --git a/fixtures/tutorial/app/entry.server.tsx b/fixtures/tutorial/app/entry.server.tsx index edf2cc0c15f..1c277b26931 100644 --- a/fixtures/tutorial/app/entry.server.tsx +++ b/fixtures/tutorial/app/entry.server.tsx @@ -12,11 +12,10 @@ export default function handleRequest( ); + responseHeaders.set("Content-Type", "text/html"); + return new Response("" + markup, { status: responseStatusCode, - headers: { - ...Object.fromEntries(responseHeaders), - "Content-Type": "text/html" - } + headers: responseHeaders }); } diff --git a/packages/remix-init/templates/_shared_js/app/entry.server.jsx b/packages/remix-init/templates/_shared_js/app/entry.server.jsx index b79eedb9841..2a902ea1764 100644 --- a/packages/remix-init/templates/_shared_js/app/entry.server.jsx +++ b/packages/remix-init/templates/_shared_js/app/entry.server.jsx @@ -11,11 +11,10 @@ export default function handleRequest( ); + responseHeaders.set("Content-Type", "text/html"); + return new Response("" + markup, { status: responseStatusCode, - headers: { - ...Object.fromEntries(responseHeaders), - "Content-Type": "text/html" - } + headers: responseHeaders }); } diff --git a/packages/remix-init/templates/_shared_ts/app/entry.server.tsx b/packages/remix-init/templates/_shared_ts/app/entry.server.tsx index edf2cc0c15f..1c277b26931 100644 --- a/packages/remix-init/templates/_shared_ts/app/entry.server.tsx +++ b/packages/remix-init/templates/_shared_ts/app/entry.server.tsx @@ -12,11 +12,10 @@ export default function handleRequest( ); + responseHeaders.set("Content-Type", "text/html"); + return new Response("" + markup, { status: responseStatusCode, - headers: { - ...Object.fromEntries(responseHeaders), - "Content-Type": "text/html" - } + headers: responseHeaders }); } diff --git a/packages/remix-node/server.ts b/packages/remix-node/server.ts index 846ce92b2c1..9dbf6652bb4 100644 --- a/packages/remix-node/server.ts +++ b/packages/remix-node/server.ts @@ -111,12 +111,11 @@ async function handleDataRequest( let locationHeader = response.headers.get("Location"); response.headers.delete("Location"); + response.headers.set("X-Remix-Redirect", locationHeader!); + return new Response("", { status: 204, - headers: { - ...Object.fromEntries(response.headers), - "X-Remix-Redirect": locationHeader! - } + headers: response.headers }); } From 8f5f397dd04f6708286e87f47f7151af906a7e20 Mon Sep 17 00:00:00 2001 From: Logan McAnsh Date: Tue, 10 Aug 2021 19:36:44 -0400 Subject: [PATCH 10/33] test(express): update createRemixHeaders to support multiple headers with the same name Signed-off-by: Logan McAnsh --- .../remix-express/__tests__/server-test.ts | 3 ++- packages/remix-express/server.ts | 24 ++++++++++--------- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/packages/remix-express/__tests__/server-test.ts b/packages/remix-express/__tests__/server-test.ts index fd91129d957..08b8fed1dd1 100644 --- a/packages/remix-express/__tests__/server-test.ts +++ b/packages/remix-express/__tests__/server-test.ts @@ -171,7 +171,8 @@ describe("express createRemixHeaders", () => { Headers { Symbol(map): Object { "set-cookie": Array [ - "__session=some_value; Path=/; Secure; HttpOnly; MaxAge=7200; SameSite=Lax,__other=some_other_value; Path=/; Secure; HttpOnly; MaxAge=3600; SameSite=Lax", + "__session=some_value; Path=/; Secure; HttpOnly; MaxAge=7200; SameSite=Lax", + "__other=some_other_value; Path=/; Secure; HttpOnly; MaxAge=3600; SameSite=Lax", ], }, } diff --git a/packages/remix-express/server.ts b/packages/remix-express/server.ts index e2cfd162d53..7d426f243da 100644 --- a/packages/remix-express/server.ts +++ b/packages/remix-express/server.ts @@ -67,19 +67,21 @@ export function createRequestHandler({ export function createRemixHeaders( requestHeaders: express.Request["headers"] ): Headers { - return new Headers( - Object.keys(requestHeaders).reduce((memo, key) => { - let value = requestHeaders[key]; - - if (typeof value === "string") { - memo[key] = value; - } else if (Array.isArray(value)) { - memo[key] = value.join(","); + let headers = new Headers(); + + for (let [key, values] of Object.entries(requestHeaders)) { + if (!values) break; + + if (Array.isArray(values)) { + for (const value of values) { + headers.append(key, value); } + } else { + headers.set(key, values); + } + } - return memo; - }, {} as { [headerName: string]: string }) - ); + return headers; } export function createRemixRequest(req: express.Request): Request { From 651f2684b99a816b48d8c1353275b2fdb4ae8694 Mon Sep 17 00:00:00 2001 From: Logan McAnsh Date: Tue, 10 Aug 2021 19:47:59 -0400 Subject: [PATCH 11/33] test(vercel): add more tests Signed-off-by: Logan McAnsh --- .../remix-vercel/__tests__/server-test.ts | 222 +++++++++++------- 1 file changed, 134 insertions(+), 88 deletions(-) diff --git a/packages/remix-vercel/__tests__/server-test.ts b/packages/remix-vercel/__tests__/server-test.ts index 78939e1b092..99f29900e95 100644 --- a/packages/remix-vercel/__tests__/server-test.ts +++ b/packages/remix-vercel/__tests__/server-test.ts @@ -1,6 +1,6 @@ -import { Response, Headers, RequestInfo, RequestInit } from "@remix-run/node"; +import { Headers, Response, RequestInfo, RequestInit } from "@remix-run/node"; import { createRequestHandler as createRemixRequestHandler } from "@remix-run/node/server"; -// @ts-ignore soon +// @ts-expect-error TODO: add type definition for this import { createServerWithHelpers } from "@vercel/node/dist/helpers"; import listen from "test-listen"; import fetch from "node-fetch"; @@ -14,10 +14,11 @@ let mockedCreateRequestHandler = createRemixRequestHandler as jest.MockedFunctio typeof createRemixRequestHandler >; -let consumeEventMock = jest.fn(); -let mockBridge = { consumeEvent: consumeEventMock }; +// TODO: add real type definition let server: any; let url: string; +let consumeEventMock = jest.fn(); +let mockBridge = { consumeEvent: consumeEventMock }; async function fetchWithProxyReq(_url: RequestInfo, opts: RequestInit = {}) { if (opts.body) { @@ -34,22 +35,20 @@ async function fetchWithProxyReq(_url: RequestInfo, opts: RequestInit = {}) { }); } -async function createApp() { - server = createServerWithHelpers((req: any, res: any) => { - // We don't have a real app to test, but it doesn't matter. We - // won't ever call through to the real createRequestHandler - // @ts-expect-error - return createRequestHandler({ build: undefined })(req, res); - }, mockBridge); +describe("vercel createRequestHandler", () => { + beforeEach(async () => { + consumeEventMock.mockClear(); - url = await listen(server); -} + server = createServerWithHelpers((req: any, res: any) => { + // We don't have a real app to test, but it doesn't matter. We + // won't ever call through to the real createRequestHandler + // @ts-expect-error + return createRequestHandler({ build: undefined })(req, res); + }, mockBridge); -beforeEach(() => { - consumeEventMock.mockClear(); -}); + url = await listen(server); + }); -describe("vercel createRequestHandler", () => { afterEach(async () => { mockedCreateRequestHandler.mockReset(); await server.close(); @@ -59,91 +58,138 @@ describe("vercel createRequestHandler", () => { jest.restoreAllMocks(); }); - it("handles a request", async () => { - mockedCreateRequestHandler.mockImplementation(() => async req => { - return new Response(`URL: ${new URL(req.url).pathname}`); - }); + describe("basic requests", () => { + it("handles requests", async () => { + mockedCreateRequestHandler.mockImplementation(() => async req => { + return new Response(`URL: ${new URL(req.url).pathname}`); + }); - await createApp(); + const res = await fetchWithProxyReq(url + "/foo/bar"); - const res = await fetchWithProxyReq(url + "/foo/bar"); + expect(res.status).toBe(200); + expect(await res.text()).toBe("URL: /foo/bar"); + }); - expect(res.status).toBe(200); - expect(await res.text()).toBe("URL: /foo/bar"); - }); + it("handles status codes", async () => { + mockedCreateRequestHandler.mockImplementation(() => async () => { + return new Response("", { status: 204 }); + }); - it("handles a request with multiple Set-Cookie headers", async () => { - mockedCreateRequestHandler.mockImplementation(() => async req => { - const headers = new Headers(req.headers); - headers.append("Cache-Control", "maxage=300"); - headers.append( - "Set-Cookie", - "__session=some_value; Path=/; Secure; HttpOnly; MaxAge=7200; SameSite=Lax" - ); - headers.append( - "Set-Cookie", - "__other=some_other_value; Path=/; Secure; HttpOnly; MaxAge=3600; SameSite=Lax" - ); + let res = await fetchWithProxyReq(url); - return new Response("check out these headerssss", { headers }); + expect(res.status).toBe(204); }); - await createApp(); - - const res = await fetchWithProxyReq(url + "/foo/bar"); - - expect(res.status).toBe(200); - expect(res.headers.get("Cache-Control")).toBe("maxage=300"); - expect(res.headers.get("Set-Cookie")).toMatchInlineSnapshot( - `"__session=some_value; Path=/; Secure; HttpOnly; MaxAge=7200; SameSite=Lax, __other=some_other_value; Path=/; Secure; HttpOnly; MaxAge=3600; SameSite=Lax"` - ); + it("sets headers", async () => { + mockedCreateRequestHandler.mockImplementation(() => async () => { + const headers = new Headers({ "X-Time-Of-Year": "most wonderful" }); + headers.append( + "Set-Cookie", + "first=one; Expires=0; Path=/; HttpOnly; Secure; SameSite=Lax" + ); + headers.append( + "Set-Cookie", + "second=two; MaxAge=1209600; Path=/; HttpOnly; Secure; SameSite=Lax" + ); + return new Response("", { headers }); + }); + + let res = await fetchWithProxyReq(url); + + expect(res.headers.get("x-time-of-year")).toBe("most wonderful"); + expect(res.headers.get("set-cookie")).toBe( + "first=one; Expires=0; Path=/; HttpOnly; Secure; SameSite=Lax, second=two; MaxAge=1209600; Path=/; HttpOnly; Secure; SameSite=Lax" + ); + }); }); }); describe("vercel createRemixHeaders", () => { - afterEach(() => { - mockedCreateRequestHandler.mockReset(); - }); + describe("creates fetch headers from vercel headers", () => { + it("handles empty headers", () => { + expect(createRemixHeaders({})).toMatchInlineSnapshot(` + Headers { + Symbol(map): Object {}, + } + `); + }); - afterAll(() => { - jest.restoreAllMocks(); - }); + it("handles simple headers", () => { + expect(createRemixHeaders({ "x-foo": "bar" })).toMatchInlineSnapshot(` + Headers { + Symbol(map): Object { + "x-foo": Array [ + "bar", + ], + }, + } + `); + }); - it("creates fetch headers from vercel headers", async () => { - expect( - createRemixHeaders({ - "Cache-Control": "maxage=300" - }) - ).toMatchInlineSnapshot(` - Headers { - Symbol(map): Object { - "Cache-Control": Array [ - "maxage=300", - ], - }, - } - `); - }); + it("handles multiple headers", () => { + expect(createRemixHeaders({ "x-foo": "bar", "x-bar": "baz" })) + .toMatchInlineSnapshot(` + Headers { + Symbol(map): Object { + "x-bar": Array [ + "baz", + ], + "x-foo": Array [ + "bar", + ], + }, + } + `); + }); + + it("handles headers with multiple values", () => { + expect(createRemixHeaders({ "x-foo": "bar, baz" })) + .toMatchInlineSnapshot(` + Headers { + Symbol(map): Object { + "x-foo": Array [ + "bar, baz", + ], + }, + } + `); + }); + + it("handles headers with multiple values and multiple headers", () => { + expect(createRemixHeaders({ "x-foo": "bar, baz", "x-bar": "baz" })) + .toMatchInlineSnapshot(` + Headers { + Symbol(map): Object { + "x-bar": Array [ + "baz", + ], + "x-foo": Array [ + "bar, baz", + ], + }, + } + `); + }); - it.todo("handles simple headers"); - it.todo("handles multiple headers"); - it.todo("handles headers with multiple values"); - it.todo("handles headers with multiple values and multiple headers"); - it("handles multiple set-cookie headers", () => { - expect( - createRemixHeaders({ - "Set-Cookie": ["foo=bar", "bar=baz"] - }) - ).toMatchInlineSnapshot(` - Headers { - Symbol(map): Object { - "Set-Cookie": Array [ - "foo=bar", - "bar=baz", - ], - }, - } - `); + it("handles multiple set-cookie headers", () => { + expect( + createRemixHeaders({ + "set-cookie": [ + "__session=some_value; Path=/; Secure; HttpOnly; MaxAge=7200; SameSite=Lax", + "__other=some_other_value; Path=/; Secure; HttpOnly; MaxAge=3600; SameSite=Lax" + ] + }) + ).toMatchInlineSnapshot(` + Headers { + Symbol(map): Object { + "set-cookie": Array [ + "__session=some_value; Path=/; Secure; HttpOnly; MaxAge=7200; SameSite=Lax", + "__other=some_other_value; Path=/; Secure; HttpOnly; MaxAge=3600; SameSite=Lax", + ], + }, + } + `); + }); }); }); From f8f52ab158c0ba2203b445ef7b71c82bc5e5a8fd Mon Sep 17 00:00:00 2001 From: Logan McAnsh Date: Wed, 11 Aug 2021 12:59:36 -0400 Subject: [PATCH 12/33] chore: ignore test directories when using tsc --- packages/remix-architect/tsconfig.json | 1 + packages/remix-express/tsconfig.json | 3 ++- packages/remix-vercel/tsconfig.json | 2 ++ 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/remix-architect/tsconfig.json b/packages/remix-architect/tsconfig.json index 0051d59df43..4674d442d62 100644 --- a/packages/remix-architect/tsconfig.json +++ b/packages/remix-architect/tsconfig.json @@ -1,5 +1,6 @@ { "include": ["**/*"], + "exclude": ["__tests__"], "compilerOptions": { "lib": ["ES2019"], "target": "ES2019", diff --git a/packages/remix-express/tsconfig.json b/packages/remix-express/tsconfig.json index 55305c7bbd0..8040ec74b95 100644 --- a/packages/remix-express/tsconfig.json +++ b/packages/remix-express/tsconfig.json @@ -1,5 +1,6 @@ { - "exclude": ["__tests__/**/*"], + "include": ["**/*"], + "exclude": ["__tests__"], "compilerOptions": { "lib": ["ES2019"], "target": "ES2019", diff --git a/packages/remix-vercel/tsconfig.json b/packages/remix-vercel/tsconfig.json index b694acc00af..331e6d5d9d1 100644 --- a/packages/remix-vercel/tsconfig.json +++ b/packages/remix-vercel/tsconfig.json @@ -1,4 +1,6 @@ { + "include": ["**/*"], + "exclude": ["__tests__"], "compilerOptions": { "lib": ["ES2019"], "target": "ES2019", From 01e513497f7a0e7155670ce967ce620ac85cb170 Mon Sep 17 00:00:00 2001 From: Logan McAnsh Date: Wed, 11 Aug 2021 14:56:38 -0400 Subject: [PATCH 13/33] feat(architect): make multi set-cookie headers work Signed-off-by: Logan McAnsh --- packages/remix-architect/server.ts | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/packages/remix-architect/server.ts b/packages/remix-architect/server.ts index 35bba4f81a6..a10a5d36ec3 100644 --- a/packages/remix-architect/server.ts +++ b/packages/remix-architect/server.ts @@ -46,9 +46,25 @@ export function createRequestHandler({ let response = await handleRequest(request, loadContext); + let cookies: string[] = []; + + // Arc/AWS API Gateway will send back set-cookies outside of response headers. + for (let [key, values] of Object.entries(response.headers.raw())) { + if (key.toLowerCase() === "set-cookie") { + for (let value of values) { + cookies.push(value); + } + } + } + + if (cookies.length) { + response.headers.delete("set-cookie"); + } + return { statusCode: response.status, headers: Object.fromEntries(response.headers), + cookies, body: await response.text() }; }; @@ -67,7 +83,7 @@ export function createRemixHeaders( } if (requestCookies) { - for (const cookie of requestCookies) { + for (let cookie of requestCookies) { headers.append("Cookie", cookie); } } From 091448f096d273ad79a3a2abd8e135e5e2f53bcd Mon Sep 17 00:00:00 2001 From: Logan McAnsh Date: Wed, 11 Aug 2021 14:56:52 -0400 Subject: [PATCH 14/33] feat(vercel): make multi set-cookie headers work Signed-off-by: Logan McAnsh --- packages/remix-vercel/__tests__/server-test.ts | 8 ++++++-- packages/remix-vercel/server.ts | 18 ++++-------------- 2 files changed, 10 insertions(+), 16 deletions(-) diff --git a/packages/remix-vercel/__tests__/server-test.ts b/packages/remix-vercel/__tests__/server-test.ts index 99f29900e95..3e3d75ae509 100644 --- a/packages/remix-vercel/__tests__/server-test.ts +++ b/packages/remix-vercel/__tests__/server-test.ts @@ -91,14 +91,18 @@ describe("vercel createRequestHandler", () => { "Set-Cookie", "second=two; MaxAge=1209600; Path=/; HttpOnly; Secure; SameSite=Lax" ); + headers.append( + "Set-Cookie", + "third=three; Expires=Wed, 21 Oct 2015 07:28:00 GMT; Path=/; HttpOnly; Secure; SameSite=Lax" + ); return new Response("", { headers }); }); let res = await fetchWithProxyReq(url); expect(res.headers.get("x-time-of-year")).toBe("most wonderful"); - expect(res.headers.get("set-cookie")).toBe( - "first=one; Expires=0; Path=/; HttpOnly; Secure; SameSite=Lax, second=two; MaxAge=1209600; Path=/; HttpOnly; Secure; SameSite=Lax" + expect(res.headers.get("set-cookie")).toEqual( + "first=one; Expires=0; Path=/; HttpOnly; Secure; SameSite=Lax, second=two; MaxAge=1209600; Path=/; HttpOnly; Secure; SameSite=Lax, third=three; Expires=Wed, 21 Oct 2015 07:28:00 GMT; Path=/; HttpOnly; Secure; SameSite=Lax" ); }); }); diff --git a/packages/remix-vercel/server.ts b/packages/remix-vercel/server.ts index 51034f8afb5..2674449b3b4 100644 --- a/packages/remix-vercel/server.ts +++ b/packages/remix-vercel/server.ts @@ -93,21 +93,11 @@ export function createRemixRequest(req: NowRequest): Request { function sendRemixResponse(res: NowResponse, response: Response): void { res.status(response.status); - let arrays = new Map(); - for (let [key, value] of response.headers.entries()) { - if (arrays.has(key)) { - let newValue = arrays.get(key).concat(value); - res.setHeader(key, newValue); - arrays.set(key, newValue); - } else { - res.setHeader(key, value); - arrays.set(key, [value]); - } - } - if (Buffer.isBuffer(response.body)) { - res.end(response.body); + res.writeHead(response.status, response.headers.raw()).end(response.body); } else { - response.body.pipe(res); + res + .writeHead(response.status, response.headers.raw()) + .end(response.body.pipe(res)); } } From 6356ca3cfb469c24b61ec1035a90518589722adb Mon Sep 17 00:00:00 2001 From: Logan McAnsh Date: Wed, 11 Aug 2021 14:57:25 -0400 Subject: [PATCH 15/33] test(architect): add tests for createRemixRequest Signed-off-by: Logan McAnsh --- .../remix-architect/__tests__/server-test.ts | 169 +++++++++++++----- packages/remix-architect/package.json | 4 + yarn.lock | 74 +++++++- 3 files changed, 202 insertions(+), 45 deletions(-) diff --git a/packages/remix-architect/__tests__/server-test.ts b/packages/remix-architect/__tests__/server-test.ts index ec1571b2521..d277b9fa390 100644 --- a/packages/remix-architect/__tests__/server-test.ts +++ b/packages/remix-architect/__tests__/server-test.ts @@ -1,3 +1,4 @@ +import lambdaTester from "lambda-tester"; import { Response, Headers } from "@remix-run/node"; import { createRequestHandler as createRemixRequestHandler } from "@remix-run/node/server"; @@ -6,6 +7,7 @@ import { createRemixRequest, createRequestHandler } from "../server"; +import { APIGatewayProxyEventV2 } from "aws-lambda"; // We don't want to test that the remix server works here (that's what the // puppetteer tests do), we just want to test the architect adapter @@ -14,7 +16,121 @@ let mockedCreateRequestHandler = createRemixRequestHandler as jest.MockedFunctio typeof createRemixRequestHandler >; -describe.skip("architect createRequestHandler", () => {}); +function createMockEvent(event: Partial = {}) { + let now = new Date(); + return { + headers: { + host: "localhost:3333", + accept: "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8", + "upgrade-insecure-requests": "1", + "user-agent": + "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/15.0 Safari/605.1.15", + "accept-language": "en-US,en;q=0.9", + "accept-encoding": "gzip, deflate", + ...event.headers + }, + isBase64Encoded: false, + rawPath: "/", + rawQueryString: "", + requestContext: { + http: { + method: "GET", + path: "/", + protocol: "http", + userAgent: + "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/15.0 Safari/605.1.15", + sourceIp: "127.0.0.1", + ...event.requestContext?.http + }, + routeKey: "ANY /{proxy+}", + accountId: "accountId", + requestId: "requestId", + apiId: "apiId", + domainName: "id.execute-api.us-east-1.amazonaws.com", + domainPrefix: "id", + stage: "test", + time: now.toISOString(), + timeEpoch: now.getTime(), + ...event.requestContext + }, + routeKey: "foo", + version: "2.0", + ...event + }; +} + +describe("architect createRequestHandler", () => { + describe("basic requests", () => { + afterEach(() => { + mockedCreateRequestHandler.mockReset(); + }); + + afterAll(() => { + jest.restoreAllMocks(); + }); + + it("handles requests", async () => { + mockedCreateRequestHandler.mockImplementation(() => async req => { + return new Response(`URL: ${new URL(req.url).pathname}`); + }); + + // @ts-expect-error not sure how Ryan got this not to complain in the express tests.. + await lambdaTester(createRequestHandler({ build: undefined })) + .event(createMockEvent({ rawPath: "/foo/bar" })) + .expectResolve(res => { + expect(res.statusCode).toBe(200); + expect(res.body).toBe("URL: /foo/bar"); + }); + }); + + it("handles status codes", async () => { + mockedCreateRequestHandler.mockImplementation(() => async () => { + return new Response("", { status: 204 }); + }); + + // @ts-expect-error not sure how Ryan got this not to complain in the express tests.. + await lambdaTester(createRequestHandler({ build: undefined })) + .event(createMockEvent({ rawPath: "/foo/bar" })) + .expectResolve(res => { + expect(res.statusCode).toBe(204); + }); + }); + + it("sets headers", async () => { + mockedCreateRequestHandler.mockImplementation(() => async () => { + let headers = new Headers(); + headers.append("X-Time-Of-Year", "most wonderful"); + headers.append( + "Set-Cookie", + "first=one; Expires=0; Path=/; HttpOnly; Secure; SameSite=Lax" + ); + headers.append( + "Set-Cookie", + "second=two; MaxAge=1209600; Path=/; HttpOnly; Secure; SameSite=Lax" + ); + headers.append( + "Set-Cookie", + "third=three; Expires=Wed, 21 Oct 2015 07:28:00 GMT; Path=/; HttpOnly; Secure; SameSite=Lax" + ); + + return new Response("", { headers }); + }); + + // @ts-expect-error not sure how Ryan got this not to complain in the express tests.. + await lambdaTester(createRequestHandler({ build: undefined })) + .event(createMockEvent({ rawPath: "/" })) + .expectResolve(res => { + expect(res.statusCode).toBe(200); + expect(res.headers["x-time-of-year"]).toBe("most wonderful"); + expect(res.cookies).toEqual([ + "first=one; Expires=0; Path=/; HttpOnly; Secure; SameSite=Lax", + "second=two; MaxAge=1209600; Path=/; HttpOnly; Secure; SameSite=Lax", + "third=three; Expires=Wed, 21 Oct 2015 07:28:00 GMT; Path=/; HttpOnly; Secure; SameSite=Lax" + ]); + }); + }); + }); +}); describe("architect createRemixHeaders", () => { describe("creates fetch headers from architect headers", () => { @@ -87,16 +203,19 @@ describe("architect createRemixHeaders", () => { it("handles cookies", () => { expect( - createRemixHeaders({}, [ + createRemixHeaders({ "x-something-else": "true" }, [ "__session=some_value; Path=/; Secure; HttpOnly; MaxAge=7200; SameSite=Lax", - "__other=some_other_value; Path=/; Secure; HttpOnly; MaxAge=3600; SameSite=Lax" + "__other=some_other_value; Path=/; Secure; HttpOnly; Expires=Wed, 21 Oct 2015 07:28:00 GMT; SameSite=Lax" ]) ).toMatchInlineSnapshot(` Headers { Symbol(map): Object { "Cookie": Array [ "__session=some_value; Path=/; Secure; HttpOnly; MaxAge=7200; SameSite=Lax", - "__other=some_other_value; Path=/; Secure; HttpOnly; MaxAge=3600; SameSite=Lax", + "__other=some_other_value; Path=/; Secure; HttpOnly; Expires=Wed, 21 Oct 2015 07:28:00 GMT; SameSite=Lax", + ], + "x-something-else": Array [ + "true", ], }, } @@ -108,43 +227,11 @@ describe("architect createRemixHeaders", () => { describe("architect createRemixRequest", () => { it("creates a request with the correct headers", () => { expect( - createRemixRequest({ - headers: { - host: "localhost:3333", - accept: - "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8", - "upgrade-insecure-requests": "1", - "user-agent": - "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/15.0 Safari/605.1.15", - "accept-language": "en-US,en;q=0.9", - "accept-encoding": "gzip, deflate" - }, - isBase64Encoded: false, - rawPath: "/", - rawQueryString: "", - requestContext: { - http: { - method: "GET", - path: "/", - protocol: "http", - userAgent: - "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/15.0 Safari/605.1.15", - sourceIp: "127.0.0.1" - }, - routeKey: "ANY /{proxy+}", - accountId: "1234567890", - requestId: "abcdefghijklmnopqrstuvwxyz", - apiId: "1234567890abcdef", - domainName: "localhost:3333", - domainPrefix: "localhost:3333", - stage: "prod", - time: "2021-08-10T19:48:50.969Z", - timeEpoch: 1628624930969 - }, - routeKey: "foo", - version: "2.0", - cookies: ["__session=value"] - }) + createRemixRequest( + createMockEvent({ + cookies: ["__session=value"] + }) + ) ).toMatchInlineSnapshot(` Request { "agent": undefined, diff --git a/packages/remix-architect/package.json b/packages/remix-architect/package.json index d139d9aedd1..f37f765b534 100644 --- a/packages/remix-architect/package.json +++ b/packages/remix-architect/package.json @@ -10,5 +10,9 @@ "peerDependencies": { "@architect/architect": "^8.3.7", "@architect/functions": "^3.13.9" + }, + "devDependencies": { + "@types/lambda-tester": "^3.6.1", + "lambda-tester": "^4.0.1" } } diff --git a/yarn.lock b/yarn.lock index 8f2802352d9..9cea664a9e8 100644 --- a/yarn.lock +++ b/yarn.lock @@ -938,6 +938,11 @@ minimatch "^3.0.4" strip-json-comments "^3.1.1" +"@extra-number/significant-digits@^1.1.1": + version "1.3.9" + resolved "https://registry.yarnpkg.com/@extra-number/significant-digits/-/significant-digits-1.3.9.tgz#06f3acc4aa688af3ed76bf5f30bca6de9d60883f" + integrity sha512-E5PY/bCwrNqEHh4QS6AQBinLZ+sxM1lT8tsSVYk8VwhWIPp6fCU/BMRVq0V8iJ8LwS3FHmaA4vUzb78s4BIIyA== + "@istanbuljs/load-nyc-config@^1.0.0": version "1.1.0" resolved "https://registry.yarnpkg.com/@istanbuljs/load-nyc-config/-/load-nyc-config-1.1.0.tgz#fd3db1d59ecf7cf121e80650bb86712f9b55eced" @@ -1212,7 +1217,7 @@ dependencies: "@sinonjs/commons" "^1.7.0" -"@types/aws-lambda@^8.10.82": +"@types/aws-lambda@*", "@types/aws-lambda@^8.10.82": version "8.10.82" resolved "https://registry.yarnpkg.com/@types/aws-lambda/-/aws-lambda-8.10.82.tgz#336062d3270f52d2156eeb7d9e497fd63684572a" integrity sha512-sJo8pz8hu+OzLRAj7Do2g66zYLizWtB3kGK6K45RWmGW+S54XXMoK3sNbvzKXfndBxYiSVExHoCNiSlt2gPmxw== @@ -1410,6 +1415,13 @@ resolved "https://registry.yarnpkg.com/@types/json5/-/json5-0.0.29.tgz#ee28707ae94e11d2b827bcbe5270bcea7f3e71ee" integrity sha1-7ihweulOEdK4J7y+UnC86n8+ce4= +"@types/lambda-tester@^3.6.1": + version "3.6.1" + resolved "https://registry.yarnpkg.com/@types/lambda-tester/-/lambda-tester-3.6.1.tgz#e09d79a191a4cef6ffa4fc4805eb614c8b15edd6" + integrity sha512-cKg0SdVrtjkiAJcVTGabD+u+eAIdjBJ+qgXiolw0hffW6B/1o+h63EptSUo/BTYpkoRxnHFkHS55y4cWja4xIw== + dependencies: + "@types/aws-lambda" "*" + "@types/lodash.debounce@^4.0.6": version "4.0.6" resolved "https://registry.yarnpkg.com/@types/lodash.debounce/-/lodash.debounce-4.0.6.tgz#c5a2326cd3efc46566c47e4c0aa248dc0ee57d60" @@ -2058,6 +2070,11 @@ anymatch@^3.0.3, anymatch@~3.1.1: normalize-path "^3.0.0" picomatch "^2.0.4" +app-root-path@^3.0.0: + version "3.0.0" + resolved "https://registry.yarnpkg.com/app-root-path/-/app-root-path-3.0.0.tgz#210b6f43873227e18a4b810a032283311555d5ad" + integrity sha512-qMcx+Gy2UZynHjOHOIXPNvpf+9cjvk3cWrBBK7zg4gH9+clobJRb9NGzcT7mQTcV/6Gm/1WelUtqxVXnNlrwcw== + arg@^4.1.0: version "4.1.3" resolved "https://registry.yarnpkg.com/arg/-/arg-4.1.3.tgz#269fc7ad5b8e42cb63c896d5666017261c144089" @@ -2720,7 +2737,7 @@ clone-deep@^1.0.0: kind-of "^5.0.0" shallow-clone "^1.0.0" -clone-deep@^4.0.0: +clone-deep@^4.0.0, clone-deep@^4.0.1: version "4.0.1" resolved "https://registry.yarnpkg.com/clone-deep/-/clone-deep-4.0.1.tgz#c19fd9bdbbf85942b4fd979c84dcf7d5f07c2387" integrity sha512-neHB9xuzh/wk0dIHweyAXv2aPGZIVk3pLMe+/RNzINf17fe0OG96QroktYAUm7SM1PBnzTabaLboqqxDyMU+SQ== @@ -3170,6 +3187,16 @@ domutils@^2.4.3, domutils@^2.4.4: domelementtype "^2.0.1" domhandler "^4.0.0" +dotenv-json@^1.0.0: + version "1.0.0" + resolved "https://registry.yarnpkg.com/dotenv-json/-/dotenv-json-1.0.0.tgz#fc7f672aafea04bed33818733b9f94662332815c" + integrity sha512-jAssr+6r4nKhKRudQ0HOzMskOFFi9+ubXWwmrSGJFgTvpjyPXCXsCsYbjif6mXp7uxA7xY3/LGaiTQukZzSbOQ== + +dotenv@^8.0.0: + version "8.6.0" + resolved "https://registry.yarnpkg.com/dotenv/-/dotenv-8.6.0.tgz#061af664d19f7f4d8fc6e4ff9b584ce237adcb8b" + integrity sha512-IrPdXQsk2BbzvCBGBOTmmSH5SodmqZNt4ERAZDmW4CT+tL8VtvinqywuANaFu4bOMWki16nqf0e4oC0QIaDr/g== + ecc-jsbn@~0.1.1: version "0.1.2" resolved "https://registry.yarnpkg.com/ecc-jsbn/-/ecc-jsbn-0.1.2.tgz#3a83a904e54353287874c564b7549386849a98c9" @@ -5203,6 +5230,35 @@ koalas@^1.0.2: resolved "https://registry.yarnpkg.com/koalas/-/koalas-1.0.2.tgz#318433f074235db78fae5661a02a8ca53ee295cd" integrity sha1-MYQz8HQjXbePrlZhoCqMpT7ilc0= +lambda-event-mock@^1.5.0: + version "1.5.0" + resolved "https://registry.yarnpkg.com/lambda-event-mock/-/lambda-event-mock-1.5.0.tgz#9cb1ce2bec4271f918d485fef0a327d194dd120f" + integrity sha512-vx1d+vZqi7FF6B3+mAfHnY/6Tlp6BheL2ta0MJS0cIRB3Rc4I5cviHTkiJxHdE156gXx3ZjlQRJrS4puXvtrhA== + dependencies: + "@extra-number/significant-digits" "^1.1.1" + clone-deep "^4.0.1" + uuid "^3.3.3" + vandium-utils "^1.2.0" + +lambda-leak@^2.0.0: + version "2.0.0" + resolved "https://registry.yarnpkg.com/lambda-leak/-/lambda-leak-2.0.0.tgz#771985d3628487f6e885afae2b54510dcfb2cd7e" + integrity sha1-dxmF02KEh/boha+uK1RRDc+yzX4= + +lambda-tester@^4.0.1: + version "4.0.1" + resolved "https://registry.yarnpkg.com/lambda-tester/-/lambda-tester-4.0.1.tgz#91f0fc1266cdceae09a5ddbbdbc209c214beb98c" + integrity sha512-ft6XHk84B6/dYEzyI3anKoGWz08xQ5allEHiFYDUzaYTymgVK7tiBkCEbuWx+MFvH7OpFNsJXVtjXm0X8iH3Iw== + dependencies: + app-root-path "^3.0.0" + dotenv "^8.0.0" + dotenv-json "^1.0.0" + lambda-event-mock "^1.5.0" + lambda-leak "^2.0.0" + semver "^6.1.1" + uuid "^3.3.3" + vandium-utils "^2.0.0" + language-subtag-registry@~0.3.2: version "0.3.21" resolved "https://registry.yarnpkg.com/language-subtag-registry/-/language-subtag-registry-0.3.21.tgz#04ac218bea46f04cb039084602c6da9e788dd45a" @@ -6826,7 +6882,7 @@ semver@7.0.0: resolved "https://registry.yarnpkg.com/semver/-/semver-7.0.0.tgz#5f3ca35761e47e05b206c6daff2cf814f0316b8e" integrity sha512-+GB6zVA9LWh6zovYQLALHwv5rb2PHGlJi3lfiqIHxR0uuwCgefcOJc59v9fv1w8GbStwxuuqqAjI9NMAOOgq1A== -semver@^6.0.0, semver@^6.3.0: +semver@^6.0.0, semver@^6.1.1, semver@^6.3.0: version "6.3.0" resolved "https://registry.yarnpkg.com/semver/-/semver-6.3.0.tgz#ee0a64c8af5e8ceea67687b133761e1becbd1d3d" integrity sha512-b39TBaTSfV6yBrapU89p5fKekE2m/NwnDocOVruQFS1/veMgdzuPcnOM34M6CwxW8jH/lxEa5rBoDeUwu5HHTw== @@ -7759,7 +7815,7 @@ utils-merge@1.0.1: resolved "https://registry.yarnpkg.com/utils-merge/-/utils-merge-1.0.1.tgz#9f95710f50a267947b2ccc124741c1028427e713" integrity sha1-n5VxD1CiZ5R7LMwSR0HBAoQn5xM= -uuid@^3.3.2: +uuid@^3.3.2, uuid@^3.3.3: version "3.4.0" resolved "https://registry.yarnpkg.com/uuid/-/uuid-3.4.0.tgz#b23e4358afa8a202fe7a100af1f5f883f02007ee" integrity sha512-HjSDRw6gZE5JMggctHBcjVak08+KEVhSIiDzFnT9S9aegmp85S/bReBVTb4QTFaRNptJ9kuYaNhnbNEOkbKb/A== @@ -7791,6 +7847,16 @@ validate-npm-package-license@^3.0.1: spdx-correct "^3.0.0" spdx-expression-parse "^3.0.0" +vandium-utils@^1.2.0: + version "1.2.0" + resolved "https://registry.yarnpkg.com/vandium-utils/-/vandium-utils-1.2.0.tgz#44735de4b7641a05de59ebe945f174e582db4f59" + integrity sha1-RHNd5LdkGgXeWevpRfF05YLbT1k= + +vandium-utils@^2.0.0: + version "2.0.0" + resolved "https://registry.yarnpkg.com/vandium-utils/-/vandium-utils-2.0.0.tgz#87389bdcb85551aaaba1cc95937ba756589214fa" + integrity sha512-XWbQ/0H03TpYDXk8sLScBEZpE7TbA0CHDL6/Xjt37IBYKLsHUQuBlL44ttAUs9zoBOLFxsW7HehXcuWCNyqOxQ== + vary@~1.1.2: version "1.1.2" resolved "https://registry.yarnpkg.com/vary/-/vary-1.1.2.tgz#2299f02c6ded30d4a5961b0b9f74524a18f634fc" From 739084c6f6ac68c947170453276e084379addaba Mon Sep 17 00:00:00 2001 From: Logan McAnsh Date: Wed, 11 Aug 2021 16:10:29 -0400 Subject: [PATCH 16/33] test(vercel): add test for createRequestHandler vercel requests are regular http IncomingMessages but with various methods on it to make it more like express, so we'll just treat it like express.. Signed-off-by: Logan McAnsh --- .../remix-vercel/__tests__/server-test.ts | 71 +++++++++++++++++-- packages/remix-vercel/package.json | 1 + yarn.lock | 31 +++++--- 3 files changed, 91 insertions(+), 12 deletions(-) diff --git a/packages/remix-vercel/__tests__/server-test.ts b/packages/remix-vercel/__tests__/server-test.ts index 3e3d75ae509..1251940ff2b 100644 --- a/packages/remix-vercel/__tests__/server-test.ts +++ b/packages/remix-vercel/__tests__/server-test.ts @@ -1,11 +1,17 @@ import { Headers, Response, RequestInfo, RequestInit } from "@remix-run/node"; import { createRequestHandler as createRemixRequestHandler } from "@remix-run/node/server"; -// @ts-expect-error TODO: add type definition for this +import { createRequest } from "node-mocks-http"; + import { createServerWithHelpers } from "@vercel/node/dist/helpers"; import listen from "test-listen"; import fetch from "node-fetch"; -import { createRemixHeaders, createRequestHandler } from "../server"; +import { + createRemixHeaders, + createRemixRequest, + createRequestHandler +} from "../server"; +import { VercelRequest } from "@vercel/node"; // We don't want to test that the remix server works here (that's what the // puppetteer tests do), we just want to test the vercel adapter @@ -42,7 +48,6 @@ describe("vercel createRequestHandler", () => { server = createServerWithHelpers((req: any, res: any) => { // We don't have a real app to test, but it doesn't matter. We // won't ever call through to the real createRequestHandler - // @ts-expect-error return createRequestHandler({ build: undefined })(req, res); }, mockBridge); @@ -198,5 +203,63 @@ describe("vercel createRemixHeaders", () => { }); describe("vercel createRemixRequest", () => { - it.todo("creates a request with the correct headers"); + it("creates a request with the correct headers", async () => { + var request = createRequest({ + method: "GET", + url: "/foo/bar", + headers: { + "x-forwarded-host": "localhost:3000", + "x-forwarded-proto": "http", + "Cache-Control": "max-age=300, s-maxage=3600" + } + }) as VercelRequest; + + expect(createRemixRequest(request)).toMatchInlineSnapshot(` + Request { + "agent": undefined, + "compress": true, + "counter": 0, + "follow": 20, + "size": 0, + "timeout": 0, + Symbol(Body internals): Object { + "body": null, + "disturbed": false, + "error": null, + }, + Symbol(Request internals): Object { + "headers": Headers { + Symbol(map): Object { + "cache-control": Array [ + "max-age=300, s-maxage=3600", + ], + "x-forwarded-host": Array [ + "localhost:3000", + ], + "x-forwarded-proto": Array [ + "http", + ], + }, + }, + "method": "GET", + "parsedURL": Url { + "auth": null, + "hash": null, + "host": "localhost:3000", + "hostname": "localhost", + "href": "http://localhost:3000/foo/bar", + "path": "/foo/bar", + "pathname": "/foo/bar", + "port": "3000", + "protocol": "http:", + "query": null, + "search": null, + "slashes": true, + }, + "redirect": "follow", + "signal": null, + }, + } + `); + }); }); diff --git a/packages/remix-vercel/package.json b/packages/remix-vercel/package.json index 648afb149eb..17f92b9d8c8 100644 --- a/packages/remix-vercel/package.json +++ b/packages/remix-vercel/package.json @@ -12,6 +12,7 @@ "devDependencies": { "@types/test-listen": "^1.1.0", "@vercel/node": "^1.8.3", + "node-mocks-http": "^1.10.1", "test-listen": "^1.1.0" } } diff --git a/yarn.lock b/yarn.lock index 9cea664a9e8..d0123f6010e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1732,7 +1732,7 @@ abortcontroller-polyfill@^1.7.3: resolved "https://registry.yarnpkg.com/abortcontroller-polyfill/-/abortcontroller-polyfill-1.7.3.tgz#1b5b487bd6436b5b764fd52a612509702c3144b5" integrity sha512-zetDJxd89y3X99Kvo4qFx8GKlt6GsvN3UcRZHwU6iFA/0KiOmhkTVhe8oRoTBiTVPZu09x3vCra47+w8Yz1+2Q== -accepts@~1.3.5, accepts@~1.3.7: +accepts@^1.3.7, accepts@~1.3.5, accepts@~1.3.7: version "1.3.7" resolved "https://registry.yarnpkg.com/accepts/-/accepts-1.3.7.tgz#531bc726517a3b2b41f850021c6cc15eaab507cd" integrity sha512-Il80Qs2WjYlJIBNzNkK6KYqlVMTbZLXgHx2oT0pU/fjRHyEp+PEfEPY0R3WCwAGVOtauxh1hOxNgIf5bv7dQpA== @@ -3081,7 +3081,7 @@ delayed-stream@~1.0.0: resolved "https://registry.yarnpkg.com/delayed-stream/-/delayed-stream-1.0.0.tgz#df3ae199acadfb7d440aaae0b29e2272b24ec619" integrity sha1-3zrhmayt+31ECqrgsp4icrJOxhk= -depd@~1.1.2: +depd@^1.1.0, depd@~1.1.2: version "1.1.2" resolved "https://registry.yarnpkg.com/depd/-/depd-1.1.2.tgz#9bcd52e14c097763e749b274c4346ed2e560b5a9" integrity sha1-m81S4UwJd2PnSbJ0xDRu0uVgtak= @@ -3897,7 +3897,7 @@ fragment-cache@^0.2.1: dependencies: map-cache "^0.2.2" -fresh@0.5.2: +fresh@0.5.2, fresh@^0.5.2: version "0.5.2" resolved "https://registry.yarnpkg.com/fresh/-/fresh-0.5.2.tgz#3d8cadd90d976569fa835ab1f8e4b23a105605a7" integrity sha1-PYyt2Q2XZWn6g1qx+OSyOhBWBac= @@ -5485,7 +5485,7 @@ meow@^7.1.1: type-fest "^0.13.1" yargs-parser "^18.1.3" -merge-descriptors@1.0.1: +merge-descriptors@1.0.1, merge-descriptors@^1.0.1: version "1.0.1" resolved "https://registry.yarnpkg.com/merge-descriptors/-/merge-descriptors-1.0.1.tgz#b00aaa556dd8b44568150ec9d1b953f3f90cbb61" integrity sha1-sAqqVW3YtEVoFQ7J0blT8/kMu2E= @@ -5549,7 +5549,7 @@ mime-types@^2.1.12, mime-types@~2.1.19, mime-types@~2.1.24: dependencies: mime-db "1.46.0" -mime@1.6.0: +mime@1.6.0, mime@^1.3.4: version "1.6.0" resolved "https://registry.yarnpkg.com/mime/-/mime-1.6.0.tgz#32cd9e5c64553bd58d19a568af452acff04981b1" integrity sha512-x0Vn8spI+wuJ1O6S7gnbaQg8Pxh4NNHb7KSINmEWKiPE4RKOplvijn+NkmYmmRgP68mc70j2EbeTFRsrswaQeg== @@ -5743,6 +5743,21 @@ node-int64@^0.4.0: resolved "https://registry.yarnpkg.com/node-int64/-/node-int64-0.4.0.tgz#87a9065cdb355d3182d8f94ce11188b825c68a3b" integrity sha1-h6kGXNs1XTGC2PlM4RGIuCXGijs= +node-mocks-http@^1.10.1: + version "1.10.1" + resolved "https://registry.yarnpkg.com/node-mocks-http/-/node-mocks-http-1.10.1.tgz#0232e99a5f66f5d2a0216a47251346bf5606d2d0" + integrity sha512-/Nz83kiJ3z+vGqxmlDyv8+L1CJno+gH23DzG3oPH9dBSfMYa5IFVwPgZpXCB2kdiiIu/HoDpZ2BuLqQs7qjFLQ== + dependencies: + accepts "^1.3.7" + depd "^1.1.0" + fresh "^0.5.2" + merge-descriptors "^1.0.1" + methods "^1.1.2" + mime "^1.3.4" + parseurl "^1.3.3" + range-parser "^1.2.0" + type-is "^1.6.18" + node-modules-regexp@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/node-modules-regexp/-/node-modules-regexp-1.0.0.tgz#8d9dbe28964a4ac5712e9131642107c71e90ec40" @@ -6065,7 +6080,7 @@ parse5@^6.0.0, parse5@^6.0.1: resolved "https://registry.yarnpkg.com/parse5/-/parse5-6.0.1.tgz#e1a1c085c569b3dc08321184f19a39cc27f7c30b" integrity sha512-Ofn/CTFzRGTTxwpNEs9PP93gXShHcTq255nzRYSKe8AkVpZY7e1fpmTfOyoIvjP5HG7Z2ZM7VS9PPhQGW2pOpw== -parseurl@~1.3.3: +parseurl@^1.3.3, parseurl@~1.3.3: version "1.3.3" resolved "https://registry.yarnpkg.com/parseurl/-/parseurl-1.3.3.tgz#9da19e7bee8d12dff0513ed5b76957793bc2e8d4" integrity sha512-CiyeOxFT/JZyN5m0z9PfXw4SCBJ6Sygz1Dpl0wqjlhDEGGBP1GnsUVEL0p63hoG1fcj3fHynXi9NYO4nWOL+qQ== @@ -6407,7 +6422,7 @@ radio-symbol@^2.0.0: ansi-green "^0.1.1" is-windows "^1.0.1" -range-parser@~1.2.1: +range-parser@^1.2.0, range-parser@~1.2.1: version "1.2.1" resolved "https://registry.yarnpkg.com/range-parser/-/range-parser-1.2.1.tgz#3cf37023d199e1c24d1a55b84800c2f3e6468031" integrity sha512-Hrgsx+orqoygnmhFbKaHE6c296J+HTAQXoxEF6gNupROmmGJRoyzfG3ccAveqCBrwr/2yxQ5BVd/GTl5agOwSg== @@ -7666,7 +7681,7 @@ type-fest@^0.8.1: resolved "https://registry.yarnpkg.com/type-fest/-/type-fest-0.8.1.tgz#09e249ebde851d3b1e48d27c105444667f17b83d" integrity sha512-4dbzIzqvjtgiM5rw1k5rEHtBANKmdudhGyBEajN01fEyhaAIhsoKNy6y7+IN93IfpFtwY9iqi7kD+xwKhQsNJA== -type-is@~1.6.17, type-is@~1.6.18: +type-is@^1.6.18, type-is@~1.6.17, type-is@~1.6.18: version "1.6.18" resolved "https://registry.yarnpkg.com/type-is/-/type-is-1.6.18.tgz#4e552cd05df09467dcbc4ef739de89f2cf37c131" integrity sha512-TkRKr9sUTxEH8MdfuCSP7VizJyzRNMjj2J2do2Jr3Kym598JVdEksuzPQCnlFPW4ky9Q+iA+ma9BGm06XQBy8g== From d7cbc230f6adbe8d21f365deff1572d5e01a6b95 Mon Sep 17 00:00:00 2001 From: Logan McAnsh Date: Wed, 11 Aug 2021 21:41:10 -0400 Subject: [PATCH 17/33] test(vercel): use supertest for better header testing Signed-off-by: Logan McAnsh --- .../remix-vercel/__tests__/server-test.ts | 78 +++++++++---------- packages/remix-vercel/package.json | 5 +- packages/remix-vercel/server.ts | 6 +- yarn.lock | 20 ++--- 4 files changed, 52 insertions(+), 57 deletions(-) diff --git a/packages/remix-vercel/__tests__/server-test.ts b/packages/remix-vercel/__tests__/server-test.ts index 1251940ff2b..64a9b568a4c 100644 --- a/packages/remix-vercel/__tests__/server-test.ts +++ b/packages/remix-vercel/__tests__/server-test.ts @@ -1,10 +1,9 @@ -import { Headers, Response, RequestInfo, RequestInit } from "@remix-run/node"; +import { Headers, Response } from "@remix-run/node"; import { createRequestHandler as createRemixRequestHandler } from "@remix-run/node/server"; import { createRequest } from "node-mocks-http"; +import supertest from "supertest"; import { createServerWithHelpers } from "@vercel/node/dist/helpers"; -import listen from "test-listen"; -import fetch from "node-fetch"; import { createRemixHeaders, @@ -20,43 +19,23 @@ let mockedCreateRequestHandler = createRemixRequestHandler as jest.MockedFunctio typeof createRemixRequestHandler >; -// TODO: add real type definition -let server: any; -let url: string; let consumeEventMock = jest.fn(); let mockBridge = { consumeEvent: consumeEventMock }; -async function fetchWithProxyReq(_url: RequestInfo, opts: RequestInit = {}) { - if (opts.body) { - // eslint-disable-next-line - // @ts-ignore look into - opts = { ...opts, body: Buffer.from(opts.body) }; - } - - consumeEventMock.mockImplementationOnce(() => opts); - - return fetch(_url, { - ...opts, - headers: { ...opts.headers, "x-now-bridge-request-id": "2" } - }); +function createApp() { + // TODO: get supertest args into the event + consumeEventMock.mockImplementationOnce(() => ({ body: "" })); + let server = createServerWithHelpers( + createRequestHandler({ build: undefined }), + mockBridge + ); + return server; } describe("vercel createRequestHandler", () => { - beforeEach(async () => { - consumeEventMock.mockClear(); - - server = createServerWithHelpers((req: any, res: any) => { - // We don't have a real app to test, but it doesn't matter. We - // won't ever call through to the real createRequestHandler - return createRequestHandler({ build: undefined })(req, res); - }, mockBridge); - - url = await listen(server); - }); - afterEach(async () => { mockedCreateRequestHandler.mockReset(); - await server.close(); + consumeEventMock.mockClear(); }); afterAll(() => { @@ -64,15 +43,27 @@ describe("vercel createRequestHandler", () => { }); describe("basic requests", () => { + afterEach(() => { + mockedCreateRequestHandler.mockReset(); + }); + + afterAll(() => { + jest.restoreAllMocks(); + }); + it("handles requests", async () => { mockedCreateRequestHandler.mockImplementation(() => async req => { return new Response(`URL: ${new URL(req.url).pathname}`); }); - const res = await fetchWithProxyReq(url + "/foo/bar"); + let request = supertest(createApp()); + + let res = await request + .get("/foo/bar") + .set({ "x-now-bridge-request-id": "2" }); expect(res.status).toBe(200); - expect(await res.text()).toBe("URL: /foo/bar"); + expect(res.text).toBe("URL: /foo/bar"); }); it("handles status codes", async () => { @@ -80,7 +71,10 @@ describe("vercel createRequestHandler", () => { return new Response("", { status: 204 }); }); - let res = await fetchWithProxyReq(url); + const request = supertest(createApp()); + const res = await request + .get("/") + .set({ "x-now-bridge-request-id": "2" }); expect(res.status).toBe(204); }); @@ -103,12 +97,16 @@ describe("vercel createRequestHandler", () => { return new Response("", { headers }); }); - let res = await fetchWithProxyReq(url); + let request = supertest(createApp()); + + let res = await request.get("/").set({ "x-now-bridge-request-id": "2" }); - expect(res.headers.get("x-time-of-year")).toBe("most wonderful"); - expect(res.headers.get("set-cookie")).toEqual( - "first=one; Expires=0; Path=/; HttpOnly; Secure; SameSite=Lax, second=two; MaxAge=1209600; Path=/; HttpOnly; Secure; SameSite=Lax, third=three; Expires=Wed, 21 Oct 2015 07:28:00 GMT; Path=/; HttpOnly; Secure; SameSite=Lax" - ); + expect(res.headers["x-time-of-year"]).toBe("most wonderful"); + expect(res.headers["set-cookie"]).toEqual([ + "first=one; Expires=0; Path=/; HttpOnly; Secure; SameSite=Lax", + "second=two; MaxAge=1209600; Path=/; HttpOnly; Secure; SameSite=Lax", + "third=three; Expires=Wed, 21 Oct 2015 07:28:00 GMT; Path=/; HttpOnly; Secure; SameSite=Lax" + ]); }); }); }); diff --git a/packages/remix-vercel/package.json b/packages/remix-vercel/package.json index 17f92b9d8c8..47a77283977 100644 --- a/packages/remix-vercel/package.json +++ b/packages/remix-vercel/package.json @@ -10,9 +10,8 @@ "@vercel/node": "^1.8.3" }, "devDependencies": { - "@types/test-listen": "^1.1.0", "@vercel/node": "^1.8.3", - "node-mocks-http": "^1.10.1", - "test-listen": "^1.1.0" + "supertest": "^6.1.5", + "node-mocks-http": "^1.10.1" } } diff --git a/packages/remix-vercel/server.ts b/packages/remix-vercel/server.ts index 2674449b3b4..dd9dff76d49 100644 --- a/packages/remix-vercel/server.ts +++ b/packages/remix-vercel/server.ts @@ -94,9 +94,11 @@ function sendRemixResponse(res: NowResponse, response: Response): void { res.status(response.status); if (Buffer.isBuffer(response.body)) { - res.writeHead(response.status, response.headers.raw()).end(response.body); + return res + .writeHead(response.status, response.headers.raw()) + .end(response.body); } else { - res + return res .writeHead(response.status, response.headers.raw()) .end(response.body.pipe(res)); } diff --git a/yarn.lock b/yarn.lock index d0123f6010e..34442e1c1a9 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1586,13 +1586,6 @@ dependencies: "@types/superagent" "*" -"@types/test-listen@^1.1.0": - version "1.1.0" - resolved "https://registry.yarnpkg.com/@types/test-listen/-/test-listen-1.1.0.tgz#db7e5b0e277b4a3ee7b90770dae1a41b186458af" - integrity sha512-y6ZfbSzYHniCeY6ZAzsQjSAdJInNVoEz4Uhsb81W+RCoNYA59yoG/+XbqPqCPj2KCU3Wa6RFWSozutkGIHIsNQ== - dependencies: - "@types/node" "*" - "@types/through@*": version "0.0.30" resolved "https://registry.yarnpkg.com/@types/through/-/through-0.0.30.tgz#e0e42ce77e897bd6aead6f6ea62aeb135b8a3895" @@ -7363,6 +7356,14 @@ supertest@^6.0.1: methods "^1.1.2" superagent "^6.1.0" +supertest@^6.1.5: + version "6.1.5" + resolved "https://registry.yarnpkg.com/supertest/-/supertest-6.1.5.tgz#b011c8465281b30c64e9d4be4cb3808b91cd1ec0" + integrity sha512-Is3pFB2TxSFPohDS2tIM8h2JOMvUQwbJ9TvTfsWAm89ZZv1CF4VTLAsQz+5+5S1wOgaMqFqSpFriU15L3e2AXQ== + dependencies: + methods "^1.1.2" + superagent "^6.1.0" + supports-color@^5.3.0: version "5.5.0" resolved "https://registry.yarnpkg.com/supports-color/-/supports-color-5.5.0.tgz#e2e69a44ac8772f78a1ec0b35b689df6530efc8f" @@ -7459,11 +7460,6 @@ test-exclude@^6.0.0: glob "^7.1.4" minimatch "^3.0.4" -test-listen@^1.1.0: - version "1.1.0" - resolved "https://registry.yarnpkg.com/test-listen/-/test-listen-1.1.0.tgz#2ba614d96c3bc9157469003027b42a495dd83b6a" - integrity sha512-OyEVi981C1sb9NX1xayfgZls3p8QTDRwp06EcgxSgd1kktaENBW8dO15i8v/7Fi15j0IYQctJzk5J+hyEBId2w== - text-table@^0.2.0: version "0.2.0" resolved "https://registry.yarnpkg.com/text-table/-/text-table-0.2.0.tgz#7f5ee823ae805207c00af2df4a84ec3fcfa570b4" From 63017e0e45cb89cb48198c5b9c760376496452a8 Mon Sep 17 00:00:00 2001 From: Logan McAnsh Date: Wed, 11 Aug 2021 22:16:16 -0400 Subject: [PATCH 18/33] chore(deps/vercel): add @types/supertest Signed-off-by: Logan McAnsh --- packages/remix-vercel/package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/remix-vercel/package.json b/packages/remix-vercel/package.json index 47a77283977..e37604fdcc2 100644 --- a/packages/remix-vercel/package.json +++ b/packages/remix-vercel/package.json @@ -10,6 +10,7 @@ "@vercel/node": "^1.8.3" }, "devDependencies": { + "@types/supertest": "^2.0.10", "@vercel/node": "^1.8.3", "supertest": "^6.1.5", "node-mocks-http": "^1.10.1" From a0b21a4cb2ee3214d54b7ef1625120e9452d4bf6 Mon Sep 17 00:00:00 2001 From: Logan McAnsh Date: Thu, 12 Aug 2021 10:16:08 -0400 Subject: [PATCH 19/33] test(express,vercel): bring tests to parity Signed-off-by: Logan McAnsh --- .../remix-express/__tests__/server-test.ts | 7 ++++- .../remix-vercel/__tests__/server-test.ts | 29 +++++-------------- 2 files changed, 14 insertions(+), 22 deletions(-) diff --git a/packages/remix-express/__tests__/server-test.ts b/packages/remix-express/__tests__/server-test.ts index 08b8fed1dd1..6d8b32d96a5 100644 --- a/packages/remix-express/__tests__/server-test.ts +++ b/packages/remix-express/__tests__/server-test.ts @@ -77,6 +77,10 @@ describe("express createRequestHandler", () => { "Set-Cookie", "second=two; MaxAge=1209600; Path=/; HttpOnly; Secure; SameSite=Lax" ); + headers.append( + "Set-Cookie", + "third=three; Expires=Wed, 21 Oct 2015 07:28:00 GMT; Path=/; HttpOnly; Secure; SameSite=Lax" + ); return new Response("", { headers }); }); @@ -86,7 +90,8 @@ describe("express createRequestHandler", () => { expect(res.headers["x-time-of-year"]).toBe("most wonderful"); expect(res.headers["set-cookie"]).toEqual([ "first=one; Expires=0; Path=/; HttpOnly; Secure; SameSite=Lax", - "second=two; MaxAge=1209600; Path=/; HttpOnly; Secure; SameSite=Lax" + "second=two; MaxAge=1209600; Path=/; HttpOnly; Secure; SameSite=Lax", + "third=three; Expires=Wed, 21 Oct 2015 07:28:00 GMT; Path=/; HttpOnly; Secure; SameSite=Lax" ]); }); }); diff --git a/packages/remix-vercel/__tests__/server-test.ts b/packages/remix-vercel/__tests__/server-test.ts index 64a9b568a4c..2f563e93fcb 100644 --- a/packages/remix-vercel/__tests__/server-test.ts +++ b/packages/remix-vercel/__tests__/server-test.ts @@ -1,16 +1,15 @@ -import { Headers, Response } from "@remix-run/node"; +import supertest from "supertest"; +import { Response, Headers } from "@remix-run/node"; import { createRequestHandler as createRemixRequestHandler } from "@remix-run/node/server"; import { createRequest } from "node-mocks-http"; -import supertest from "supertest"; - import { createServerWithHelpers } from "@vercel/node/dist/helpers"; +import { VercelRequest } from "@vercel/node"; import { createRemixHeaders, createRemixRequest, createRequestHandler } from "../server"; -import { VercelRequest } from "@vercel/node"; // We don't want to test that the remix server works here (that's what the // puppetteer tests do), we just want to test the vercel adapter @@ -33,18 +32,10 @@ function createApp() { } describe("vercel createRequestHandler", () => { - afterEach(async () => { - mockedCreateRequestHandler.mockReset(); - consumeEventMock.mockClear(); - }); - - afterAll(() => { - jest.restoreAllMocks(); - }); - describe("basic requests", () => { - afterEach(() => { + afterEach(async () => { mockedCreateRequestHandler.mockReset(); + consumeEventMock.mockClear(); }); afterAll(() => { @@ -57,7 +48,6 @@ describe("vercel createRequestHandler", () => { }); let request = supertest(createApp()); - let res = await request .get("/foo/bar") .set({ "x-now-bridge-request-id": "2" }); @@ -71,10 +61,8 @@ describe("vercel createRequestHandler", () => { return new Response("", { status: 204 }); }); - const request = supertest(createApp()); - const res = await request - .get("/") - .set({ "x-now-bridge-request-id": "2" }); + let request = supertest(createApp()); + let res = await request.get("/").set({ "x-now-bridge-request-id": "2" }); expect(res.status).toBe(204); }); @@ -98,7 +86,6 @@ describe("vercel createRequestHandler", () => { }); let request = supertest(createApp()); - let res = await request.get("/").set({ "x-now-bridge-request-id": "2" }); expect(res.headers["x-time-of-year"]).toBe("most wonderful"); @@ -202,7 +189,7 @@ describe("vercel createRemixHeaders", () => { describe("vercel createRemixRequest", () => { it("creates a request with the correct headers", async () => { - var request = createRequest({ + let request = createRequest({ method: "GET", url: "/foo/bar", headers: { From fc408262cd07545d257124fc239e6ee6533e838c Mon Sep 17 00:00:00 2001 From: Logan McAnsh Date: Thu, 12 Aug 2021 10:32:58 -0400 Subject: [PATCH 20/33] fix(express): read port from request app settings Signed-off-by: Logan McAnsh --- packages/remix-express/__tests__/server-test.ts | 13 +++++++++---- packages/remix-express/server.ts | 2 ++ 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/packages/remix-express/__tests__/server-test.ts b/packages/remix-express/__tests__/server-test.ts index 6d8b32d96a5..145e3602d51 100644 --- a/packages/remix-express/__tests__/server-test.ts +++ b/packages/remix-express/__tests__/server-test.ts @@ -195,7 +195,12 @@ describe("express createRemixRequest", () => { headers: { "Cache-Control": "max-age=300, s-maxage=3600" }, - pipe: jest.fn() + pipe: jest.fn(), + app: { + settings: { + port: "3000" + } + } }); expect(createRemixRequest(expressRequest)).toMatchInlineSnapshot(` @@ -223,12 +228,12 @@ describe("express createRemixRequest", () => { "parsedURL": Url { "auth": null, "hash": null, - "host": "localhost", + "host": "localhost:3000", "hostname": "localhost", - "href": "http://localhost/foo/bar", + "href": "http://localhost:3000/foo/bar", "path": "/foo/bar", "pathname": "/foo/bar", - "port": null, + "port": "3000", "protocol": "http:", "query": null, "search": null, diff --git a/packages/remix-express/server.ts b/packages/remix-express/server.ts index 7d426f243da..d83d513f5a8 100644 --- a/packages/remix-express/server.ts +++ b/packages/remix-express/server.ts @@ -87,6 +87,8 @@ export function createRemixHeaders( export function createRemixRequest(req: express.Request): Request { let origin = `${req.protocol}://${req.hostname}`; let url = new URL(req.url, origin); + let port = req.app.settings?.port; + if (port) url.port = port; let init: RequestInit = { method: req.method, From 5a92783536e23dd30c8b04824f465824ca665c99 Mon Sep 17 00:00:00 2001 From: Logan McAnsh Date: Thu, 12 Aug 2021 10:42:06 -0400 Subject: [PATCH 21/33] fix(express): req.get("host") returns the port Signed-off-by: Logan McAnsh --- packages/remix-express/server.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/remix-express/server.ts b/packages/remix-express/server.ts index d83d513f5a8..b0f9eb555ff 100644 --- a/packages/remix-express/server.ts +++ b/packages/remix-express/server.ts @@ -85,10 +85,8 @@ export function createRemixHeaders( } export function createRemixRequest(req: express.Request): Request { - let origin = `${req.protocol}://${req.hostname}`; + let origin = `${req.protocol}://${req.get("host")}`; let url = new URL(req.url, origin); - let port = req.app.settings?.port; - if (port) url.port = port; let init: RequestInit = { method: req.method, From c291fd7ada84a4bffe44a6b4885e523c7f19c7f4 Mon Sep 17 00:00:00 2001 From: Logan McAnsh Date: Thu, 12 Aug 2021 10:52:37 -0400 Subject: [PATCH 22/33] test(express): update request mocking Signed-off-by: Logan McAnsh --- .../remix-express/__tests__/server-test.ts | 19 +++++++++---------- packages/remix-express/package.json | 2 +- yarn.lock | 5 ----- 3 files changed, 10 insertions(+), 16 deletions(-) diff --git a/packages/remix-express/__tests__/server-test.ts b/packages/remix-express/__tests__/server-test.ts index 145e3602d51..938a64f0fbf 100644 --- a/packages/remix-express/__tests__/server-test.ts +++ b/packages/remix-express/__tests__/server-test.ts @@ -2,7 +2,7 @@ import express from "express"; import supertest from "supertest"; import { Response, Headers } from "@remix-run/node"; import { createRequestHandler as createRemixRequestHandler } from "@remix-run/node/server"; -import { getMockReq } from "@jest-mock/express"; +import { createRequest } from "node-mocks-http"; import { createRemixHeaders, @@ -188,18 +188,14 @@ describe("express createRemixHeaders", () => { describe("express createRemixRequest", () => { it("creates a request with the correct headers", async () => { - const expressRequest = getMockReq({ + const expressRequest = createRequest({ url: "/foo/bar", + method: "GET", protocol: "http", hostname: "localhost", headers: { - "Cache-Control": "max-age=300, s-maxage=3600" - }, - pipe: jest.fn(), - app: { - settings: { - port: "3000" - } + "Cache-Control": "max-age=300, s-maxage=3600", + Host: "localhost:3000" } }); @@ -219,9 +215,12 @@ describe("express createRemixRequest", () => { Symbol(Request internals): Object { "headers": Headers { Symbol(map): Object { - "Cache-Control": Array [ + "cache-control": Array [ "max-age=300, s-maxage=3600", ], + "host": Array [ + "localhost:3000", + ], }, }, "method": "GET", diff --git a/packages/remix-express/package.json b/packages/remix-express/package.json index aa582ec1ff6..d79e8588f66 100644 --- a/packages/remix-express/package.json +++ b/packages/remix-express/package.json @@ -10,9 +10,9 @@ "express": "^4.17.1" }, "devDependencies": { - "@jest-mock/express": "^1.4.3", "@types/express": "^4.17.9", "@types/supertest": "^2.0.10", + "node-mocks-http": "^1.10.1", "supertest": "^6.0.1" } } diff --git a/yarn.lock b/yarn.lock index 34442e1c1a9..b20a66c0175 100644 --- a/yarn.lock +++ b/yarn.lock @@ -959,11 +959,6 @@ resolved "https://registry.yarnpkg.com/@istanbuljs/schema/-/schema-0.1.3.tgz#e45e384e4b8ec16bce2fd903af78450f6bf7ec98" integrity sha512-ZXRY4jNvVgSVQ8DL3LTcakaAtXwTVUxE81hslsyD2AtoXW/wVob10HkOJ1X/pAlcI7D+2YoZKg5do8G/w6RYgA== -"@jest-mock/express@^1.4.3": - version "1.4.3" - resolved "https://registry.yarnpkg.com/@jest-mock/express/-/express-1.4.3.tgz#2849cbc82365d3bacf542f8ee58d5455d5478852" - integrity sha512-qf7BGFl0T+o1/PuBWaLjRp0fZIbuZOE/VHX6oBAez8gvYZzRmJAwafD9QBSOFQccLczn2d2SdMTzWFFUSTQ0lg== - "@jest/console@^26.6.2": version "26.6.2" resolved "https://registry.yarnpkg.com/@jest/console/-/console-26.6.2.tgz#4e04bc464014358b03ab4937805ee36a0aeb98f2" From 7b8a3ddd78260b143da9b86958d370a0ab7e5a2d Mon Sep 17 00:00:00 2001 From: Logan McAnsh Date: Fri, 13 Aug 2021 13:56:00 -0400 Subject: [PATCH 23/33] chore: update notes --- packages/remix-architect/__tests__/server-test.ts | 3 --- packages/remix-vercel/__tests__/server-test.ts | 5 ++++- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/remix-architect/__tests__/server-test.ts b/packages/remix-architect/__tests__/server-test.ts index d277b9fa390..d2040ebb7a5 100644 --- a/packages/remix-architect/__tests__/server-test.ts +++ b/packages/remix-architect/__tests__/server-test.ts @@ -74,7 +74,6 @@ describe("architect createRequestHandler", () => { return new Response(`URL: ${new URL(req.url).pathname}`); }); - // @ts-expect-error not sure how Ryan got this not to complain in the express tests.. await lambdaTester(createRequestHandler({ build: undefined })) .event(createMockEvent({ rawPath: "/foo/bar" })) .expectResolve(res => { @@ -88,7 +87,6 @@ describe("architect createRequestHandler", () => { return new Response("", { status: 204 }); }); - // @ts-expect-error not sure how Ryan got this not to complain in the express tests.. await lambdaTester(createRequestHandler({ build: undefined })) .event(createMockEvent({ rawPath: "/foo/bar" })) .expectResolve(res => { @@ -116,7 +114,6 @@ describe("architect createRequestHandler", () => { return new Response("", { headers }); }); - // @ts-expect-error not sure how Ryan got this not to complain in the express tests.. await lambdaTester(createRequestHandler({ build: undefined })) .event(createMockEvent({ rawPath: "/" })) .expectResolve(res => { diff --git a/packages/remix-vercel/__tests__/server-test.ts b/packages/remix-vercel/__tests__/server-test.ts index 2f563e93fcb..64bfe3d6c95 100644 --- a/packages/remix-vercel/__tests__/server-test.ts +++ b/packages/remix-vercel/__tests__/server-test.ts @@ -47,7 +47,8 @@ describe("vercel createRequestHandler", () => { return new Response(`URL: ${new URL(req.url).pathname}`); }); - let request = supertest(createApp()); + let request = supertest(createApp({})); + // note: vercel's createServerWithHelpers requires a x-now-bridge-request-id let res = await request .get("/foo/bar") .set({ "x-now-bridge-request-id": "2" }); @@ -62,6 +63,7 @@ describe("vercel createRequestHandler", () => { }); let request = supertest(createApp()); + // note: vercel's createServerWithHelpers requires a x-now-bridge-request-id let res = await request.get("/").set({ "x-now-bridge-request-id": "2" }); expect(res.status).toBe(204); @@ -86,6 +88,7 @@ describe("vercel createRequestHandler", () => { }); let request = supertest(createApp()); + // note: vercel's createServerWithHelpers requires a x-now-bridge-request-id let res = await request.get("/").set({ "x-now-bridge-request-id": "2" }); expect(res.headers["x-time-of-year"]).toBe("most wonderful"); From 9b7a9b899c0f8c0e29912861ed6479e078e7739b Mon Sep 17 00:00:00 2001 From: Logan McAnsh Date: Fri, 13 Aug 2021 18:54:17 -0400 Subject: [PATCH 24/33] fix(express): im dumb Signed-off-by: Logan McAnsh --- packages/remix-express/server.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/remix-express/server.ts b/packages/remix-express/server.ts index b0f9eb555ff..ee14db0dba9 100644 --- a/packages/remix-express/server.ts +++ b/packages/remix-express/server.ts @@ -70,14 +70,14 @@ export function createRemixHeaders( let headers = new Headers(); for (let [key, values] of Object.entries(requestHeaders)) { - if (!values) break; - - if (Array.isArray(values)) { - for (const value of values) { - headers.append(key, value); + if (values) { + if (Array.isArray(values)) { + for (const value of values) { + headers.append(key, value); + } + } else { + headers.set(key, values); } - } else { - headers.set(key, values); } } From 00512cd8aa72d85b1dde16d62b31faf6b193acf0 Mon Sep 17 00:00:00 2001 From: Logan McAnsh Date: Tue, 17 Aug 2021 12:09:46 -0400 Subject: [PATCH 25/33] feat: actually make multiple set-cookie headers work Signed-off-by: Logan McAnsh --- packages/remix-node/headers.ts | 66 +++++----------------------------- 1 file changed, 9 insertions(+), 57 deletions(-) diff --git a/packages/remix-node/headers.ts b/packages/remix-node/headers.ts index 491c4a676c8..03a76f0da4a 100644 --- a/packages/remix-node/headers.ts +++ b/packages/remix-node/headers.ts @@ -14,10 +14,12 @@ export function getDocumentHeaders( let loaderHeaders = routeLoaderResponses[index] ? routeLoaderResponses[index].headers : new Headers(); - let headers = new Headers( routeModule.headers - ? routeModule.headers({ loaderHeaders, parentHeaders }) + ? routeModule.headers({ + loaderHeaders, + parentHeaders + }) : undefined ); @@ -31,61 +33,11 @@ export function getDocumentHeaders( } function prependCookies(parentHeaders: Headers, childHeaders: Headers): void { - if (parentHeaders.has("Set-Cookie")) { - childHeaders.set( - "Set-Cookie", - concatSetCookieHeaders( - parentHeaders.get("Set-Cookie")!, - childHeaders.get("Set-Cookie") - ) - ); - } -} - -/** - * Merges two `Set-Cookie` headers, eliminating duplicates and preserving the - * original ordering. - */ -function concatSetCookieHeaders( - parentHeader: string, - childHeader: string | null -): string { - if (!childHeader || childHeader === parentHeader) { - return parentHeader; - } - - let finalCookies: RawSetCookies = new Map(); - let parentCookies = parseSetCookieHeader(parentHeader); - let childCookies = parseSetCookieHeader(childHeader); - - for (let [name, value] of parentCookies) { - finalCookies.set(name, childCookies.get(name) || value); - } - - for (let [name, value] of childCookies) { - if (!finalCookies.has(name)) { - finalCookies.set(name, value); + for (const [key, values] of Object.entries(parentHeaders.raw())) { + if (key.toLowerCase() === "set-cookie") { + for (const value of values) { + childHeaders.append(key, value); + } } } - - return serializeSetCookieHeader(finalCookies); -} - -type RawSetCookies = Map; - -function parseSetCookieHeader(header: string): RawSetCookies { - return header.split(/\s*,\s*/g).reduce((map, pair) => { - let [name, value] = pair.split("="); - return map.set(name, value); - }, new Map()); -} - -function serializeSetCookieHeader(cookies: RawSetCookies): string { - let pairs: string[] = []; - - for (let [name, value] of cookies) { - pairs.push(name + "=" + value); - } - - return pairs.join(", "); } From 94aa8e1e655475fc1f9da10a54640e9df8b390c3 Mon Sep 17 00:00:00 2001 From: Logan McAnsh Date: Tue, 17 Aug 2021 12:21:58 -0400 Subject: [PATCH 26/33] test: add test for multiple set cookie headers Signed-off-by: Logan McAnsh --- .../app/routes/multiple-set-cookies.tsx | 40 +++++++++++++++++++ .../tests/multiple-set-cookies-test.ts | 36 +++++++++++++++++ 2 files changed, 76 insertions(+) create mode 100644 fixtures/gists-app/app/routes/multiple-set-cookies.tsx create mode 100644 fixtures/gists-app/tests/multiple-set-cookies-test.ts diff --git a/fixtures/gists-app/app/routes/multiple-set-cookies.tsx b/fixtures/gists-app/app/routes/multiple-set-cookies.tsx new file mode 100644 index 00000000000..c2606af1c5d --- /dev/null +++ b/fixtures/gists-app/app/routes/multiple-set-cookies.tsx @@ -0,0 +1,40 @@ +import * as React from "react"; +import type { + ActionFunction, + LoaderFunction, + MetaFunction, + RouteComponent +} from "remix"; +import { redirect, json, Headers, Form } from "remix"; + +let loader: LoaderFunction = async ({ request }) => { + let headers = new Headers(); + headers.append("Set-Cookie", "foo=bar"); + headers.append("Set-Cookie", "bar=baz"); + return json({}, { headers }); +}; + +let action: ActionFunction = async () => { + let headers = new Headers(); + headers.append("Set-Cookie", "another=one"); + headers.append("Set-Cookie", "how-about=two"); + return redirect("/multiple-set-cookies", { headers }); +}; + +let meta: MetaFunction = () => ({ + title: "Multi Set Cookie Headers" +}); + +let MultipleSetCookiesPage: RouteComponent = () => { + return ( + <> +

👋

+
+ +
+ + ); +}; + +export default MultipleSetCookiesPage; +export { action, loader, meta }; diff --git a/fixtures/gists-app/tests/multiple-set-cookies-test.ts b/fixtures/gists-app/tests/multiple-set-cookies-test.ts new file mode 100644 index 00000000000..84744344911 --- /dev/null +++ b/fixtures/gists-app/tests/multiple-set-cookies-test.ts @@ -0,0 +1,36 @@ +import type { Browser, Page } from "puppeteer"; +import puppeteer from "puppeteer"; + +import { collectResponses } from "./utils"; + +const testPort = 3000; +const testServer = `http://localhost:${testPort}`; + +describe("can set multiple set cookies headers", () => { + let browser: Browser; + let page: Page; + + beforeEach(async () => { + browser = await puppeteer.launch(); + page = await browser.newPage(); + }); + + afterEach(() => browser.close()); + + describe("loader headers", () => { + it("are correct", async () => { + let responses = collectResponses( + page, + url => url.pathname === "/multiple-set-cookies" + ); + + await page.goto(`${testServer}/multiple-set-cookies`); + + expect(responses).toHaveLength(1); + expect(responses[0].headers()["set-cookie"]).toMatchInlineSnapshot(` + "foo=bar + bar=baz" + `); + }); + }); +}); From 863408a1a28dd5c655c09ea157380cd67faa4bc8 Mon Sep 17 00:00:00 2001 From: Logan McAnsh Date: Tue, 17 Aug 2021 12:27:35 -0400 Subject: [PATCH 27/33] test: update config snapshots due to new route Signed-off-by: Logan McAnsh --- fixtures/gists-app/app/routes/index.jsx | 3 +++ .../gists-app/tests/__snapshots__/server-html-test.ts.snap | 1 + packages/remix-dev/__tests__/readConfig-test.ts | 7 +++++++ 3 files changed, 11 insertions(+) diff --git a/fixtures/gists-app/app/routes/index.jsx b/fixtures/gists-app/app/routes/index.jsx index 57524380c26..2140240e5a5 100644 --- a/fixtures/gists-app/app/routes/index.jsx +++ b/fixtures/gists-app/app/routes/index.jsx @@ -75,6 +75,9 @@ export default function Index() { Render error in nested route with ErrorBoundary +
  • + Multiple Set Cookie Headers +
  • Preferences diff --git a/fixtures/gists-app/tests/__snapshots__/server-html-test.ts.snap b/fixtures/gists-app/tests/__snapshots__/server-html-test.ts.snap index 36c2481e1b9..9c817ec7742 100644 --- a/fixtures/gists-app/tests/__snapshots__/server-html-test.ts.snap +++ b/fixtures/gists-app/tests/__snapshots__/server-html-test.ts.snap @@ -29,6 +29,7 @@ exports[`the server HTML for the root URL is correct 1`] = ` >Render error in nested route with ErrorBoundary
    • +
  • Multiple Set Cookie Headers
  • Preferences
    • diff --git a/packages/remix-dev/__tests__/readConfig-test.ts b/packages/remix-dev/__tests__/readConfig-test.ts index 94b1d3df0c5..fb974991444 100644 --- a/packages/remix-dev/__tests__/readConfig-test.ts +++ b/packages/remix-dev/__tests__/readConfig-test.ts @@ -141,6 +141,13 @@ describe("readConfig", () => { "parentId": "root", "path": "methods", }, + "routes/multiple-set-cookies": Object { + "caseSensitive": false, + "file": "routes/multiple-set-cookies.tsx", + "id": "routes/multiple-set-cookies", + "parentId": "root", + "path": "multiple-set-cookies", + }, "routes/prefs": Object { "caseSensitive": false, "file": "routes/prefs.tsx", From 658927247705f649f26f0f1f059367702e7a2de9 Mon Sep 17 00:00:00 2001 From: Logan McAnsh Date: Wed, 18 Aug 2021 12:36:30 -0400 Subject: [PATCH 28/33] chore(vercel): remove extraneous setting of status code Signed-off-by: Logan McAnsh --- packages/remix-vercel/server.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/remix-vercel/server.ts b/packages/remix-vercel/server.ts index dd9dff76d49..d1119ce0247 100644 --- a/packages/remix-vercel/server.ts +++ b/packages/remix-vercel/server.ts @@ -91,8 +91,6 @@ export function createRemixRequest(req: NowRequest): Request { } function sendRemixResponse(res: NowResponse, response: Response): void { - res.status(response.status); - if (Buffer.isBuffer(response.body)) { return res .writeHead(response.status, response.headers.raw()) From 8930cd3fafaf1df4f7a95751e68c81502961e8c0 Mon Sep 17 00:00:00 2001 From: Michael Jackson Date: Wed, 18 Aug 2021 08:42:08 -0700 Subject: [PATCH 29/33] Add space after comma --- docs/guides/styling.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/guides/styling.md b/docs/guides/styling.md index a0604257c89..a9ec6885fa6 100644 --- a/docs/guides/styling.md +++ b/docs/guides/styling.md @@ -222,7 +222,7 @@ Here's some sample code to show how you might use Styled Components with Remix: ); - responseHeaders.set("Content-Type","text/html") + responseHeaders.set("Content-Type", "text/html") return new Response("" + markup, { status: responseStatusCode, From 37631e934c925048998ba705f68782ee89ede7e7 Mon Sep 17 00:00:00 2001 From: Michael Jackson Date: Wed, 18 Aug 2021 09:45:22 -0700 Subject: [PATCH 30/33] Code formatting --- packages/remix-server-runtime/headers.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/remix-server-runtime/headers.ts b/packages/remix-server-runtime/headers.ts index af8498f046d..6aff238de4c 100644 --- a/packages/remix-server-runtime/headers.ts +++ b/packages/remix-server-runtime/headers.ts @@ -14,10 +14,7 @@ export function getDocumentHeaders( : new Headers(); let headers = new Headers( routeModule.headers - ? routeModule.headers({ - loaderHeaders, - parentHeaders - }) + ? routeModule.headers({ loaderHeaders, parentHeaders }) : undefined ); From 1721032448c4e690ef50277538dee281a3e92f69 Mon Sep 17 00:00:00 2001 From: Logan McAnsh Date: Wed, 18 Aug 2021 13:35:04 -0400 Subject: [PATCH 31/33] feat(getDocumentHeaders): re-add support for multiple set-cookie headers Signed-off-by: Logan McAnsh --- .../gists-app/app/routes/multiple-set-cookies.tsx | 3 ++- packages/remix-server-runtime/headers.ts | 15 ++++++++------- packages/remix-server-runtime/package.json | 4 +++- yarn.lock | 12 ++++++++++++ 4 files changed, 25 insertions(+), 9 deletions(-) diff --git a/fixtures/gists-app/app/routes/multiple-set-cookies.tsx b/fixtures/gists-app/app/routes/multiple-set-cookies.tsx index c2606af1c5d..43620d9ea66 100644 --- a/fixtures/gists-app/app/routes/multiple-set-cookies.tsx +++ b/fixtures/gists-app/app/routes/multiple-set-cookies.tsx @@ -5,7 +5,8 @@ import type { MetaFunction, RouteComponent } from "remix"; -import { redirect, json, Headers, Form } from "remix"; +import { redirect, json, Form } from "remix"; +import { Headers } from "@remix-run/node"; let loader: LoaderFunction = async ({ request }) => { let headers = new Headers(); diff --git a/packages/remix-server-runtime/headers.ts b/packages/remix-server-runtime/headers.ts index 6aff238de4c..e4b233e162a 100644 --- a/packages/remix-server-runtime/headers.ts +++ b/packages/remix-server-runtime/headers.ts @@ -1,7 +1,7 @@ +import { splitCookiesString } from "set-cookie-parser"; import type { ServerBuild } from "./build"; import type { ServerRoute } from "./routes"; import type { RouteMatch } from "./routeMatching"; - export function getDocumentHeaders( build: ServerBuild, matches: RouteMatch[], @@ -28,11 +28,12 @@ export function getDocumentHeaders( } function prependCookies(parentHeaders: Headers, childHeaders: Headers): void { - for (const [key, values] of Object.entries(parentHeaders.raw())) { - if (key.toLowerCase() === "set-cookie") { - for (const value of values) { - childHeaders.append(key, value); - } - } + let parentSetCookieString = parentHeaders.get("Set-Cookie"); + + if (parentSetCookieString) { + let cookies = splitCookiesString(parentSetCookieString); + cookies.forEach(cookie => { + childHeaders.append("Set-Cookie", cookie); + }); } } diff --git a/packages/remix-server-runtime/package.json b/packages/remix-server-runtime/package.json index c1985e79faa..ad8009a87be 100644 --- a/packages/remix-server-runtime/package.json +++ b/packages/remix-server-runtime/package.json @@ -10,6 +10,7 @@ "history": "^5.0.0", "jsesc": "^3.0.1", "react-router-dom": "^6.0.0-beta.0", + "set-cookie-parser": "^2.4.8", "source-map": "^0.7.3" }, "peerDependencies": { @@ -17,7 +18,8 @@ "react-dom": ">=16.8" }, "devDependencies": { - "@types/jsesc": "^2.5.1" + "@types/jsesc": "^2.5.1", + "@types/set-cookie-parser": "^2.4.1" }, "sideEffects": false } diff --git a/yarn.lock b/yarn.lock index 863e2a4aa05..53984453a7b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1549,6 +1549,13 @@ "@types/mime" "^1" "@types/node" "*" +"@types/set-cookie-parser@^2.4.1": + version "2.4.1" + resolved "https://registry.yarnpkg.com/@types/set-cookie-parser/-/set-cookie-parser-2.4.1.tgz#49403d3150f6f296da8e51b3e9e7e562eaf105b4" + integrity sha512-N0IWe4vT1w5IOYdN9c9PNpQniHS+qe25W4tj4vfhJDJ9OkvA/YA55YUhaC+HNmMMeLlOSnBW9UMno0qlt5xu3Q== + dependencies: + "@types/node" "*" + "@types/signal-exit@^3.0.0": version "3.0.0" resolved "https://registry.yarnpkg.com/@types/signal-exit/-/signal-exit-3.0.0.tgz#75e3b17660cf1f6c6cb8557675b4e680e43bbf36" @@ -6916,6 +6923,11 @@ set-blocking@^2.0.0: resolved "https://registry.yarnpkg.com/set-blocking/-/set-blocking-2.0.0.tgz#045f9782d011ae9a6803ddd382b24392b3d890f7" integrity sha1-BF+XgtARrppoA93TgrJDkrPYkPc= +set-cookie-parser@^2.4.8: + version "2.4.8" + resolved "https://registry.yarnpkg.com/set-cookie-parser/-/set-cookie-parser-2.4.8.tgz#d0da0ed388bc8f24e706a391f9c9e252a13c58b2" + integrity sha512-edRH8mBKEWNVIVMKejNnuJxleqYE/ZSdcT8/Nem9/mmosx12pctd80s2Oy00KNZzrogMZS5mauK2/ymL1bvlvg== + set-getter@^0.1.0: version "0.1.0" resolved "https://registry.yarnpkg.com/set-getter/-/set-getter-0.1.0.tgz#d769c182c9d5a51f409145f2fba82e5e86e80376" From 601d67a0707df8daf274995de6bd6d434eee678a Mon Sep 17 00:00:00 2001 From: Logan McAnsh Date: Wed, 18 Aug 2021 13:40:08 -0400 Subject: [PATCH 32/33] test(arc,vercel): update mock import Signed-off-by: Logan McAnsh --- packages/remix-architect/__tests__/server-test.ts | 4 ++-- packages/remix-vercel/__tests__/server-test.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/remix-architect/__tests__/server-test.ts b/packages/remix-architect/__tests__/server-test.ts index d2040ebb7a5..45d5c2f95f3 100644 --- a/packages/remix-architect/__tests__/server-test.ts +++ b/packages/remix-architect/__tests__/server-test.ts @@ -1,6 +1,6 @@ import lambdaTester from "lambda-tester"; import { Response, Headers } from "@remix-run/node"; -import { createRequestHandler as createRemixRequestHandler } from "@remix-run/node/server"; +import { createRequestHandler as createRemixRequestHandler } from "@remix-run/server-runtime"; import { createRemixHeaders, @@ -11,7 +11,7 @@ import { APIGatewayProxyEventV2 } from "aws-lambda"; // We don't want to test that the remix server works here (that's what the // puppetteer tests do), we just want to test the architect adapter -jest.mock("@remix-run/node/server"); +jest.mock("@remix-run/server-runtime"); let mockedCreateRequestHandler = createRemixRequestHandler as jest.MockedFunction< typeof createRemixRequestHandler >; diff --git a/packages/remix-vercel/__tests__/server-test.ts b/packages/remix-vercel/__tests__/server-test.ts index 64bfe3d6c95..9d8c16cfd8e 100644 --- a/packages/remix-vercel/__tests__/server-test.ts +++ b/packages/remix-vercel/__tests__/server-test.ts @@ -1,6 +1,6 @@ import supertest from "supertest"; import { Response, Headers } from "@remix-run/node"; -import { createRequestHandler as createRemixRequestHandler } from "@remix-run/node/server"; +import { createRequestHandler as createRemixRequestHandler } from "@remix-run/server-runtime"; import { createRequest } from "node-mocks-http"; import { createServerWithHelpers } from "@vercel/node/dist/helpers"; import { VercelRequest } from "@vercel/node"; @@ -13,7 +13,7 @@ import { // We don't want to test that the remix server works here (that's what the // puppetteer tests do), we just want to test the vercel adapter -jest.mock("@remix-run/node/server"); +jest.mock("@remix-run/server-runtime"); let mockedCreateRequestHandler = createRemixRequestHandler as jest.MockedFunction< typeof createRemixRequestHandler >; From 7df7d19900031f8a3ef98776477df33288fe8da7 Mon Sep 17 00:00:00 2001 From: Logan McAnsh Date: Wed, 18 Aug 2021 14:08:29 -0400 Subject: [PATCH 33/33] chore: update changelog Signed-off-by: Logan McAnsh --- CHANGES.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index 6998f3cf9c1..c0a8c84eea8 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -13,6 +13,8 @@ - Updated .npmrc to minimum required version for webcrypto API (v15) - Abstracted req and res types through the platform - Add base64 encoding primitives to node globals (atob and btoa) +- adds and improves testing around node adapters +- fixes the ability to set multiple Set-Cookie headers This is a history of changes to [Remix](https://remix.run).