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

[Feature] A type can be defined for rows in the query result #150

Closed
yutak23 opened this issue Nov 28, 2023 · 6 comments · Fixed by #154
Closed

[Feature] A type can be defined for rows in the query result #150

yutak23 opened this issue Nov 28, 2023 · 6 comments · Fixed by #154

Comments

@yutak23
Copy link
Contributor

yutak23 commented Nov 28, 2023

With the current type definition, the result of the following query got.rows[0] would be any.

const got = await connection.execute('SELECT 1, null from dual;');
const row = got.rows[0];
// typeof row is any

Therefore, we do not get the benefit of this when developing in TypeScript (properties are not suggested, accessing a property that does not exist, and it becomes undefined).

I think it would be better if we could define a type for row as follows

image

@mattrobenolt
Copy link
Member

Excuse my ignorance, but I'm not personally very familiar with how TypeScript works.

Isn't this more the job of something like Drizzle or Prisma or some other ORM to add this strong type checking on top of this?

Or is this just generics and making ExecutedQuery<T> rather than like ExecutedQuery<any> which is basically what we're doing today. And then each Row is like, Row<T>?

@mattrobenolt
Copy link
Member

Also, I see you put this together: https://github.com/yutak23/serverless-mysql-http 👀

You'd probably be interested in trying out https://github.com/mattrobenolt/ps-http-sim which is ultimately something we'd likely produce officially and will be much closer to behaving how PlanetScale behaves. :)

@yutak23
Copy link
Contributor Author

yutak23 commented Nov 28, 2023

Isn't this more the job of something like Drizzle or Prisma or some other ORM to add this strong type checking on top of this?

In this case, it would be difficult to use ORM. Because it is only by using the ORM model to execute SQL that the true value of the ORM is demonstrated.

As long as SQL is executed in @planetscale/database, I don't think ORM can be used.

Or is this just generics and making ExecutedQuery<T> rather than like ExecutedQuery<any> which is basically what we're doing today. And then each Row is like, Row<T>?

I have prepared a PR and would appreciate your review.

@mattrobenolt
Copy link
Member

As long as SQL is executed in @planetscale/database, I don't think ORM can be used.

I think this is slightly unrelated at this point, but this is what Prisma and Drizzle do. Drizzle currently works with this driver explicitly, so I'm not sure how that works, but it has been the answer for mapping rows to your actual schema.

I have prepared a PR and would appreciate your review.

I will try and get this in front of folks that know TypeScript for you. :)

@yutak23
Copy link
Contributor Author

yutak23 commented Nov 28, 2023

I think this is slightly unrelated at this point, but this is what Prisma and Drizzle do. Drizzle currently works with this driver explicitly, so I'm not sure how that works, but it has been the answer for mapping rows to your actual schema.

Excuse my ignorance,

I checked the official site and it looks like Drizzle is available, although I haven't tried it. I would like to try Drizzle first. Thank you very much.

https://orm.drizzle.team/docs/quick-mysql/planetscale

@mattrobenolt
Copy link
Member

Overall,I think our change here you're proposing is good. I'm not sure how this will affect API compatibility with things like Drizzle that are likely depending on the current type signatures? Again, not sure how this ecosystem works too much.

I'm not sure if a major version bump is required just because of an TypeScript signature changing or not.

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