From 560c968f533c0bb3dc44d1d0b23b65e0a0136736 Mon Sep 17 00:00:00 2001
From: dabdine <1955040+dabdine@users.noreply.github.com>
Date: Fri, 2 Dec 2022 11:42:55 -0800
Subject: [PATCH] Fix 4199: TypedResponse allows incompatible types (#4734)

* Fixes #4199: Do not allow assignment of incompatible TypedResponses

* Add myself to contributors.yml

* Create light-sheep-give.md

* slight changeset tweak

* additional changeset tweaks

Co-authored-by: Pedro Cattori <pcattori@gmail.com>
---
 .changeset/light-sheep-give.md                | 26 +++++++++++++++++++
 contributors.yml                              |  1 +
 .../__tests__/responses-test.ts               |  5 ++++
 packages/remix-server-runtime/responses.ts    |  2 +-
 4 files changed, 33 insertions(+), 1 deletion(-)
 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..16e4efa030a
--- /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<SomeType>`,
+Typescript would not show errors when the loader or action returned an incompatible type.
+
+For example:
+
+```ts
+export const action = async (args: ActionArgs): Promise<TypedResponse<string>> => {
+    return json(42);
+};
+```
+
+In this case, Typescript would not show an error even though `42` is clearly not a `string`.
+
+This happens because `json` returns a `TypedResponse<string>`,
+but because `TypedReponse<string>` was previously just `Response & { json: () => Promise<string> }`
+and `Response` already defines `{ json: () => Promise<any> }`, type erasure caused `Promise<any>` to be used for `42`.
+
+To fix this, we explicitly omit the `Response`'s `json` property before intersecting with `{ json: () => Promise<T> }`.
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
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<TypedResponse<number>, 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 = <Data extends unknown>(
 
 // must be a type since this is a subtype of response
 // interfaces must conform to the types they extend
-export type TypedResponse<T extends unknown = unknown> = Response & {
+export type TypedResponse<T extends unknown = unknown> = Omit<Response, 'json'> & {
   json(): Promise<T>;
 };