JSON type throws error on certain types of valid JSON on create and update

[thesunny]

Thank you @Jolg42 for addressing the previous two JSON issues:

• JSON types should return a JSON object and not a JavaScript object #691
• JSON type is broken for array. Array comes back as string. prisma#2619 (comment)

This issue is the last (I think) for end-to-end JSON typing to work.

Bug description

We get a TypeScript error during a create or update for valid JSON in a JSON column if it contains optional properties like this.

type UserJSON = {
  name: string
  age?: number
}

The reason this happens is because TypeScript transforms the value of the age property by adding undefined so that it becomes number | undefined. The preceding type and the following type are identical:

type UserJSON = {
  name: string
  age?: number | undefined
}

This is a known limitation of TypeScript. It cannot distinguish a missing property from a property which can be set to undefined. microsoft/TypeScript#13195

How to reproduce

• Create any prisma model with a JSON column
• Do a create or update to the JSON column with a valid JSON object that has an optional property

This code clarifies why this happens:

// JSON values from `type-fest` as used in Prisma
export type JsonObject = {[Key in string]?: JsonValue};
export interface JsonArray extends Array<JsonValue> {}
export type JsonValue = string | number | boolean | null | JsonObject | JsonArray;

type User = {
  name: string,
  age?: number
}

type IsUserJSON = User extends JsonObject ? true : false
// false but should be true

Expected behavior

The way to fix it is to define a UJsonValue like this and use that to type JSON values in a create or update:

// JSON values with `undefined` allowed
export type UJsonObject = { [Key in string]?: UJsonValue }
export interface UJsonArray extends Array<UJsonValue> {}
export type UJsonValue =
  | undefined
  | string
  | number
  | boolean
  | null
  | UJsonObject
  | UJsonArray

type User = {
  name: string
  age?: number
}

type IsUserUJSON = User extends UJsonObject ? true : false
// true

The most accurate and strictest way to type this is to have the find methods return a JsonValue and the create and update methods to accept a UJsonValue.

This works because JSON data coming out of Prisma is guaranteed to be in the restrictive JsonValue type. It never contains undefined.

In an end to end API, that value comes out of the database will then be typed into something like User.

The end user makes a change and saves it to Prisma. The JSON data coming into Prisma should accept UJsonValue. If it is typed to accept JsonValue, we get a TypeScript error even though User is valid JSON.

The less accurate method is to add undefined to JsonValue and be done with it; however, this affects users that are strictly typing their API against JsonValue as typed in type-fest. It won't affect me and I think most users because if it flows through an API, that API has to accept undefined or else it breaks whenever there is an optional property (ie. same problem in a different place).

In order of desirability, it goes like this:

• JsonValue for find and UJsonValue for create and update (Fixes all cases)
• JsonValue extended with undefined (solves all my use cases and probably most people)
• JsonValue as it is (breaks valid JSON with optional properties on create and update)

## Prisma information

Beta 2.0.0-beta7

Environment & setup

• OS: Mac OS
• Database: PostgreSQL
• Prisma version: 2.0.0-beta7
• Node.js version: 12.16.2

[timsuchanek]

Thanks for raising the issue @thesunny !

If we add undefined, we also would need to add Date, as you could pass in dates, which are persisted as strings with JSON.stringify.
Then I'm wondering if we don't just want to support any, as we're fine with everything, as long as it's serializable.

Is there anything from your side that speaks against using any here?

[thesunny]

Hi @timsuchanek,

I think it's prudent to apply the Principle of Least Surprise https://en.wikipedia.org/wiki/Principle_of_least_astonishment

One of the criticisms of old MySQL was that it would silently type cast things that in other databases would raise errors.

In the same way, it feels surprising to me to pass in a Date, a Function an enum or a class and get back a string. In a situation where I intend this to happen, it makes sense to need to be explicit and cast them to a string.

For me, the principle of least surprise for an ORM is that if I put a certain type of data in, it comes back as the same type.

undefined is different because we aren't trying to add an invalid data type undefined into the database. We instead want to be able to create data types with optional values which are valid values for a JSON column type.

Due to a limitation in TypeScript, we can't actually describe optional values without also adding undefined. This is a limitation in TypeScript at the moment microsoft/TypeScript#13195. If it is ever fixed, I'd recommend at that time to remove undefined from create and update.

Also, for clarity, for data coming out of Prisma (ie. find), keeping it as JSON can help move data through an API like in nextjs that can only accept JSON values.

By the way, I've made workaround to all these issues for myself so they are no longer critical for me; however, Prisma is so great, that I really want it to gain greater adoption. I think paying attention to these little details will pay off, kind of in the same way that you've paid attention to getting the typing right with partial selects and your really interesting and smart choices around how foreign keys affect the call signatures. Prisma has literally caused me to create better models because of how the better models create better UI.

[timsuchanek]

Thanks for the elaborate answer and the kind words.

We'll call your UJsonValue InputJsonValue to keep it with our existing conventions and apart from that implement it as you suggested.

[timsuchanek]

Thanks a lot for reporting 🙏
This issue is fixed in the latest dev version of @prisma/cli.
You can try it out with npm i -g @prisma/cli@dev.

In case it’s not fixed for you - please let us know and we’ll reopen this issue!

[thesunny]

Hi @timsuchanek

Thanks for the update. Really appreciate the work you and everyone on your team put into Prisma.