Architectural design suggestion

[rluvaton]

Hey @eveningkid, I want to contribute to this project, and when I read the code I noticed that you specify manually the DB types this library support https://github.com/eveningkid/denodb/blob/10d6447ac23a6b936ffbcfa5e0bc8a39e1c1f899/lib/database.ts#L56-L60

Then I though that it should have plugin architecture in a way that you should write something like:

// PostgresConnector and PostgresTranslator are a constructor
// PostgresPlugin will have {connector: PostgresConnector, translator: PostgresTranslator}
Database.register(PostgresPlugin)

Advantages

• Extract the implementation
• Reduce bundle size:
  • Only import DB connector that in use
• In case of a change in one connector no need to redeploy
• Solve the issue when some DB API change between DB versions

[eveningkid]

Hi Raz,

First of all, thank you for taking the time of reading the code.

Regarding your suggestion, I think you are very right. Now about the API itself, I would still like to keep the simplicity of what we have at the moment so here is my suggestion.

Just a reminder of what we have at the moment:

const db = new Database('postgres', {
  database: 'my-database',
  host: 'url-to-db.com',
  username: 'username',
  password: 'password',
  port: 5432, // optional
});

And how I would see this using your plugin approach:

import PostgreSQLConnector from './deps.ts';

// Or we could even drop the `new` from the plugin
// and `connector` could also be `connection`/`plugin`
const connection = new PostgreSQLConnector({
  database: 'my-database',
  host: 'url-to-db.com',
  username: 'username',
  password: 'password',
  port: 5432, // optional
});

const db = new Database(connection);
// or
const db = Database.use(connection);
// I just feel that with `use` people would expect this to be chainable such as `Database.use(A).use(B)`
// although in our case we would only have connectors

Let me know how you feel about this, it is obviously an open discussion :)

Keep in touch

[rluvaton]

Now about the API itself, I would still like to keep the simplicity of what we have at the moment so here is my suggestion.

Agree.

// Or we could even drop the `new` from the plugin
// and `connector` could also be `connection`/`plugin`
const connection = new PostgreSQLConnector({
  database: 'my-database',
  host: 'url-to-db.com',
  username: 'username',
  password: 'password',
  port: 5432, // optional
});

const db = new Database(connection);

Will PostgreSQLConnector have the translator and if so, will the connector initialize it? and we need to find a better name cause connector is one side of the communication with the DB, there is also the translator. I think about it as more of a constant export. this is why I thought about register

I just feel that with use people would expect this to be chainable such as Database.use(A).use(B) although in our case we would only have connectors

I think you are right

[eveningkid]

Will PostgreSQLConnector have the translator and if so, will the connector initialize it? and we need to find a better name cause connector is one side of the communication with the DB, there is also the translator. I think about it as more of a constant export. this is why I thought about register

I think these details are about the implementation, not what the user will think about. What I mean is that, if this comes with a translator and others, this is not something the user cares about.

This is what could happen under-the-hood:

class PostgreSQLConnector extends Connector {
  constructor(config) {
    this.translator = new PostgreSQLTranslator(); // or SQLTranslator() I forgot
    this.config = config;
  }
}

As a user, you just want to make a connection to your database as fast as possible so whatever happens in the background shouldn't obviously transpire in the API itself.

If I think about what's the simplest way of writing this (from our experience using ORMs), you just want to create a database and pass a configuration to it. No need to register anything else, even if we as developers see this as a plugin, right?

I don't know if connector makes sense API-wise but from a user perspective this is what they're doing. Hope this makes sense!

[rluvaton]

I think these details are about the implementation, not what the user will think about. What I mean is that, if this comes with a translator and others, this is not something the user cares about.

Yeah, I was talking about the implementation cause I was thinking about how to implement it.

This is what could happen under-the-hood:

class PostgreSQLConnector extends Connector {
  constructor(config) {
    this.translator = new PostgreSQLTranslator(); // or SQLTranslator() I forgot
    this.config = config;
  }
}

That looks fine 👍 , do you want me to start to implement it or there are things we forget to talk about?

If I think about what's the simplest way of writing this (from our experience using ORMs), you just want to create a database and pass a configuration to it. No need to register anything else, even if we as developers see this as a plugin, right?

👍

[eveningkid]

I think we're good. If there is anything you want to talk about, feel free to use this issue or the PR you'll create :)

This might be quite a huge PR btw so try to keep it as simple as possible. I am sure you will do great!

You really have space to do what you feel is right so give it a try!