Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

truncateDb does not work with objection's knexSnakeCaseMappers #83

Open
krishnagopinath opened this issue Mar 15, 2020 · 5 comments
Open

Comments

@krishnagopinath
Copy link
Contributor

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
Copy link

elhigu commented Mar 15, 2020

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
Copy link
Contributor Author

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!

@krishnagopinath
Copy link
Contributor Author

@elhigu
Copy link

elhigu commented Mar 15, 2020

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
Copy link
Contributor Author

krishnagopinath commented Mar 17, 2020

Alrighty, thanks @elhigu !

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants