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

feat(db-postgres, db-sqlite): drizzle schema generation #9953

Merged
merged 2 commits into from
Dec 19, 2024

Conversation

r1tsuu
Copy link
Member

@r1tsuu r1tsuu commented Dec 13, 2024

This PR allows to have full type safety on payload.drizzle with a single command

pnpm payload generate:db-schema

Which generates TypeScript code with Drizzle declarations based on the current database schema.

Example of generated file with the website template:
https://gist.github.com/r1tsuu/b8687f211b51d9a3a7e78ba41e8fbf03

Video that shows the power:

Screen.Recording.2024-12-13.at.08.44.52.mov

We also now proxy drizzle package the same way we do for Lexical so you don't have to install it (and you shouldn't because you may have version mismatch).
Instead, you can import from Drizzle like this:

import {
  pgTable,
  index,
  foreignKey,
  integer,
  text,
  varchar,
  jsonb,
  boolean,
  numeric,
  serial,
  timestamp,
  uniqueIndex,
  pgEnum,
} from '@payloadcms/db-postgres/drizzle/pg-core'
import { sql } from '@payloadcms/db-postgres/drizzle'
import { relations } from '@payloadcms/db-postgres/drizzle/relations'

Fixes #4318

In the future we can also support types generation for mongoose / raw mongodb results.

@r1tsuu r1tsuu changed the title feat(db-postgres): drizzle schema generation feat(db-postgres, db-sqlite): drizzle schema generation Dec 13, 2024
@r1tsuu r1tsuu requested review from DanRibbens and jmikrut December 13, 2024 09:18
@valerioleo
Copy link

Soooo happy for this! Great work team.

@r1tsuu r1tsuu force-pushed the feat/drizzle-types-gen branch from 6eb83ea to 9ed9bf2 Compare December 13, 2024 16:39
@willviles
Copy link
Contributor

willviles commented Dec 13, 2024

This is awesome @r1tsuu. I love that this PR is close. QQs:

  1. Does this generate duplicate schemas for custom tables & relations passed in the beforeSchemaInit hook?
  2. If the Payload schema is extended in afterSchemaInit, will the generated schema include these amends?

@willviles
Copy link
Contributor

willviles commented Dec 13, 2024

Also - one big thing I've noticed from using Drizzle introspection is that the output doesn't work well when entities have either self or circular relationships.

For example, tables with circular foreign key references such as the one below create an error on foreignKey definitions, resulting in an any type attributed to each table.

// Function implicitly has return type 'any' because it does not have a return type annotation and is referenced directly or indirectly in one of its return expressions.
export const mediaInPayload = payload.table("media", {
    ownerId: varchar("owner_id"),
}, (table) => {
    return {
        mediaOwnerIdUsersIdFk: foreignKey({
            columns: [table.ownerId],
            foreignColumns: [usersInPayload.id],
            name: "media_owner_id_users_id_fk"
        }).onDelete("set null"),
    }
});

// Function implicitly has return type 'any' because it does not have a return type annotation and is referenced directly or indirectly in one of its return expressions.
export const usersInPayload = payload.table("users", {
    avatarId: varchar("avatar_id"),
}, (table) => {
    return {
        usersAvatarIdMediaIdFk: foreignKey({
            columns: [table.avatarId],
            foreignColumns: [mediaInPayload.id],
            name: "users_avatar_id_media_id_fk"
        }).onDelete("set null"),
    }
});

Issue refs can be found in this Discord thread & these docs.

Is this an issue that may pose problems?

@r1tsuu r1tsuu requested a review from denolfe as a code owner December 14, 2024 00:13
@r1tsuu r1tsuu force-pushed the feat/drizzle-types-gen branch 2 times, most recently from 09bb841 to 11483a5 Compare December 15, 2024 08:00
@r1tsuu r1tsuu force-pushed the feat/drizzle-types-gen branch 3 times, most recently from 473ad46 to 0f55ee8 Compare December 16, 2024 06:05
test/jest.setup.js Outdated Show resolved Hide resolved
r1tsuu added a commit that referenced this pull request Dec 16, 2024
### What?
Abstracts SQL schema building, significantly reducing code duplication
for SQLite / Postgres

db-sqlite lines count From:
```sh
 wc -l **/*.ts
      62 src/connect.ts
      32 src/countDistinct.ts
       9 src/createJSONQuery/convertPathToJSONTraversal.ts
      86 src/createJSONQuery/index.ts
      15 src/defaultSnapshot.ts
       6 src/deleteWhere.ts
      21 src/dropDatabase.ts
      15 src/execute.ts
     178 src/index.ts
     139 src/init.ts
      19 src/insert.ts
      19 src/requireDrizzleKit.ts
     544 src/schema/build.ts
      27 src/schema/createIndex.ts
      38 src/schema/getIDColumn.ts
      13 src/schema/idToUUID.ts
      28 src/schema/setColumnID.ts
     787 src/schema/traverseFields.ts
      18 src/schema/withDefault.ts
     248 src/types.ts
    2304 total
```

To:
```sh
wc -l **/*.ts
      62 src/connect.ts
      32 src/countDistinct.ts
       9 src/createJSONQuery/convertPathToJSONTraversal.ts
      86 src/createJSONQuery/index.ts
      15 src/defaultSnapshot.ts
       6 src/deleteWhere.ts
      21 src/dropDatabase.ts
      15 src/execute.ts
     180 src/index.ts
      39 src/init.ts
      19 src/insert.ts
      19 src/requireDrizzleKit.ts
     149 src/schema/buildDrizzleTable.ts
      32 src/schema/setColumnID.ts
     258 src/types.ts
     942 total
```

Builds abstract schema in shared drizzle package that later gets
converted by SQLite/ Postgres specific implementation into drizzle
schema.
This, apparently can also help here
#9953. It should be very
trivial to implement a MySQL adapter with this as well.
@r1tsuu r1tsuu force-pushed the feat/drizzle-types-gen branch from 2ce70a3 to dba8c96 Compare December 16, 2024 15:19
@magicspon
Copy link

Well this give us access to the eq function from drizzle?

@r1tsuu r1tsuu force-pushed the feat/drizzle-types-gen branch from e0273a5 to b8915be Compare December 17, 2024 02:13
Copy link
Contributor

@DanRibbens DanRibbens left a comment

Choose a reason for hiding this comment

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

Looks good! You will need to add this feature to the docs. Otherwise, I'm ready to approve.

…h of follwing:

@r1tsuu
custom code gen for drizzle schema

@r1tsuu
codegen with custom pg schema

8e91f92
@r1tsuu
jsonb numeric default values serialize

e79be1e
@r1tsuu
sqlite column to code converter

025e3ff
@r1tsuu
add prettify and log args
13c0033
@r1tsuu
add do not modify by hand

b1b4993
@r1tsuu
beautify property access output

a7f8738
@r1tsuu
fix uuid default random SQLite

fix columnToCodeConverter regression

fix: escape default values properly in generated code

revert jest setup
@r1tsuu r1tsuu force-pushed the feat/drizzle-types-gen branch 4 times, most recently from 2fceb0f to 65e1988 Compare December 19, 2024 13:03
@r1tsuu
Copy link
Member Author

r1tsuu commented Dec 19, 2024

@DanRibbens

  • Wrote docs.
  • Wrote an integration test that ensures the generated file is compilable and exports every table/relation/enum. Improved pattern for conditionals in tests.
  • Few additional fixes.

@willviles
Sorry for the delay, I'm ready to answer your questions. We don't use Drizzle's introspection anymore here and this PR does all the schema codegen logic by itself.
Why? I noticed that what Drizzle generates through the introspection is very different from what we have in payload.db.drizzle. For example, export table names, names of properties for columns, different relations that don't actually exist in payload.db.drizzle (or have different names under payloa.db.drizzle).
The new method generates 100% the same code of the schema that Payload actually uses for its migrations / schema push, therefore, now you can even swap payload migrator to Drizzle directly, though it's not recommended.

Does this generate duplicate schemas for custom tables & relations passed in the beforeSchemaInit hook?

If you added them by changing schema - no. However, there's a different way now through mutating the "raw" Payload SQL schema in the beforeSchemaInit hook:
https://github.com/payloadcms/payload/pull/9953/files?plain=0#diff-583b55d282ab9145378592b5476dc40bfbbe46b3e0223b388886dee4fbfc518bR251-R290

You can mutate as well adapter.rawRelations to add your relations. Note that in beforeSchemaInit the Payload SQL schema exists, while Drizzle's doesn't (as before).

If the Payload schema is extended in afterSchemaInit, will the generated schema include these amends?
See above the note.

Also - one big thing I've noticed from using Drizzle introspection is that the output doesn't work well when entities have either self or circular relationships.
For example, tables with circular foreign key references such as the one below create an error on foreignKey definitions, resulting in an any type attributed to each table.

We don't use introspection anymore as I said, I included the workaround with : AnyPgColumn from https://orm.drizzle.team/docs/indexes-constraints#foreign-key with references to itself in to the generated output:
https://github.com/payloadcms/payload/pull/9953/files#diff-2583a68a595e56a58529bfea4c91c0c5bf168962e01b14fc51699927b5038dbdR78-R91
See example:
https://github.com/payloadcms/payload/pull/9953/files#diff-4d1376127d269fc05a0ece3ea2a7d5c56701311274ce5a0163ff45d25c38ef4bR204-R210
The generated table here is not any here.

@magicspon

Well this give us access to the eq function from drizzle?

Yes! You can import it from the adapter package directly:

import { eq, sql, and } from '@payloadcms/db-postgres/drizzle'

…ll refactor later other tests 3. re-export geoemtryColumn from payloadcms/db-postgres directly 4. fix SQLite numeric default typescript issue 5. fix: include : AnyPgColumn to generated references if to itself see https://orm.drizzle.team/docs/indexes-constraints#foreign-key
@r1tsuu r1tsuu force-pushed the feat/drizzle-types-gen branch from 9ca4da6 to 8f3221f Compare December 19, 2024 13:31
@DanRibbens DanRibbens merged commit 23f1ed4 into main Dec 19, 2024
66 checks passed
@DanRibbens DanRibbens deleted the feat/drizzle-types-gen branch December 19, 2024 16:08
Copy link
Contributor

🚀 This is included in version v3.10.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants