Vincit/knex-db-manager

`truncateDb` does not work with objection's `knexSnakeCaseMappers`

Opened this issue · 5 comments

krishnagopinath commented

I am using knex-db-manager's truncateDb method to remove data after every test. I noticed that when used in conjunction with Objection's knexSnakeCaseMappers, truncateDb does not actually do anything!

I believe this is happening is because of hard-coding of 'table_name' here, while knex is returning 'tableName' - https://github.com/Vincit/knex-db-manager/blob/master/lib/MySqlDatabaseManager.js#L134

Currently, I am removing the fields returning by knexSnakeCaseMappers when I pass knex config into the dbManager's config.

const { omit } = require('lodash')
const { knexSnakeCaseMappers } = require('objection')

// This file has knexSnakeCaseMappers spread into it
// { 
//    // other config props
//    ....knexSnakeCaseMappers()
// }
const knexConfig = require('./knexfile') 

const dbManager = require('knex-db-manager').databaseManagerFactory({
	knex: omit(knexConfig, Object.keys(knexSnakeCaseMappers())),
	dbManager: {...}
})

Is there any other way of dealing with this? Thank you!

I'd be happy to brainstorm and potentially add a PR, if necessary!

elhigu commented

Yeah... I think that method that caches DB table names should disable mappers, because it does queries to databases internal tables. The same way that inside knex in migrations mappers are ignored when queries to knex internal tables are made.

As a side note, I think it is a bad idea to use those mappers. They will just increase probability that something in knex/objection/knde-db-manager etc. libraries break and doesn't really give any benefit for developer. Camel / pascal case in db column names are not so hard to manage when you get used to it and saves one from many potentially really hard to debug problems.

krishnagopinath commented

Thanks for the response, @elhigu!

should disable mappers

What do you mean by this? Something like this? https://github.com/knex/knex/blob/master/lib/migrate/Migrator.js#L38

As a side note, I think it is a bad idea to use those mappers.

I agree with this, but for my small project, it has become such a hassle to serialize data to camelCase before sending it out to the UI, which "feels" better to use.

I'll keep your opinion in the back of my head and refactor when I can, thanks again!

elhigu commented

Yep, something like that should do. I'm not sure if actually ew should add some method to knex for allowing to disable them, just for single query... Or maybe knex-db-manager could create another instance of knex using the same pool, but without those snake case wrapper configs. Though I'm not sure if that feature allows to override those configs.

krishnagopinath commented

Alrighty, thanks @elhigu !

I'll try making a PR for this over the weekend 👍