prisma / prisma

Next-generation ORM for Node.js & TypeScript | PostgreSQL, MySQL, MariaDB, SQL Server, SQLite, MongoDB and CockroachDB
https://www.prisma.io
Apache License 2.0
39.35k stars 1.54k forks source link

Use of Readonly types instead of mutable in TS typings #5025

Open PinkaminaDianePie opened 4 years ago

PinkaminaDianePie commented 4 years ago

Currently all arrays in input positions use Array type and it leads to a lot of issues, such as incompatibility with Facebook DataLoader library. DataLoader expect keys to be a ReadonlyArray, while Prisma expects {where: { id: { in: keys } }} to accept mutable Array. I think it's safe to change types of arrays in input positions to ReadonlyArray as well (i believe Prisma doesn't mutate them under the hood, so it's safe to do that), while keeping types in output positions as Array (we shouldn't enforce users to keep result array immutable, so we can't use ReadonlyArray here). If I understand correctly, currently we use only one generic for all input types: export declare type Enumerable<T> = T | Array<T>;, so changing it to export declare type Enumerable<T> = T | ReadonlyArray<T>; should be enough.

It would be nice to have immutable types for objects as well, like

export declare type UserCreateInput = Readonly<{
    id?: string | null;
    name: string;
}>;

instead of

export declare type UserCreateInput = {
    id?: string | null;
    name: string;
};

because of the same reasons as described above. If Prisma doesn't mutate this object, it's better to mark it as immutable, so user will be able to pass both mutable/immutable objects, not only mutable ones like right now

reiv commented 4 years ago

Seeing as it's been several months now - is this still being considered? At least the addition of ReadonlyArray<T> to the definition of Enumerable as @PinkaminaDianePie suggested seems like a trivial and non-breaking change.

I suppose in the meantime one could cook up a quick and dirty regex to post-process the generated index.d.ts, but that's obviously not optimal.

Sytten commented 4 years ago

Yeah I faced the same issue the other day, minor annoyance that could be easily fixed @timsuchanek

timsuchanek commented 3 years ago

Thanks everyone for your comments! After getting seduced to a quick fix, we realized, that this has negative consequences for other use-cases that we don't want to introduce.

Example:

import { IntFilter, PrismaClient } from '@prisma/client'

async function main() {
  const prisma = new PrismaClient()

  const ids: number[] = []

  const filter: IntFilter = {
    in: []
  }

  if (ids && Array.isArray(filter.in)) {
    filter.in.push(...ids)
  }

  const data = await prisma.user.findMany({
    where: {
      id: filter
    }
  })

  console.log(data)
  prisma.$disconnect()
}

In words: It takes the opportunity to programmatically, type-safe construct arguments. The way to go here is to already assign the Prisma input type like IntFilter to the variable and then mutate the object as needed. That's a very common pattern (not just with arrays) that's being used with Prisma Client, which we still want to allow.

Sytten commented 3 years ago

Agreed but what about setting Enumerate to

export type Enumerable<T> = T | Array<T> | ReadonlyArray<T>

That would solve my issue with dataloaders. Dataloaders use a ReadonlyArray in the load method so currently I have to cast it to an array. https://github.com/graphql/dataloader/blob/master/src/index.d.ts#L74

pantharshit00 commented 3 years ago

Another report: https://github.com/prisma/prisma/issues/5743

I added the candidate label to this.

dimaMachina commented 3 years ago

Any news? Or otherwise how I can overwrite Prisma Enumarable type with https://github.com/prisma/prisma/issues/5025#issuecomment-732396483 solution?

chrisui commented 2 years ago

Any chance we could get another look at this? It would appear there isn't really a great reason why prisma could not accept a readonly array as an input because no mutation should be expected within prisma-land of inputs.

flevi29 commented 2 years ago

This might not belong here but I leave it here in the hopes that someone might find it useful. I just need to know whether a property can be Enumerable or not, and if it is just simplify it to a dead simple array T[]. I really have a hard time understanding generics, but I got a working solution somehow:

import { Prisma } from '@prisma/client';

type Unpacked<T> = T extends (infer U)[] ? U : never;

type PrismaMappedSomething = {
  [k in keyof Prisma.PrismaSomething]: Unpacked<
    Prisma.PrismaSomething[k]
  > extends never
    ? Prisma.PrismaSomething[k]
    : Unpacked<Prisma.PrismaSomething[k]>[];
};

It can probably be simplified and made more elegant but any attempt at it failed for me.