Skip to content

Commit

Permalink
Fix 4199: TypedResponse allows incompatible types (#4734)
Browse files Browse the repository at this point in the history
* 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>
  • Loading branch information
dabdine and pcattori committed Dec 2, 2022
1 parent 48349f4 commit 560c968
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 1 deletion.
26 changes: 26 additions & 0 deletions .changeset/light-sheep-give.md
Original file line number Diff line number Diff line change
@@ -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> }`.
1 change: 1 addition & 0 deletions contributors.yml
Original file line number Diff line number Diff line change
Expand Up @@ -452,3 +452,4 @@
- zachdtaylor
- zainfathoni
- zhe
- dabdine
5 changes: 5 additions & 0 deletions packages/remix-server-runtime/__tests__/responses-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion packages/remix-server-runtime/responses.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
};

Expand Down

0 comments on commit 560c968

Please sign in to comment.