planetscale / database-js

A Fetch API-compatible PlanetScale database driver
https://planetscale.com/docs/tutorials/planetscale-serverless-driver
Apache License 2.0
1.16k stars 34 forks source link

Better Documentation #180

Open PeytonHanel opened 1 month ago

PeytonHanel commented 1 month ago

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 commented 1 month ago

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. https://github.com/drizzle-team/drizzle-orm/issues/1743#issuecomment-1879479647.

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

Correct.