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

Better Documentation #180

Open
PeytonHanel opened this issue Jul 18, 2024 · 5 comments
Open

Better Documentation #180

PeytonHanel opened this issue Jul 18, 2024 · 5 comments

Comments

@PeytonHanel
Copy link

I just started using the Planetscale serverless driver for JavaScript, but I'm having a hard time understanding it. The documentation explains the very basics but doesn't go into detail on any of the return objects to the provided functions, why you would want to use any of the things provided over the other, nor any caveats I should/shouldn't know. All of the provided objects/classes/types in javascript lack JsDocs.

Having detailed documentation is useful so developers can quickly understand how to use the library without having to tinker and try things out. I've provided some examples below that could be improved on, but it's not a comprehensive list.

The docs say that we can connect to our DB with connect and Client, but they don't explain why I would want to use one over the other apart from this line

Use the Client connection factory class to create fresh connections for each transaction or web request handler.

The Client class has a execute function but also has another function called connection which returns a Connection object, which also has an execute function. Why are there two? Are they actually the same? When do I use one over the other?

There's also no mention of pooling, which I'm not even sure matters for serverless, but I feel like it should be addressed so I'm not overloading my DB with connections.

I'm also assuming that parameterized SQL calls won't cause me to have a SQL injection. You have an article here but it doesn't address this library at all.

Etc.

Thank you

@ayrton
Copy link
Contributor

ayrton commented Jul 19, 2024

Thank you for your feedback.

The docs say that we can connect to our DB with connect and Client, but they don't explain why I would want to use one over the other apart from this line

I agree that the usage between connect and Client is confusing. Let me try to provide some insights, and hopefully some answers.

Using a global connection can lead to data inconsistencies, because a connection is not really meant to be shared across requests. In practice this means you'll almost certainly want to use Client over the current connect.

We have talked internally about updating the connect function to return a Client instance.

-export function connect(config: Config): Connection {
-  return new Connection(config)
+export function connect(config: Config): Client {
+  return new Client(config)
}

But we hadn't yet because of backwards compatibility. Often people use database-js through an ORM eg. Drizzle, Prisma. Older versions of Drizzle (<0.29.4) (and Prisma) did not accept Client instances. This has been patched since and both now enforce the usage of a Client instance (ref).

It's been almost half a year since Drizzle started to accept Client instances - we can start considering changing the defaults now. If people want a single global connection they'd still be able to instantiate one directly.

The Client class has a execute function but also has another function called connection which returns a Connection object, which also has an execute function. Why are there two? Are they actually the same? When do I use one over the other?

They are not the same. By design Client#execute instantiates a new connection for every query you run. Use Client when you don't want to share any connections, use Connection when you do.

@mattrobenolt did a deep-dive into the nuances, I highly recommend reading it if you want to get a deeper understanding. drizzle-team/drizzle-orm#1743 (comment).

There's also no mention of pooling, which I'm not even sure matters for serverless, but I feel like it should be addressed so I'm not overloading my DB with connections.

Database-js is made for being used within serverless environments, you don't have to worry about managing your database connections.

Connection instances are often mistaken for database connections, maybe there's a better name for them. Quoting @mattrobenolt

Connection instances are used to track state, they are more synonymous with a "session". One underlying TCP connection is reused, but for multiplexing over that single TCP connection, the session is required to track state.

@PeytonHanel
Copy link
Author

This is super helpful. Thank you so much.

Can you speak at all to the parameterized SQL calls and how you prevent SQL injections?

Do you think you will add an API guide to the documentation? I have no plans to use an ORM and fully intend on writing all my own SQL queries and keeping them in my codebase.

Thank you.

@ayrton
Copy link
Contributor

ayrton commented Jul 19, 2024

Can you speak at all to the parameterized SQL calls and how you prevent SQL injections?

You can either use the built-in escaping, or use an external library, like sqlstring. String interpolation would not be safe. (Ref)

Do you think you will add an API guide to the documentation?

What are you looking for exactly? The API surface is pretty small and should be fully covered with the README. We do welcome any contributions to make things better.

I have no plans to use an ORM and fully intend on writing all my own SQL queries and keeping them in my codebase.

This is perfectly reasonable.

@PeytonHanel
Copy link
Author

So just to be clear, doing this is safe against SQL injections:

const results1 = await conn.execute('select 1 from dual where 1=?', [42])

@mattrobenolt
Copy link
Member

Correct.

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

No branches or pull requests

3 participants