Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(client): use objects for DbNull, JsonNull, AnyNull #13783

Merged
merged 34 commits into from
Jun 24, 2022
Merged

Conversation

aqrln
Copy link
Member

@aqrln aqrln commented Jun 13, 2022

Closes https://github.com/prisma/client-planning/issues/18
Fixes #9243

Make DbNull, JsonNull and AnyNull special objects.

Breaking change: when passing "DbNull", "JsonNull" and "AnyNull" strings to JSON fields, they will now always be interpreted as strings (while previously they could be interpreted either as strings or as special values).

  • If you use literal "DbNull", "JsonNull" and "AnyNull" strings, please update to use Prisma.DbNull, Prisma.JsonNull and Prisma.AnyNull. If you are already using the named constants (which should be the case for most people since that's how it's documented they should be used), no action is required on your side.
  • If you now get a type error when passing Prisma.DbNull as the value of a JSON field, this indicates a bug in your code which wasn't caught by our types previously. The field you are trying to store DbNull in is not nullable in the schema, and a literal "DbNull" string was stored in the database instead of NULL.
  • If you had to sanitize user input to not be equal to "DbNull" or "JsonNull" before using it as a value of a JSON column, this is no longer needed, it is safe to pass it as is. Most people would use String and not Json columns in the cases which would have been affected by this, but I'm mentioning it anyway because it was raised as one of the concerns in the original issue.

First iteration notes

This implementation is fairly generic on the runtime side, but I had to dumb down the generation side compared to what I had initially.

My first approach was to make it possible to mark any enum as symbol enum, generate a registry of all used symbols in Prisma.Symbols namespace on-the-fly as such enums are encountered when we traverse the DMMF and then reference them in each specific enum from that registry.

It's important that we can't just declare the symbols as unique symbol in each specific enum in isolation because that would make, e.g., JsonNull from JsonNullValueInput and NullableJsonNullValueInput incompatible between each other on the type level, while we want to preserve the behavior of string literal enums that makes enum variants with identical names interchangeable across different enums — otherwise it won't be possible to use the global helpers in the Prisma namespace like Prisma.JsonNull.

However, that didn't play well with those helpers in a different aspect: since they are hardcoded and always present, they would need Prisma.Symbols.JsonNull, Prisma.Symbols.DbNull and Prisma.Symbols.AnyNull to always be present as well, which, as it turned out, is not always the case, since they would only be generated if JSON fields are used in the schema.

Thus I decided to declare the top-level helpers as unique symbols and reference them directly from the Prisma namespace in the generated enums.

If we need any new symbol enums in the future and we don't want to pollute the Prisma namespace with their variants, it will still be possible to re-introduce the symbols namespace and generate the top-level helpers conditionally (possibly even defined either as a reference to a symbol from the registry or as a unique symbol stub for compatibility, so that all global symbols are always present). However, for now it seems like an overkill.

Second iteration notes

DbNull, JsonNull and AnyNull are now special objects instead of symbols. This allows for much better type errors when these values are misused.

Before:

Type 'unique symbol' is not assignable to type 'unique symbol | InputJsonValue'.

After:

Type 'DbNull' is not assignable to type 'JsonNull | InputJsonValue'.

@aqrln aqrln requested review from a team and SevInf and removed request for a team June 13, 2022 12:15
Copy link
Contributor

@SevInf SevInf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left couple of questions/suggestions, but LGTM overall.

packages/client/src/runtime/utils/common.ts Outdated Show resolved Hide resolved
packages/client/src/runtime/utils/common.ts Outdated Show resolved Hide resolved
packages/client/src/generation/TSClient/common.ts Outdated Show resolved Hide resolved
@aqrln aqrln added this to the 4.0.x milestone Jun 13, 2022
Copy link
Member

@millsp millsp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, nice work 👏

@aqrln aqrln requested review from SevInf and millsp June 22, 2022 21:42
@aqrln aqrln changed the title fix(client): use symbols for DbNull, JsonNull, AnyNull fix(client): use objects for DbNull, JsonNull, AnyNull Jun 22, 2022
@aqrln aqrln merged commit 372fc97 into main Jun 24, 2022
@aqrln aqrln deleted the prisma4/null-symbols branch June 24, 2022 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Avoid using magic string values for JsonNull/DbNull
3 participants