From 27787cc56bafd67ba8398d87af33bcbe10d8a6e3 Mon Sep 17 00:00:00 2001 From: Derek Abdine <1955040+dabdine@users.noreply.github.com> Date: Thu, 1 Dec 2022 10:36:44 -0800 Subject: [PATCH 1/5] Fixes #4199: Do not allow assignment of incompatible TypedResponses --- packages/remix-server-runtime/__tests__/responses-test.ts | 5 +++++ packages/remix-server-runtime/responses.ts | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/remix-server-runtime/__tests__/responses-test.ts b/packages/remix-server-runtime/__tests__/responses-test.ts index a6b92b311f0..0506d28c965 100644 --- a/packages/remix-server-runtime/__tests__/responses-test.ts +++ b/packages/remix-server-runtime/__tests__/responses-test.ts @@ -44,6 +44,11 @@ describe("json", () => { expect(result).toMatchObject({ hello: "remix" }); }); + it("disallows unmatched typed responses", async () => { + let response = json("hello"); + isEqual, typeof response>(false); + }); + it("disallows unserializables", () => { // @ts-expect-error expect(() => json(124n)).toThrow(); diff --git a/packages/remix-server-runtime/responses.ts b/packages/remix-server-runtime/responses.ts index 748f98b1d86..dd1dbad3ed3 100644 --- a/packages/remix-server-runtime/responses.ts +++ b/packages/remix-server-runtime/responses.ts @@ -5,7 +5,7 @@ export type JsonFunction = ( // must be a type since this is a subtype of response // interfaces must conform to the types they extend -export type TypedResponse = Response & { +export type TypedResponse = Omit & { json(): Promise; }; From 987348fe268dd44087dcf1664219362d7a9dda95 Mon Sep 17 00:00:00 2001 From: dabdine <1955040+dabdine@users.noreply.github.com> Date: Thu, 1 Dec 2022 10:40:08 -0800 Subject: [PATCH 2/5] Add myself to contributors.yml --- contributors.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/contributors.yml b/contributors.yml index 18c8c6b362d..6a99603e613 100644 --- a/contributors.yml +++ b/contributors.yml @@ -452,3 +452,4 @@ - zachdtaylor - zainfathoni - zhe +- dabdine From 612dc47f76a205cc67891cee941322b4d8a768e4 Mon Sep 17 00:00:00 2001 From: Pedro Cattori Date: Fri, 2 Dec 2022 10:59:02 -0500 Subject: [PATCH 3/5] Create light-sheep-give.md --- .changeset/light-sheep-give.md | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 .changeset/light-sheep-give.md diff --git a/.changeset/light-sheep-give.md b/.changeset/light-sheep-give.md new file mode 100644 index 00000000000..530fbabe907 --- /dev/null +++ b/.changeset/light-sheep-give.md @@ -0,0 +1,26 @@ +--- +"remix": patch +"@remix-run/serve": patch +"@remix-run/server-runtime": patch +--- + +Fix `TypedResponse` so that Typescript correctly shows errors for incompatible types in loaders and actions. + +Previously, when the return type of a loader or action was explicitly set to `TypedResponse`, +Typescript would not show errors when the loader or action returned an incompatible type. + +For example: + +```ts +export const action = async (args: ActionArgs): Promise> => { + return json(42); +}; +``` + +In this case, Typescript would now show an error even though `42` is clearly not a `string`. + +In this case, happens because `json` returns a `TypedResponse`, +but because `TypedReponse` is just `Response & { json: () => Promise }` +and `Response` already defines `{ json: () => Promise }`, then type erasure caused `Promise` to be used for `42`. + +To fix this, we explicitly omit the `Response`'s `json` property before intersecting with `{ json: () => Promise }`. From d9f1f21f0a97fb97fa8cfa5619970008bdb5be88 Mon Sep 17 00:00:00 2001 From: Derek Abdine <1955040+dabdine@users.noreply.github.com> Date: Fri, 2 Dec 2022 09:36:08 -0800 Subject: [PATCH 4/5] slight changeset tweak --- .changeset/light-sheep-give.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.changeset/light-sheep-give.md b/.changeset/light-sheep-give.md index 530fbabe907..9bd48a8cb40 100644 --- a/.changeset/light-sheep-give.md +++ b/.changeset/light-sheep-give.md @@ -17,10 +17,10 @@ export const action = async (args: ActionArgs): Promise> = }; ``` -In this case, Typescript would now show an error even though `42` is clearly not a `string`. +In this case, Typescript would not show an error even though `42` is clearly not a `string`. -In this case, happens because `json` returns a `TypedResponse`, +This happens because `json` returns a `TypedResponse`, but because `TypedReponse` is just `Response & { json: () => Promise }` -and `Response` already defines `{ json: () => Promise }`, then type erasure caused `Promise` to be used for `42`. +and `Response` already defines `{ json: () => Promise }`, type erasure causes `Promise` to be used for `42`. To fix this, we explicitly omit the `Response`'s `json` property before intersecting with `{ json: () => Promise }`. From e9bb4ef450c2a8060562fc61006b2e6963994fcf Mon Sep 17 00:00:00 2001 From: Derek Abdine <1955040+dabdine@users.noreply.github.com> Date: Fri, 2 Dec 2022 09:39:29 -0800 Subject: [PATCH 5/5] additional changeset tweaks --- .changeset/light-sheep-give.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.changeset/light-sheep-give.md b/.changeset/light-sheep-give.md index 9bd48a8cb40..16e4efa030a 100644 --- a/.changeset/light-sheep-give.md +++ b/.changeset/light-sheep-give.md @@ -20,7 +20,7 @@ export const action = async (args: ActionArgs): Promise> = In this case, Typescript would not show an error even though `42` is clearly not a `string`. This happens because `json` returns a `TypedResponse`, -but because `TypedReponse` is just `Response & { json: () => Promise }` -and `Response` already defines `{ json: () => Promise }`, type erasure causes `Promise` to be used for `42`. +but because `TypedReponse` was previously just `Response & { json: () => Promise }` +and `Response` already defines `{ json: () => Promise }`, type erasure caused `Promise` to be used for `42`. To fix this, we explicitly omit the `Response`'s `json` property before intersecting with `{ json: () => Promise }`.