adonisjs / lucid

AdonisJS SQL ORM. Supports PostgreSQL, MySQL, MSSQL, Redshift, SQLite and many more
https://lucid.adonisjs.com/
MIT License
1.02k stars 189 forks source link

Passing custom client to database sqlite connection #1034

Open discoverlance-com opened 2 weeks ago

discoverlance-com commented 2 weeks ago

Package version

@adonisjs/lucid@20.6.0

Describe the bug

Usually, when setting up the sqlite database connection in adonis database config, you have to pass in a client: 'sqlite' | 'sqlite3' | 'better-sqlite3';

But knex should also be able to accept custom clients as seen in the example from libsql-node-sqlite3 and shown below:

import { Knex, knex } from "knex";
const Client_SQLite3 = require("knex/lib/dialects/sqlite3");

class Client_Libsql extends Client_SQLite3 {
    _driver() {
        return require("@libsql/sqlite3");
    }
}
const db = knex({
    client: Client_Libsql as any,
    connection: {
        filename: url,
    },
});

In the example in my project, I am doing a similar thing but using the package @libsql/knex-libsql package, which does the same thing as above when you check it's source code:

const Client_SQLite3 = require("knex/lib/dialects/sqlite3");

class Client_Libsql extends Client_SQLite3 {
    _driver() {
        return require("@libsql/sqlite3");
    }
}

Object.assign(Client_Libsql.prototype, {
    dialect: "sqlite3",
    driverName: "sqlite3",
});

module.exports = Client_Libsql;

This however, does not work in adonis with lucid setup when I set my client to the example above.

For instance, it fails in the patchKnex function because the client is resolved as a class as seen below when I try to console.log the client in the patchKnex function:

function patchKnex(knex, configFn) { 
     const client = knex.client;
    console.log({ client: client.config.client})
    ...
}

// this is the result from the console.log

{
  client: {
    log: Logger {
      name: 'sqlite',
      adonisLogger: [LoggerManager],
      warn: [Function: bound ],
      error: [Function: bound ],
      deprecate: [Function: bound ],
      debug: [Function: bound ]
    },
    client: [class Client_Libsql extends Client_SQLite3] // this is the client,
    connection: {
      filename: 'libsql://example.turso.io?authToken=some-token`
    },
    useNullAsDefault: true,
    migrations: { naturalSort: true, paths: [Array] },
    debug: false
  }
}

I tried to skip that by just adding a return statement at the top and it bypassed the patch function but the lucid query client also gives this error:


[ error ] dialects[this.connection.dialectName] is not a constructor
          at new QueryClient (file:///home/user/dream-guard-pay/hello-world/node_modules/@adonisjs/lucid/build/src/query_client/index.js:49:24)

Upon checking that file and console.logging the dialect name, I realised that it's showing (resolving as a class once more)


constructor(mode, connection, emitter) {
        ...
        console.log({dialectName: this.connection.dialectName})
        this.dialect = new dialects[this.connection.dialectName](this, this.connection.config);
    }

// this is the result of the console.log
{ dialectName: [class Client_Libsql extends Client_SQLite3] }

I am not able to tell or find out so far, how knex is resolved or the connections config is used by lucid to setup the connection to the database but I assume that it's what causes the issue because knex can also accept a custom client as shown in the libsql-node-sqlite3 example above and lucid uses knex for it's database connection setup.

Reproduction repo

No response

discoverlance-com commented 2 weeks ago

After doing some digging in the code base, I think it might be from here, where the dialectName is set in the connection file

// src/connection/index.ts

constructor(
    public name: string,
    public config: ConnectionConfig,
    private logger: Logger
  ) {
    ...
    this.dialectName = resolveClientNameWithAliases(this.config.client)
    ...
  }

Since'@adonisjs/lucid' defineConfig function expects the client passed for sqlite to be a string, 'sqlite' | 'sqlite3' | 'better-sqlite3', I assume that it tries to resolve the class as a string ( [class Client_Libsql extends Client_SQLite3]).

So it looks the database config is stringified before passed to the connection?

discoverlance-com commented 2 weeks ago

After doing some digging in the code base, I think it might be from here, where the dialectName is set in the connection file

// src/connection/index.ts

constructor(
    public name: string,
    public config: ConnectionConfig,
    private logger: Logger
  ) {
    ...
    this.dialectName = resolveClientNameWithAliases(this.config.client)
    ...
  }

Since'@adonisjs/lucid' defineConfig function expects the client passed for sqlite to be a string, 'sqlite' | 'sqlite3' | 'better-sqlite3', I assume that it tries to resolve the class as a string ( [class Client_Libsql extends Client_SQLite3]).

So it looks the database config is stringified before passed to the connection?

I see a solution in the following pattern:


// in src/types/database.ts
// for each connection config, we need to edit the client option to accept a knex type too... 
// I am not sure about the database clients like postgres but sqlite should be able to accept this custom config

import {Knex} from 'knex'
export type SqliteConfig = SharedConfigNode & {
  client: 'sqlite' | 'sqlite3' | 'better-sqlite3' | typeof Knex.Client
  connection: {
    filename: string
    flags?: string[]
    debug?: boolean
    mode?: any
  }
  replicas?: never
}

// in src/connection/index.ts
// update the this.dialectName to consider a custom client
export class Connection extends EventEmitter {
    ...
    constructor(name, config, logger) {
        super();
        this.name = name;
        this.config = config;
        this.logger = logger;
        this.validateConfig();

        if(
            typeof this.config.client === 'function' && 
            this.config.client.prototype instanceof knex.Client
        ) {
            const Client = this.config.client; // Get the constructor reference
            const newClient = new Client({...this.config}); // we can spread all config except client but this is simple to spread all
            this.dialectName = resolveClientNameWithAliases(newClient.dialect);
        }
        else {
            this.dialectName = resolveClientNameWithAliases(this.config.client);
        }
        ...
    }

    // Finally, to make it also work in "knex-dynamic-connection", we need to update the patchKnex function
    // index.ts (in the library)
    // update the function to also consider a client that is a function from knex

    export function patchKnex(
       knex: Knex,
       configFn: (config: Knex.Config) => Knex.ConnectionConfig
    ): void {
        const client = knex.client;
        let clientName= '';
        if(
            typeof client.config.client === 'function'
            // I also wanted to check for typeof being knex.Client but it was not working so I ignored it
        ) {
            const Client = client.config.client; // Get the constructor reference
            const newClient = new Client({...client.config});
            clientName = resolveClientNameWithAliases(newClient.dialect);
        }
        else {
            clientName = resolveClientNameWithAliases(client.config.client);
        }
    }
}

I am not sure how this affects the packages altogether but by updating those (in the build of the packages in node_modules as I was testing), it worked for me to be able to use a custom client.

futuregerald commented 1 week ago

This is great, I'd love to be able to use turso with Adonisjs. Hoping that it can be supported properly rather than having to modify a file in node_modules. Appreciate the investigation you did here.

thetutlage commented 1 week ago

I got it to work with few tweaks. @discoverlance-com thanks for digging into it and providing insights.

Let's have libSQL supported as a first-class citizen

futuregerald commented 1 week ago

I got it to work with few tweaks. @discoverlance-com thanks for digging into it and providing insights.

Let's have libSQL supported as a first-class citizen

Love this! mind sharing your code with the tweaks you made?