kysely-org / kysely

A type-safe typescript SQL query builder
https://kysely.dev
MIT License
10.44k stars 266 forks source link

Issue while extracting init logic to separate module #175

Closed jmezzeragp closed 2 years ago

jmezzeragp commented 2 years ago

Hi. I've been checking this tool for a few days and find it amazing, thanks!!

I'm developing a serverless application that will deploy a number of different lambdas, many of which need to access kyselys functionalities. For that, I want to extract the init logic new Kysely<Database>{...} to a reusable method which can be called from the different functions that require it.

My current codebase is as follows

const createDBConnection = () => new Kysely<SilverTables>({
    dialect: new MysqlDialect({
        // Empty pool configuration. Pool is mandatory to instantiate a Kysely object.
        // We won't be relying on Kysely to execute the query, only to build it.
        pool: createPool({}),
    })
})

const db = createDBConnection();

//entry point
const handler: Handler<APIGatewayEvent, ReturnType> = async (event) => {
    const queryBuilder = createQueryBuilder('1');
    const queryResult = await executeQuery(queryBuilder);

    return queryResult;
}

const createQueryBuilder = (id: string) => db.selectFrom('table).where('id', '=', id).select(['column']);

const executeQuery = <DB, TB extends keyof DB, O>(queryBuilder: SelectQueryBuilder<DB, TB, O>): Promise<O[]> => {
    const compiledQuery = queryBuilder.compile();
    let queryString = compiledQuery.sql;
    const params = compiledQuery.parameters;

    for (const param of params)
        queryString = queryString.replace('?', String(param));

    return dataAccessAPI(queryString);
}

If if move the createDBConnection method to a separate module, I start getting the following error: Property '#private' in type 'SelectQueryBuilder' refers to a different member that cannot be accessed from within type 'SelectQueryBuilder'.. Any ideas?

Thanks in advance

igalklebanov commented 2 years ago

Are you using any framework such as sst to build your ser erless app?

jmezzeragp commented 2 years ago

SAM, but at this stage, the issue (IMHO) is not related to the app architecture

igalklebanov commented 2 years ago

If stuff starts breaking when you're moving code around, I'd suspect the build tools and frameworks as well..

We're using kysely in production, exposing a similar "gimme db instance" function from a local package to other modules in our monorepo, and we haven't had such issues. Serverless using CDK + esbuild.

kysely's unit tests use an instance created in another module, and are run against a freshly built dist folder.

What kysely version are you using? Are you using other kysely related packages?

Could you provide a repository that reproduces this issue?

koskimas commented 2 years ago

This is a little bit off topic, but regarding the createDBConnection function: you don't need to create a dummy mysql pool. You can do this instead:

const createDBConnection = () => new Kysely<SilverTables>({
    dialect: {
        createAdapter: () => new MysqlAdapter(),
        createDriver: () => new DummyDriver(),
        createIntrospector: (db: Kysely<unknown>) => new MysqlIntrospector(db),
        createQueryCompiler: () => new MysqlQueryCompiler(),
    }
})

This way your module doesn't depend on the mysql2 module at all. All of those classes can be imported from kysely.

koskimas commented 2 years ago

One thing that could cause problems like this is if you have two copies of Kysely installed and you try to use them together. For example if the module that exports createDBConnection imports a different version of Kysely than the place that uses it, you'd get an error like that.

Make sure that's not happening. If you're not able to do that otherwise, something like this should give you the answer:

import { Kysely } from 'kysely'
import { createDBConnection } from './somewhere'

console.log('Do I have two versions installed: ', (createDBConnection() instanceof Kysely) ? 'no' : 'yes!')
koskimas commented 2 years ago

@jmezzeragp Feel free to continue the discussion here even though I closed it.