refactor(remix-server-runtime): reimplement Jsonify type utility

[pcattori]

Fixes #7246 , #3931

type-fest implementation is causing type errors for Remix users. Tried fixing the errors in type-fest, but the repo setup causes some errors not to be reproducible there (and to be inconsistent with Typescript playgrounds). Additionally, there were many test cases in type-fest for Jsonify whose "expected vs actual" I disagreed with.

So for now, we can reintroduce our own implementation. Later, type-fest can choose which parts they wish to adopt or not.

Testing strategy

Tests are collocated in the same file as the implementation and are typecheck-only tests.

TODO

• Implement @nrako 's suggestion
• changeset
• later: write up a doc to address misunderstandings from Incorrect type from useLoaderData #3931

[changeset-bot]

🦋 Changeset detected

Latest commit: be4cb07

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 16 packages

Name	Type
@remix-run/react	Patch
@remix-run/server-runtime	Patch
@remix-run/testing	Patch
@remix-run/cloudflare	Patch
@remix-run/deno	Patch
@remix-run/dev	Patch
@remix-run/node	Patch
@remix-run/cloudflare-pages	Patch
@remix-run/cloudflare-workers	Patch
@remix-run/architect	Patch
@remix-run/express	Patch
@remix-run/serve	Patch
create-remix	Patch
remix	Patch
@remix-run/css-bundle	Patch
@remix-run/eslint-config	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[nrako]

@pcattori - I've recently noticed your PR, and I indeed believe that it makes sense to repatriate this logic back to Remix, especially since the adoption of type-fest seems to be causing significant migration issues for several Remix's users. Moreover, this feature, in my humble opinion, is quite crucial and central to Remix, and since only Jsonify is used, I think your changes are a very sensible approach for Remix.

However, I'd like to bring to your attention the following PR I had submitted for type-fest: sindresorhus/type-fest#690

This PR addresses an issue with type-fest, which I experienced through Remix v2, and seeing your PR, I'm wondering if you could incorporate my patch into your own PR.

[nrako]

To help you understand the gist of the problem, please find bellow an example of a patch:

diff --git a/packages/remix-server-runtime/jsonify.ts b/packages/remix-server-runtime/jsonify.ts
index 9531f3d51..ebdbd67c0 100644
--- a/packages/remix-server-runtime/jsonify.ts
+++ b/packages/remix-server-runtime/jsonify.ts
@@ -5,6 +5,9 @@ export type Jsonify<T> =
   // any
   IsAny<T> extends true ? any :
 
+  // toJSON which is tested first in case an object also implement a "JSONPrimitive" interface
+  T extends { toJSON(): infer U } ? (U extends JsonValue ? U : unknown) :
+
   // primitives
   T extends JsonPrimitive ? T :
   T extends String ? string :
@@ -21,9 +24,6 @@ export type Jsonify<T> =
   // Not JSON serializable
   T extends NotJson ? never :
 
-  // toJSON
-  T extends { toJSON(): infer U } ? (U extends JsonValue ? U : unknown) :
-
   // tuple & array
   T extends [] ? [] :
   T extends readonly [infer F, ...infer R] ? [NeverToNull<Jsonify<F>>, ...Jsonify<R>] :
@@ -140,6 +140,7 @@ export type _tests = [
   Expect<Equal<Jsonify<Date>, string>>,
   Expect<Equal<Jsonify<{ toJSON(): undefined }>, unknown>>,
   Expect<Equal<Jsonify<{ toJSON(): Date }>, unknown>>,
+  Expect<Equal<Jsonify<BooleanWithToJson>, string>>,
 
   // tuple & array
   Expect<Equal<Jsonify<[]>, []>>,
@@ -244,3 +245,7 @@ type EmptyObject = { [emptyObjectSymbol]?: never };
 
 // adapted from https://github.com/type-challenges/type-challenges/blob/main/utils/index.d.ts
 type IsAny<T> = 0 extends 1 & T ? true : false;
+
+interface BooleanWithToJson extends Boolean {
+  toJSON(): string;
+}

p.s: I like your approach of simply collocating the typecheck-only tests in the file directly 👍🏽

EDIT: you might prefer to move the toJSON typechecks tests first to match the new cases order

[pcattori]

@nrako thanks for the suggestion! I included your improvement and added you as a co-author for that commit

[github-actions]

🤖 Hello there,

We just published version 2.1.0-pre.0 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

[github-actions]

🤖 Hello there,

We just published version 2.1.0 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!