t3-oss / create-t3-turbo

Clean and simple starter repo using the T3 Stack along with Expo React Native
https://turbo.t3.gg
MIT License
4.28k stars 352 forks source link

use drizzle-zod #989

Closed juliusmarminge closed 1 month ago

juliusmarminge commented 2 months ago

Fixes #988

x-ref: https://github.com/t3-oss/create-t3-turbo/discussions/966

john093e commented 2 months ago

Hello @juliusmarminge,

I've tried to follow all your changes, and it seems to have resolved my issue with fetching schema data inside a client component. Awesome!

Additionally, you introduced the use of drizzle-zod and demonstrated its application in the post form. On my side, this has created a new issue. For the useForm:

const form = useForm({
  schema: createPostSchema, // Using it from the db package as in your PR
  defaultValues: {
    content: "",
    title: "",
  },
})

The fields now include a null type addition (which wasn't present before when using CreatePostSchema from @acme/validators):

title?: string | null | undefined;
content?: string | null | undefined;

This change is causing an error in VSCode for the input fields, as null is not allowed for the Input field component.

Note: Is there a specific reason you removed the "schema" folder? On my side, I didn't move any files from there; I simply added schema.ts at the root of src/ (as in your PR) and exported all the files from there like so: src/schema.ts:

export * from "./schema/auth"
export * from "./schema/post"
export { mySqlTable } from "./schema/_table"

This setup seems to work well and maintains ease of reading and updating, as a single file table can quickly become overly lengthy.

juliusmarminge commented 2 months ago

Note: Is there a specific reason you removed the "schema" folder? On my side, I didn't move any files from there; I simply added schema.ts at the root of src/ (as in your PR) and exported all the files from there like so:

No reason, just preference. Splitting up in multiple files may be beneficial as your schema grows but it's a bit premature to do so when there's only 4 tables

john093e commented 2 months ago

Hello, I'm not sure if this is the best place to post this, but since we are updating the Drizzle setup in this PR, I think we should also adapt it to the latest update of AuthJs.

1. Enable foreign key constraints on PlanetScale: Settings: check the box for foreign key constraints.

2. Adapt the Drizzle Schema:

export const users = mySqlTable("user", {
  ...
    email: text("email").notNull().unique(), //<-- Added 'unique'
  emailVerified: timestamp("emailVerified", { mode: "date", fsp: 3 }), // <-- Remove the default value
  ...
});

export const accounts = mySqlTable("account", {
  ...
  access_token: varchar("access_token", { length: 255 }), // <-- Changed to varchar(255)
  ...
  id_token: varchar("id_token", { length: 2048 }), // <-- Changed to varchar(2048)
  ...
});

export const sessions = mySqlTable("session", {
  id: varchar("id", { length: 26 }).$defaultFn(() => ulid()).primaryKey(), // <-- Add an 'id' column (choose whichever method suits you for ID generation)
  sessionToken: varchar("sessionToken", { length: 255 }).notNull().unique(), // <-- Change 'primaryKey' to 'unique'
  userId: varchar("userId", { length: 255 }).notNull().references(() => users.id, { onDelete: "cascade" }), // Add the reference constraints
  ...
});

3. Adapt the authentication configuration in "packages/auth/src/config.ts":

import {
  // mySqlTable,
  accounts,
  sessions,
  users,
  verificationTokens,
} from "@acme/db/schema" // <-- Remove the import of mySqlTable and directly import the four necessary schemas

...
export const authConfig = {
  adapter: DrizzleAdapter(createDBClient(), {
    usersTable: users,
    accountsTable: accounts,
    sessionsTable: sessions,
    verificationTokensTable: verificationTokens,
  }), // <-- Replace 'mySqlTable' with the specific tables
  ....
};
john093e commented 2 months ago

I might have done something wrong; I think the Drizzle adapter isn't compatible with Edge. :'( "issue with crypto"

Azzerty23 commented 2 months ago

I might have done something wrong; I think the Drizzle adapter isn't compatible with Edge. :'( "issue with crypto"

I'm not sure about your context, but when I read "Edge" and "crypto," I remembered this brilliant blog post: https://zenstack.dev/blog/adapt-to-edge#node-apis. Maybe it's something worth looking into.

TLDR: "crypto" is provided as a built-in on Edge, so there's no need to import the package in this environment.

dBianchii commented 2 months ago

I might have done something wrong; I think the Drizzle adapter isn't compatible with Edge. :'( "issue with crypto"

I'm not sure about your context, but when I read "Edge" and "crypto," I remembered this brilliant blog post: https://zenstack.dev/blog/adapt-to-edge#node-apis. Maybe it's something worth looking into.

TLDR: "crypto" is provided as a built-in on Edge, so there's no need to import the package in this environment.

Maybe, don't use Edge? Lately even Vercel is acknowledging we shouldn't place stuff like db calls in edge-location or even edge-runtime

andrewdoro commented 2 months ago

I might have done something wrong; I think the Drizzle adapter isn't compatible with Edge. :'( "issue with crypto"

I'm not sure about your context, but when I read "Edge" and "crypto," I remembered this brilliant blog post: https://zenstack.dev/blog/adapt-to-edge#node-apis. Maybe it's something worth looking into. TLDR: "crypto" is provided as a built-in on Edge, so there's no need to import the package in this environment.

Maybe, don't use Edge? Lately even Vercel is acknowledging we shouldn't place stuff like db calls in edge-location or even edge-runtime

lol drizzle should work on edge (that's one of their selling points). If you are doing any auth check in the middleware then you are automatically using the edge runtime.

juliusmarminge commented 2 months ago

There were a bunch of bugs/breaking stuff in drizzle adapter 1.0 so maybe if you used that then you'd have some issues.

1.0.1 should be out soon with fixes

john093e commented 2 months ago

I am just confirming that there are no more edge runtime issues with the latest drizzle-adapter for AuthJS. 🥳