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

Passing context to plugins on a query by query basis #627

Open
jordanjoyce opened this issue Aug 2, 2023 · 7 comments
Open

Passing context to plugins on a query by query basis #627

jordanjoyce opened this issue Aug 2, 2023 · 7 comments
Labels
api Related to library's API built-in plugin Related to a built-in plugin custom plugin Related to a custom plugin question Further information is requested

Comments

@jordanjoyce
Copy link

Have a use case where I need to stop certain columns from transforming to snake case. I've forked the CamelCasePlugin and got the functionality working however to get this to work on a query by query basis I currently need to use the following pattern

qb.withoutPlugins().withPlugin(new CamelCasePlugin({}, noTransformCols]))

I've recently migrated from Knex and they had a queryContext function that I used for similar functionality. Wondering if there's an easier way to do this that I've missed or not.

Thanks

@igalklebanov igalklebanov added question Further information is requested built-in plugin Related to a built-in plugin custom plugin Related to a custom plugin api Related to library's API labels Aug 2, 2023
@igalklebanov
Copy link
Member

Hey 👋

To get this to work per-query, all you have to do is use .withPlugin(new CamelCasePlugin({}, noTransformCols)). I don't understand, from your post, why you'd need .withoutPlugins().

@igalklebanov igalklebanov added the needs more info An issue that lacks detail label Aug 6, 2023
@jordanjoyce
Copy link
Author

@igalklebanov Thanks for the quick response, should have mentioned the CamelCasePlugin is used globally, so I have to use .withoutPlugins() to stop it running twice. This pattern is fine as it's only used in a few queries. Was just reaching out to make sure I wasn't missing a more obvious solution!

@igalklebanov igalklebanov removed the needs more info An issue that lacks detail label Aug 7, 2023
@igalklebanov
Copy link
Member

igalklebanov commented Aug 7, 2023

@jordanjoyce I see. Given your usage, there is no better way right now.

@koskimas a few ideas...

maybe we could offer some kind of state that can be global and local (overriding globals) that all plugins and drivers can consume? this would remove the forced usage of .withoutPlugins (that can be risky - removing more than you need) or .withPlugin (no need to instantiate a new plugin instance per invocation) everywhere.

const db = new Kysely<DB>({
  // ...
  plugins: [new MyCamelCasePlugin()],
  vars: {
    // some global vars.
  }
})

db
  .$withVars({ noCamelCase: ['column_0', 'column_1'] })
  .selectFrom('table').select(['column_0', 'column_1', 'column_2'])
  .execute()

This can also facilitate prepared statements (passing names as vars and driver consuming these to prepare if not prepared with that name yet).

plugin dedup/override by some name.

@koskimas
Copy link
Member

koskimas commented Sep 8, 2023

Don't know about this. I added something like this to objection and it opened a door for all kinds of hacks people shouldn't have done. I'm afraid adding something like this will make people add ORM features as plugins which just leads to a really really shitty ORM and a LOT of issues and feature requests for us to solve.

This is not optimal but you could do something like this

export db = new Kysely<DB>({
  dialect,
  plugins: PLUGINS
})
export const PLUGINS = [
  new CamelCasePlugin(camelCaseConfig),
  ...OTHER_PLUGINS
]

export function noCamelCaseFor(columns: string[]) {
  return <T extends SelectQueryBuilder<any, any, any>>(qb: T) {
   let qb = qb
     .withoutPlugins()
     .withPlugin(new CamelCasePlugin(camelCaseConfig), columns)
    
    for (const p of OTHER_PLUGINS) {
      qb = qb.withPlugin(p)
    }

    return qb
  }
}

And usage:

db.selectFrom('person')
  .select('first_name')
  .$call(noCamelCaseFor(['first_name']))

@geetesh911
Copy link

geetesh911 commented Jan 29, 2024

Don't know about this. I added something like this to objection and it opened a door for all kinds of hacks people shouldn't have done. I'm afraid adding something like this will make people add ORM features as plugins which just leads to a really really shitty ORM and a LOT of issues and feature requests for us to solve.

This is not optimal but you could do something like this

export db = new Kysely<DB>({
  dialect,
  plugins: PLUGINS
})
export const PLUGINS = [
  new CamelCasePlugin(camelCaseConfig),
  ...OTHER_PLUGINS
]

export function noCamelCaseFor(columns: string[]) {
  return <T extends SelectQueryBuilder<any, any, any>>(qb: T) {
   let qb = qb
     .withoutPlugins()
     .withPlugin(new CamelCasePlugin(camelCaseConfig), columns)
    
    for (const p of OTHER_PLUGINS) {
      qb = qb.withPlugin(p)
    }

    return qb
  }
}

And usage:

db.selectFrom('person')
  .select('first_name')
  .$call(noCamelCaseFor(['first_name']))

qb does not have withoutPlugins method. Can you suggest any other way?

@ethanresnick
Copy link
Contributor

I also have a plugin that I’d like to be able to opt-out of for individual queries. If something like $withVars is a bad idea, maybe two new query builder methods like this could be convenient?

// A new overload for withoutPlugins that returns an 
// identical query builder with just these plugins removed
// (where each plugin is identified by === equality)
withoutPlugins(plugins: readonly KyselyPlugin[]): 

// returns a qb that retains only those plugins which pass the predicate.
// Useful when the consumer doesn’t have a reference to
// the exact plugin object (including in certain mocking/testing scenarios)
filterPlugins(predicate: (it: KyselyPlugin) => boolean): 

I’m not sure if those methods are worth it, but their intention seems slightly clearer, and they could be implemented more efficiently than the version shown above where the plugins have to be added back one-by-one, creating a new qb each time.

@bakasmarius
Copy link

qb does not have withoutPlugins method. Can you suggest any other way?

While writing a test for #1281 I learned that even though qb does not have withoutPlugins method, but db itself has one, maybe that could help you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Related to library's API built-in plugin Related to a built-in plugin custom plugin Related to a custom plugin question Further information is requested
Projects
None yet
Development

No branches or pull requests

6 participants