drizzle-team / drizzle-orm

Headless TypeScript ORM with a head. Runs on Node, Bun and Deno. Lives on the Edge and yes, it's a JavaScript ORM too 😅
https://orm.drizzle.team
Apache License 2.0
21.77k stars 495 forks source link

[BUG]: Splitting database schemas into a separate package in monorepo causes TS2345 Property `[IsDrizzleTable]` is missing in type #1558

Closed ari-becker closed 3 days ago

ari-becker commented 7 months ago

What version of drizzle-orm are you using?

0.29.0

What version of drizzle-kit are you using?

0.20.4 (but not relevant)

Describe the Bug

  1. Copy/paste the example drizzle adapter code from https://github.com/nextauthjs/next-auth/blob/693a0723a7bad2b0950e4c6006a99c98852c4beb/packages/adapter-drizzle/src/lib/pg.ts into my own project
  2. Note that the example code works (leave aside the deprecated primaryKey(col1, col2), which I fixed).
  3. My project is a monorepo, so the database schemas (which were in Prisma) are in a separate package. Move the table definitions for users, accounts, sessions, and verificationTokens into a separate package, where tsc --build is producing a dist/ folder and package.json's exports references dist/index.d.ts etc., then import the table definitions in the original app package which is making the insert calls.
  4. Receive the following error: TS2345 and Property [isDrizzleTable] is missing in type:

image

the compiled .d.ts looks like this:

image

Expected behavior

Not to have the tsc error. I expect the types to be portable.

Environment & setup

I use a monorepo with pnpm and syncpack and have confirmed that both the main app package and the database schema package are using the same 0.29.0 version of drizzle-orm.

sonipranjal commented 7 months ago

Facing the same issue

sonipranjal commented 7 months ago

Fixed!

It was because I was using different versions of drizzle orm inside my workspaces. So if you update all the drizzle orms to the same version it will work nicely!!

ari-becker commented 7 months ago

Still an issue in 0.29.1.

Verified it wasn't due to some kind of caching issue in Webstorm, here's the raw tsc --build output: image

Triple-checked that the versions of drizzle-orm are the same: image

Angelelz commented 7 months ago

I've had a monorepo setup for forever with drizzle in its separate package and never had this issue. Can you create a reproduction issue to investigate?

ari-becker commented 7 months ago

ok @Angelelz I think I have a reproduction for you: https://github.com/ari-becker/drizzle-1558-repro

  1. run pnpm install
  2. run pnpm build:tsc

There's a lot of errors (hazard of copy/pasting/deleting) but the relevant one is the second one:

image

and just to verify again that versions match:

image

delivey commented 7 months ago

Facing same issue. Versions match as well.

Angelelz commented 7 months ago

ok @Angelelz I think I have a reproduction for you: https://github.com/ari-becker/drizzle-1558-repro

  1. run pnpm install
  2. run pnpm build:tsc

There's a lot of errors (hazard of copy/pasting/deleting) but the relevant one is the second one:

image

and just to verify again that versions match:

image

Just submitted a pr to your reproduction. Please see my notes there. Take a look and let me know if we can close this issue.

ari-becker commented 7 months ago

ok @Angelelz I think I have a reproduction for you: https://github.com/ari-becker/drizzle-1558-repro

  1. run pnpm install
  2. run pnpm build:tsc

There's a lot of errors (hazard of copy/pasting/deleting) but the relevant one is the second one: image and just to verify again that versions match: image

Just submitted a pr to your reproduction. Please see my notes there. Take a look and let me know if we can close this issue.

@Angelelz I see your PR. Thanks for taking the time to run through it. Adding a few comments here just to keep the discussion in one place -

a) I switched back module and moduleResolution to node 16 and tsc --build still passes. So that's not the issue (and anyway, for a database schema, they really need to be node16 so that I can write Node code to interface with the database)

b) Because you eliminated the type errors by using as any, I'm wondering if doing so isn't also squashing the [IsDrizzleTable] type errors. So let me spend some time trying to refine the reproduction repo so that there's no applicative type errors, and only the [IsDrizzleTable] type error.

ari-becker commented 7 months ago

Hi @Angelelz ,

I cleaned up the code so that there's a single error: https://github.com/ari-becker/drizzle-1558-repro/tree/v2

image

Interestingly, if I now do what you first did, which is from here to go into packages/tsconfig/base.json and set module to ES2022 and moduleResolution to bundler, then the build passes. Do you have any insight into why node16 would cause this error? Is it not supported by Drizzle, or something like that? I do need node16 since, after all, the code accessing the database is a Node backend and not running in the browser...

Angelelz commented 7 months ago

Is it not supported by Drizzle, or something like that?

I think it's quite the opposite. The drizzle team have done a lot to support both modern and legacy js. If you see the built code (for example in npm) you'll see that for each file in the tree, our build system has produced 6 files with the following extensions: .cjs .cjs.map .d.cts .d.ts .js .js.map. I haven't dug too much into all the module resolution strategies and target builds fiasco in typescript, but I think you have a couple of options:

  1. You can leave base.json as is and just override the module and moduleResolution flags in packages/database/tsconfig.json or
  2. You might want to try to import from either "drizzle-orm/pg-core/index.cts" or "drizzle-orm/pg-core/index.js" in you packages/database/src/drizzle/auth.ts file.

Let me know how it goes.

ari-becker commented 7 months ago

I'm really at a loss, I guess my Typescript-foo just isn't at a level high enough to really debug this.

The following seems to be sufficient to trigger the bug, which strips out everything related to monorepos, typescript project references, etc.:

import {
  text,
  pgTable,
} from "drizzle-orm/pg-core";

const IsDrizzleTable = Symbol.for("drizzle:IsDrizzleTable")

export const user = pgTable("User", {
  id: text("id")
    .notNull()
    .primaryKey(),
}) satisfies {[IsDrizzleTable]: true};

image

which is of course found in every table because of this line: https://github.com/drizzle-team/drizzle-orm/blob/0a4e3b265ce121675e7baa14f3a39669ea387e6d/drizzle-orm/src/table.ts#L105 using the same Symbol.for defined here: https://github.com/drizzle-team/drizzle-orm/blob/0a4e3b265ce121675e7baa14f3a39669ea387e6d/drizzle-orm/src/table.ts#L39 . It's not marked as @internal so Typescript isn't stripping it out before packaging drizzle-orm either.

Because it's not found, TS doesn't recognize it as a table when passing it into db.select() etc.

Angelelz commented 7 months ago

which is of course found in every table because of this line:

https://github.com/drizzle-team/drizzle-orm/blob/0a4e3b265ce121675e7baa14f3a39669ea387e6d/drizzle-orm/src/table.ts#L105

using the same Symbol.for defined here: https://github.com/drizzle-team/drizzle-orm/blob/0a4e3b265ce121675e7baa14f3a39669ea387e6d/drizzle-orm/src/table.ts#L39

. It's not marked as @internal so Typescript isn't stripping it out before packaging drizzle-orm either. Because it's not found, TS doesn't recognize it as a table when passing it into db.select() etc.

This is why I don't think it's an issue in drizzle but in your setup.

I submitted another PR to your secondary branch. Basically applied the module and moduleResolution but to that specific package, that shouldn't affect the rest of your application. Also removed the extension from the index.ts.

Let me know.

ari-becker commented 7 months ago

@Angelelz I think this whole discussion over bundler vs node16 is leading to the root cause.

I see that drizzle-orm is using esnext (which currently resolves to es2022) and bundler: https://github.com/drizzle-team/drizzle-orm/blob/0a4e3b265ce121675e7baa14f3a39669ea387e6d/tsconfig.json#L7 . I remember reading somewhere that setting it as such in a library is basically "contagious" in that it forces users using the library to also use bundler. But let me find a source for that for you

ari-becker commented 7 months ago

Yeah the relevant guidance is from the Typescript handbook: https://www.typescriptlang.org/docs/handbook/modules/theory.html?#module-resolution-for-libraries

In short, "moduleResolution": "bundler" is infectious, allowing code that only works in bundlers to be produced.

Except... the code that tsx is generating is producing code that imports with .js extensions added, so that it should work with node16. Maybe some kind of bug with tsx then? Still researching...

Angelelz commented 7 months ago

If you look at the built code in your node_modules (or in npm), you can see that there is no tsconfig.json.

Only the package.json and the many, many files in different formats to be used by whatever setup you have. It's just .js with some d.ts. So I don't think it matters as long as it's valid JavaScript.

Edit: did you check if it would work better by installing drizzle-orm in the main package.json as opposed to installing it in the package? Or the opposite I didn't check that.

ari-becker commented 7 months ago

Only the package.json and the many, many files in different formats to be used by whatever setup you have. It's just .js with some d.ts. So I don't think it matters as long as it's valid JavaScript.

I still wonder. I skimmed the build script, it's doing a lot of custom stuff. Maybe it's all correct (certainly this issue isn't attracting a lot of attention), maybe not. When I turned off skipLibCheck in the tsconfig, I got a lot of errors. I didn't bring it up before because they don't seem to be related to this, and it's well known that skipLibCheck: false can bring up a lot of false positives when the types aren't generated by tsc:

skipLibCheck errors ```text node_modules/.pnpm/drizzle-orm@0.29.1_postgres@3.4.3/node_modules/drizzle-orm/mysql-core/query-builders/delete.d.ts(30,22): error TS2420: Class 'MySqlDeleteBase' incorrectly implements interface 'SQLWrapper'. Property 'getSQL' is missing in type 'MySqlDeleteBase' but required in type 'SQLWrapper'. node_modules/.pnpm/drizzle-orm@0.29.1_postgres@3.4.3/node_modules/drizzle-orm/mysql-core/query-builders/select.d.ts(199,244): error TS2344: Type 'string' does not satisfy the constraint 'keyof this & string'. Type 'string' is not assignable to type 'keyof this & string'. Type 'string' is not assignable to type 'keyof this'. Type '"session"' is not assignable to type '"_" | "getSQL" | "as" | "union" | "intersect" | "except" | "where" | "toSQL" | "$dynamic" | "having" | "orderBy" | "groupBy" | "limit" | "offset" | "leftJoin" | "rightJoin" | "innerJoin" | ... 4 more ... | "exceptAll"'. node_modules/.pnpm/drizzle-orm@0.29.1_postgres@3.4.3/node_modules/drizzle-orm/mysql-core/query-builders/select.d.ts(225,247): error TS2344: Type 'string' does not satisfy the constraint 'keyof this & string'. node_modules/.pnpm/drizzle-orm@0.29.1_postgres@3.4.3/node_modules/drizzle-orm/mysql-core/query-builders/select.d.ts(251,248): error TS2344: Type 'string' does not satisfy the constraint 'keyof this & string'. node_modules/.pnpm/drizzle-orm@0.29.1_postgres@3.4.3/node_modules/drizzle-orm/mysql-core/query-builders/select.d.ts(292,251): error TS2344: Type 'string' does not satisfy the constraint 'keyof this & string'. node_modules/.pnpm/drizzle-orm@0.29.1_postgres@3.4.3/node_modules/drizzle-orm/mysql-core/query-builders/select.d.ts(318,245): error TS2344: Type 'string' does not satisfy the constraint 'keyof this & string'. node_modules/.pnpm/drizzle-orm@0.29.1_postgres@3.4.3/node_modules/drizzle-orm/mysql-core/query-builders/select.d.ts(359,248): error TS2344: Type 'string' does not satisfy the constraint 'keyof this & string'. node_modules/.pnpm/drizzle-orm@0.29.1_postgres@3.4.3/node_modules/drizzle-orm/mysql-core/query-builders/select.d.ts(511,22): error TS18052: Non-abstract class 'MySqlSelectBase' does not implement all abstract members of 'MySqlSelectQueryBuilderBase' node_modules/.pnpm/drizzle-orm@0.29.1_postgres@3.4.3/node_modules/drizzle-orm/mysql-core/db.d.ts(1,38): error TS2307: Cannot find module 'mysql2/promise' or its corresponding type declarations. node_modules/.pnpm/drizzle-orm@0.29.1_postgres@3.4.3/node_modules/drizzle-orm/mysql-core/query-builders/select.types.d.ts(130,1149): error TS2344: Type 'MySqlSetOperatorExcludedMethods' does not satisfy the constraint '"_" | "getSQL" | "as" | "union" | "intersect" | "except" | "where" | "toSQL" | "prepare" | "execute" | "iterator" | "$dynamic" | "catch" | "finally" | "then" | "having" | "orderBy" | ... 10 more ... | "exceptAll"'. Type '"session"' is not assignable to type '"_" | "getSQL" | "as" | "union" | "intersect" | "except" | "where" | "toSQL" | "prepare" | "execute" | "iterator" | "$dynamic" | "catch" | "finally" | "then" | "having" | "orderBy" | ... 10 more ... | "exceptAll"'. node_modules/.pnpm/drizzle-orm@0.29.1_postgres@3.4.3/node_modules/drizzle-orm/sqlite-core/columns/blob.d.ts(2,23): error TS2688: Cannot find type definition file for 'bun-types'. node_modules/.pnpm/drizzle-orm@0.29.1_postgres@3.4.3/node_modules/drizzle-orm/sqlite-core/query-builders/query.d.ts(25,22): error TS2420: Class 'SQLiteRelationalQuery' incorrectly implements interface 'SQLWrapper'. Property 'getSQL' is missing in type 'SQLiteRelationalQuery' but required in type 'SQLWrapper'. node_modules/.pnpm/drizzle-orm@0.29.1_postgres@3.4.3/node_modules/drizzle-orm/sqlite-core/query-builders/raw.d.ts(11,22): error TS2420: Class 'SQLiteRaw' incorrectly implements interface 'SQLWrapper'. Property 'getSQL' is missing in type 'SQLiteRaw' but required in type 'SQLWrapper'. node_modules/.pnpm/drizzle-orm@0.29.1_postgres@3.4.3/node_modules/drizzle-orm/sqlite-core/query-builders/select.d.ts(202,247): error TS2344: Type 'string' does not satisfy the constraint 'keyof this & string'. Type 'string' is not assignable to type 'keyof this & string'. Type 'string' is not assignable to type 'keyof this'. Type '"config"' is not assignable to type '"_" | "getSQL" | "as" | "union" | "intersect" | "except" | "where" | "toSQL" | "$dynamic" | "having" | "orderBy" | "groupBy" | "limit" | "offset" | "leftJoin" | "rightJoin" | "innerJoin" | "fullJoin" | "unionAll"'. node_modules/.pnpm/drizzle-orm@0.29.1_postgres@3.4.3/node_modules/drizzle-orm/sqlite-core/query-builders/select.d.ts(228,250): error TS2344: Type 'string' does not satisfy the constraint 'keyof this & string'. node_modules/.pnpm/drizzle-orm@0.29.1_postgres@3.4.3/node_modules/drizzle-orm/sqlite-core/query-builders/select.d.ts(254,251): error TS2344: Type 'string' does not satisfy the constraint 'keyof this & string'. node_modules/.pnpm/drizzle-orm@0.29.1_postgres@3.4.3/node_modules/drizzle-orm/sqlite-core/query-builders/select.d.ts(280,248): error TS2344: Type 'string' does not satisfy the constraint 'keyof this & string'. node_modules/.pnpm/drizzle-orm@0.29.1_postgres@3.4.3/node_modules/drizzle-orm/sqlite-core/query-builders/select.d.ts(421,22): error TS18052: Non-abstract class 'SQLiteSelectBase' does not implement all abstract members of 'SQLiteSelectQueryBuilderBase' node_modules/.pnpm/drizzle-orm@0.29.1_postgres@3.4.3/node_modules/drizzle-orm/sqlite-core/query-builders/select.types.d.ts(122,1157): error TS2344: Type 'SQLiteSetOperatorExcludedMethods' does not satisfy the constraint '"_" | "getSQL" | "values" | "as" | "union" | "intersect" | "except" | "where" | "toSQL" | "prepare" | "execute" | "$dynamic" | "catch" | "finally" | "then" | "having" | "orderBy" | ... 10 more ... | "run"'. Type '"config"' is not assignable to type '"_" | "getSQL" | "values" | "as" | "union" | "intersect" | "except" | "where" | "toSQL" | "prepare" | "execute" | "$dynamic" | "catch" | "finally" | "then" | "having" | "orderBy" | ... 10 more ... | "run"'. node_modules/.pnpm/drizzle-orm@0.29.1_postgres@3.4.3/node_modules/drizzle-orm/pg-core/query-builders/delete.d.ts(32,22): error TS2420: Class 'PgDeleteBase' incorrectly implements interface 'SQLWrapper'. Property 'getSQL' is missing in type 'PgDeleteBase' but required in type 'SQLWrapper'. node_modules/.pnpm/drizzle-orm@0.29.1_postgres@3.4.3/node_modules/drizzle-orm/pg-core/query-builders/select.d.ts(521,22): error TS18052: Non-abstract class 'PgSelectBase' does not implement all abstract members of 'PgSelectQueryBuilderBase' ```

but I'm kinda getting to the end of the timebox of what I'm willing to spend on this.

Edit: did you check if it would work better by installing drizzle-orm in the main package.json as opposed to installing it in the package? Or the opposite I didn't check that.

This wouldn't work in pnpm by default, the node_modules for the package doesn't include the root node_modules, to improve isolation.

Angelelz commented 7 months ago

Your issue is that for some reason typescript isn't picking up the symbols properties from the super class Table [IsDrizzleTable], even though you can clearly see they are there. Even if you look at the table.d.ts file you can see the symbol declared there.

The build script is specifically designed to generate all those files so it works in every setup.

Also the skipLibCheck shouldn't have anything to do with your issue, Drizzle adds all the drivers as peerDependencies, so the user install only what they need, it you don't have the other installed, typescript will yell at you even if you're not using them. It also adds an exports property that declares all the files that are exported, in js and cjs formats, also the types.

I ran out of ideas as well.

This solution doesn't work for you either?

ari-becker commented 7 months ago

This solution doesn't work for you either?

No, by changing moduleResolution to node, typescript no longer uses the exports field in package.json but the legacy main and types fields only. Like bundler, it would likely force me to adopt tsup / esbuild, which are incompatible with Typescript's project references, which means using something like turbo to coordinate building the packages in order (and even then, Turbo doesn't really support a watch mode) for type-checking, instead of just tsc --build --watch.

If I needed to make this work, i.e. for my day job, then I'd do something like that - but for a side-project it's making me wonder if I shouldn't just write raw queries in postgres.js, have manually managed types, and get back to building.

Angelelz commented 7 months ago

I consulted the team, They're looking into the bundler thing. I wasn't aware of that and the consequences, mostly because it hasn't affected my own setup. We'll see.

Angelelz commented 6 months ago

@ari-becker What do you think should be the correct module resolution in drizzle?

ari-becker commented 6 months ago

@ari-becker What do you think should be the correct module resolution in drizzle?

As per Typescript's recommendation, node16. The recommendation calls for nodenext, but this is an unstable pointer that currently points to node16, so, node16 for stability.

Angelelz commented 6 months ago

I was playing around with it yesterday, I tried changing the module and the module resolution, I tried with both node16 and nodenext, I pnpm linked it, but I didn't see any difference. I'll investigate more tonight if I have time.

I have the suspicion you problem is not in tsconfig, but it's actually the possibly different instances of drizzle installed in your next app and in your packages. I didn't look deeper but if you have time, try installing drizzle at the root of the monorepo. I'm not an expert in pnpm workspaces in monorepos so I don't know if that's even possible.

ari-becker commented 6 months ago

I was playing around with it yesterday, I tried changing the module and the module resolution, I tried with both node16 and nodenext, I pnpm linked it, but I didn't see any difference. I'll investigate more tonight if I have time.

I have the suspicion you problem is not in tsconfig, but it's actually the possibly different instances of drizzle installed in your next app and in your packages. I didn't look deeper but if you have time, try installing drizzle at the root of the monorepo. I'm not an expert in pnpm workspaces in monorepos so I don't know if that's even possible.

image

They are symlinked to the exact same location in the pnpm store.

Arvinje commented 6 months ago

Faced the exact same error. Turned out I was using different drizzle-orm package versions within packages of the monorepo. Using a unified version fixed the issue for me.

Angelelz commented 6 months ago

@ari-becker I believe I found what is the problem with your setup. You see, drizzle-orm ships with support for both commonjs and ECMA modules at the same folder location, hence 6 different file extension per typescript file in the source. In your setup, the database package was using modules in the module resolution, but the nextjs package was using bundles (which results in commonjs). So your database was pulling the package from the module implementation in drizzle, and your nextjs package was pulling it from the commonjs implementation. Even though they're the same version, there were different symbol definitions in each, hence, the type errors. This was not easy to figure out, but you either need to have both packages pull from the module implementation or the commonjs implementation, you'll have to choose. I opened another PR on your repro with the sole change to the tsconfig on your nextjs package, to make it pull from the module implementation. Please confirm this fixes the issue so we can close.

ari-becker commented 6 months ago

Please confirm this fixes the issue so we can close.

I trust that tsc --build works on that. I can't use it in my real monorepo though because NextJS doesn't support moduleResolution: node16.

Are the symbols a hard requirement? Sounds like this bug goes away if the symbols are just dropped from the original drizzle code.

Angelelz commented 6 months ago

Are the symbols a hard requirement? Sounds like this bug goes away if the symbols are just dropped from the original drizzle code.

I did attempt to remove some of them from drizzle and I found that typescript was still complaining. The error was about types having the same name but being unrelated. It can all be traced back to the fact that you are using different code with different resolution strategies. Drizzle just makes that so transparent that it was hard to find the reason for this bug.

To answer your question, drizzle is heavily reliant on symbols to properly identify objects at runtime. You can pass a CTE, a table, a subquery to the .from() method in a select and it will be handled properly thanks to the symbols.

ari-becker commented 6 months ago

@Angelelz I guess this is unfixable then?

Suggested workaround is probably to move all the drizzle code into @local/database and have @local/database export functions that make the actual drizzle calls, thus isolating the drizzle dependency into @local/database, i.e. the "data access layer / DAL" pattern. Sound about right?

Angelelz commented 6 months ago

@Angelelz I guess this is unfixable then?

I can't give you a verdict on this because I'm not part of the core team. But IMO there's nothing to fix. Even if we shipped the ESM code and commonjs code in different folders, if you use one in a package and another in another package, it will still be different code. You need to pick one and stick with that in your mono repo.

The workaround that you mentioned feels like it should work.

Maybe you could make sure you pull from only one type of code via the file extensions? Maybe you can try that in either package and see what happens. Say, import from drizzle-orm/index.cjs or drizzle-orm/index.js

johtso commented 3 months ago

Fixed!

It was because I was using different versions of drizzle orm inside my workspaces. So if you update all the drizzle orms to the same version it will work nicely!!

Thanks! This solved my problem. Just needed to not have drizzle-orm in my main package's dependencies, and instead rely on my database package installing it.

dkmooers commented 3 months ago

I'm having this issue with bun, and it's preventing me from updating drizzle-orm beyond 0.29.3. I have a simple repo setup, and only the one version of drizzle-orm, so there's no version mismatch. createInsertSchema type defs break and won't infer any types from the referenced table. Any other ideas on how to fix this? Would love to be able to update drizzle-orm and use batching!

EDIT: Looks like it was a bun issue. Fixed it by manually deleting drizzle-orm and drizzle-zod from node_modules and re-running bun install. (Reference: https://github.com/drizzle-team/drizzle-orm/issues/1515#issuecomment-1812760250)