chrishoermann / zod-prisma-types

Generator creates zod types for your prisma models with advanced validation
Other
616 stars 47 forks source link

Invalid imports for an ESM project? #49

Closed pedropmedina closed 1 year ago

pedropmedina commented 1 year ago

In a project using type: module the build fails as the top prima client import generated looks like this:

import * as PrismaClient from "../../../node_modules/.prisma/client";

The ESM project expects the .js extension.

Is there a work around for this?

chrishoermann commented 1 year ago

@pedropmedina thx for the report. I'll look into it. What I found on a quick search is that node has an option to skip out on that requirement of adding a file extension for ESM modules - maybe you could try this workaround and see if it helps?

pedropmedina commented 1 year ago

Hey @chrishoermann thanks for the quick response. I was wondering why we need to import the prisma client in the first place. Wouldn't it be possible to generate the zod schemas while generating and ditch the client all together?

export const ContactAddressBookEntityAddressArgsSchema: z.ZodType<PrismaClient.Prisma.ContactAddressBookEntityAddressArgs> = z.object({
  select: z.lazy(() => ContactAddressBookEntityAddressSelectSchema).optional(),
  include: z.lazy(() => ContactAddressBookEntityAddressIncludeSchema).optional(),
}).strict();

Is the z.ZodType<PrismaClient.Prisma.ContactAddressBookEntityAddressArgs> necessary in the example above?

chrishoermann commented 1 year ago

@pedropmedina In your mentioned schema the type is necessary to enable the use of recursive types as stated in the docs. Sure, it would be possible to recreate this types from the primsa dmmf and use these instead but this would be a redundancy that would nearly double the amount of code that needs to be generated.

two other things are that enums are created with zod.nativeEnum(Prisma.MyEnum) and that Decimal is checked with the Prisma.Decimal.isDecimal(number) typeguard. It would be no problem to switch the enums to zod.enum([....]) but the check of decimals would mean that decimaljs needs to be added as a dependeny to your project and then needs to be imported into the generated file. I think that would not be that great of a developer experience especially because everything we need is already present in the prisma index.d.ts.

One thing I wonder in your inital report is that the import is not

import * as PrismaClient from "@prisma/client"

but

import * as PrismaClient from "../../../node_modules/.prisma/client"

Is this a custom prisma client output path you use?

btw. did the workaround with disabling the file extension in node work?

pedropmedina commented 1 year ago

@chrishoermann makes sense.

Is this a custom prisma client output path you use?

yes, I am using a custom output path as I'm working in a monorepo and I have several prisma databases. Have experience working on a monorepo with several prisma clients and if so how do handle it?

btw. did the workaround with disabling the file extension in node work?

I kept this workspace as commonjs for now. I will probably look into converting back to esm soon with your recommendation.

chrishoermann commented 1 year ago

Have experience working on a monorepo with several prisma clients and if so how do handle it?

Sadly no. I only worked on projects with one client and I'm also not that experienced in using esm. It bugged me once when I didn't have the time to dig into the problems, so I sticked with commonjs.

pedropmedina commented 1 year ago

Hey @chrishoermann this continues to be an issue. No sure if others are experiencing the same or perhaps most people are using cjs, but in my case I'm forced to use esm and the prisma client import is breaking everything.

I would like to propose a solution which I think might be easy to implement. Here're the steps:

1 - Some time ago you try implementing a custom path for the prisma client, but then you removed it as that issue was regarding custom outputs and not esm imports. I think that having the option to override the import as first implemented in that issue would solve part of this problem as we will be able to specify the file ext .js.

2 - Replace the default import to import { Prisma } from '@prisma/client/{custompath}' instead of import * as PrismaClient from '@prisma/client' which add the runtime as well into the import causing it to be added to the final bundle and eventually breaking the app.

3 - Everywhere in the file where you have PrismaClient namespace e.g. PrismaClient.Prisma.SomeType needs to be replaced to Prisma.SomeType.

Does this make any sense and will you be willing to make this changes?

Thanks a lot for this lib!

cqh963852 commented 1 year ago

Replace the default import to import { Prisma } from '@prisma/client/{custompath}' instead of import * as PrismaClient from '@prisma/client' which add the runtime as well into the import causing it to be added to the final bundle and eventually breaking the app.

This is unachievable. @prisma/client has a export *

image

pedropmedina commented 1 year ago

What do you mean by unachievable @cqh963852? Also I want to confirm that the issue with the runtime bundled into the final file was fixed by properly importing the types like import { type Customer } from @prisma/client. It wasn't an issue with this lib.

chrishoermann commented 1 year ago

@pedropmedina I'm close to getting the version that supports generation of multifiles ready. There the Prisma namespace is imported directly like import { Prisma } from '@prisma/client/{custompath}. The final version or at least a beta should be ready by the weekend if nothing unexpected happens.

pedropmedina commented 1 year ago

Awesome @chrishoermann, looking forward to it. Thank you!

cqh963852 commented 1 year ago

@pedropmedina

unachievable

I means If you want

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

Prisma.xxx

be

import type { xxx } from "@prisma/client/Prisma";

It is unachievable. Because @prisma/client package has no sub path. The impl is export * from '.prisma/client'


Do you mean

import type { Prisma } from "@prisma/client";
pedropmedina commented 1 year ago

I have a monorepo and I setting the prisma custom output to the node_modules/@prisma/client/{appname} thus Prisma will be accessible from there @cqh963852.

What I meant is that we could consume just { Prisma } instead of the entire export since we don't really need anything else.

Either way, the problem I was facing wasn't really related to this.

I see @chrishoermann mentioned he'll be moving things over to this import style moving forwards. I do think we need to have a way of overriding the import line as in my case since I'm setting a custom location the path looks like this ../../../../node_modules/@prisma/client/{customapp} and since we're inside of the node_modules I could simply convert it to @prisma/client/{customapp}. This is useful in the instance of a monorepo since the production build won't be relative to the outputted relative path.

Another option could be to determine whether this is a node_module dependency and handle this automatically @chrishoermann ?

chrishoermann commented 1 year ago

@pedropmedina I just released version 2.0.0-beta.0 that supports generation of multiple files and also has the possibility of adding a custom path to the prismaCient. The beta docs can be found here.

npm:

npm install zod-prisma-types@2.0.0-beta.0

yarn:

yarn add zod-prisma-types@2.0.0-beta.0

pnpm:

pnpm add zod-prisma-types@2.0.0-beta.0
pedropmedina commented 1 year ago

Awesome work @chrishoermann. Thanks a lot for this lib and your effort! I will look into it tonight. When are you planing for the official release?

chrishoermann commented 1 year ago

@pedropmedina I think in the next few days it should be ready for the official release. I just hope to get bit of feedback from the community before making it official since there are a few breaking changes (like handling of custom imports). I'm sure there are a few use cases that I didn't think of that can cause problems. I also need to write a few more tests and check if the docs need some more refinement.

yukimotochern commented 1 year ago

I also thinks that the dependency to prisma client could be a problem. I am using the generated zod schema in react project in a mono repo setup. Prisma client don't work in browser and I would not want to bundle it event if it works there.

chrishoermann commented 1 year ago

@Yukimotochern It is a good point but I think it would be a bit of an overkill to recreate every input- and outputType when these types are already present in the Prisma namespace. And correct me if I'm wrong but the types should be stripped from the code when bundling? The only things that come to my mind that do not rely just on prisma namespace types are enums (they are not in the prisma namespace) with the current z.nativeenum() implementation and decimals with the reliance on z.instanceof(Prisma.Decimal).

Enums are no problem to change but I need to check if the decimal implementation would also work as expected without reliance on the Prisma.Decimal Class.

chrishoermann commented 1 year ago

@Yukimotochern I forgot to mention that when you use the useMultipleFiles option only the Prisma namespace is imported and not the whole client.

chrishoermann commented 1 year ago

@Yukimotochern in 2.0.0-beta.4 only the Prisma namespace is imported instead of the whole prisma client content. Also, if you don't have any json fields, Prisma is imported just as a type. For json fields I currently need Prisma to be imported not as a type since the transformJsonNull function relies on Prisma.DbNull and Prisma.JsonNull. Maybe I find a solution how to circumvent this requirement and import Prisma always as a type.

pedropmedina commented 1 year ago

Hi @chrishoermann I was trying 2.0.0-beta.0 and 2.0.0-beta.4 and nothing seems to be generated. I tried tweaking the setup, but that did nothing. Am I missing something?

Also I don't know how you feel about this, but in the past I have had issue git and naming files and directories with camel and pascal case. Wouldn't it make more sense to stick to kebab-case instead?

Here's my config:

generator zod-prisma-types {
  provider                         = "zod-prisma-types"
  output                           = "./generated/zod"
  prismaClientPath                 = "@prisma/client/netsuite"
  useMultipleFiles                 = true
  createInputTypes                 = true
  useDefaultValidators             = false
  createOptionalDefaultValuesTypes = false
  addInputTypeValidation           = false
}
chrishoermann commented 1 year ago

@pedropmedina what do you mean by nothing is generated? not even the ouput folder? Was the ✔ Generated Zod Prisma Types to .\prisma\generated\zod in 31ms message shown after generation?

Regarding kebab-case: Do you mean file naming? I think this would get a bit messy because for imports the genertaor relies on the name of the file to be the same as the name of the schema.

pedropmedina commented 1 year ago

The files get generated, but there's not content in them @chrishoermann.

Regarding kebab-case: Do you mean file naming? I think this would get a bit messy because for imports the genertaor relies on the name of the file to be the same as the name of the schema.

Yeah, I was referring to the file and dir naming.

chrishoermann commented 1 year ago

That is strange. To me that seems not to be an issue with the genereator. The generator uses fs.writeFile to create files and write the stringified content from code-block-writer.

pedropmedina commented 1 year ago

Yeah, it's weird. In fact not even the defaults (where we write to a single index.ts file) is working. Which version of node are you on? I'm using v16.16.0 @chrishoermann

pedropmedina commented 1 year ago

There's some weird behavior taking place. Sometimes it generates some of the content for modelSchemas/*Schema.ts files, sometimes it does not generate anything. ~ 90% of the generated files have not content in them. The inputTypeSchemas and outputTypeSchemas generated files are entirely empty. Anyone else experiencing this issue??

chrishoermann commented 1 year ago

@pedropmedina I'm currently on node version 16.17.1

pedropmedina commented 1 year ago

I updated to the same version, removed everything, reinstall node_modules and nothing @chrishoermann. Also tested on multiple machines. Not even the single file export works. There's definitely something off in the new 2.* release as version 1.6.8 works fine.

chrishoermann commented 1 year ago

@pedropmedina can you please try version 2.0.3? I changed fs.writeFile to fs.writeFileSync hopefully this solves the issue.

pedropmedina commented 1 year ago

That's fixed it @chrishoermann. The only issue I'm seeing is with the prisma client path.

This is my setup:

generator zod-prisma-types {
  provider             = "zod-prisma-types"
  output               = "./generated/zod"
  prismaClientPath     = "@prisma/client/netsuite"
  useMultipleFiles     = true
  useDefaultValidators = false

Here's the path generated: import { type Prisma } from '../../../../../../../../prisma/@prisma/client/netsuite';

chrishoermann commented 1 year ago

@pedropmedina sorry my bad. I overengineered a bit. should work now in v 2.0.4

pedropmedina commented 1 year ago

Thank you @chrishoermann. I ran into another issue regarding relationships where the same schema gets imported twice on the same file. Take as an example the Contact model below:

model Contact {
  id                     Int       @id @unique

  company                Int?

  defaultBillingAddressRef    ContactAddressBookEntityAddress? @relation("default_billing_address_ref", fields: [defaultBillingAddress], references: [nkey])

 defaultShippingAddressRef   ContactAddressBookEntityAddress? @relation("default_shipping_address_ref", fields: [defaultShippingAddress], references: [nkey])
 ...

Generates something like this:

import { z } from 'zod';
import { type Prisma } from '@prisma/client/netsuite';
import { CountryArgsSchema } from '../outputTypeSchemas/CountryArgsSchema';
import { ContactAddressBookArgsSchema } from '../outputTypeSchemas/ContactAddressBookArgsSchema';
import { ContactFindManyArgsSchema } from '../outputTypeSchemas/ContactFindManyArgsSchema'; // <- duplicate
import { ContactFindManyArgsSchema } from '../outputTypeSchemas/ContactFindManyArgsSchema';  // <- duplicate
import { ContactAddressBookEntityAddressCountOutputTypeArgsSchema } from '../outputTypeSchemas/ContactAddressBookEntityAddressCountOutputTypeArgsSchema';

export const ContactAddressBookEntityAddressIncludeSchema: z.ZodType<Prisma.ContactAddressBookEntityAddressInclude> = z.object({
  countryRef: z.union([z.boolean(),z.lazy(() => CountryArgsSchema)]).optional(),
  contactAddressBookRef: z.union([z.boolean(),z.lazy(() => ContactAddressBookArgsSchema)]).optional(),
  contactsDefaultBillingAddressRef: z.union([z.boolean(),z.lazy(() => ContactFindManyArgsSchema)]).optional(),
  contactsDefaultShippingAddressRef: z.union([z.boolean(),z.lazy(() => ContactFindManyArgsSchema)]).optional(),
  _count: z.union([z.boolean(),z.lazy(() => ContactAddressBookEntityAddressCountOutputTypeArgsSchema)]).optional(),
}).strict()

export default ContactAddressBookEntityAddressIncludeSchema;
chrishoermann commented 1 year ago

@pedropmedina is fixed in 2.0.5. I didn't use a Set in the include schema files. This caused the duplicate imports.

pedropmedina commented 1 year ago

Hi @chrishoermann we're almost there. Imports look good in inputTypeSchema/ and modelSchema directories, however the outputTypeSchemas still shows duplicates

import { ContactAddressBookArgsSchema } from '../outputTypeSchemas/ContactAddressBookArgsSchema';
import { ContactFindManyArgsSchema } from '../outputTypeSchemas/ContactFindManyArgsSchema'; // <- dup
import { ContactFindManyArgsSchema } from '../outputTypeSchemas/ContactFindManyArgsSchema'; // <- dup
import { ContactAddressBookEntityAddressCountOutputTypeArgsSchema } from '../outputTypeSchemas/ContactAddressBookEntityAddressCountOutputTypeArgsSchema';
chrishoermann commented 1 year ago

@pedropmedina thanx for the hint, you sould get a bug bounty :wink: Should now be fixed in latest version

chrishoermann commented 1 year ago

closing since I think the points in this issue have been addressed.