feathers-sequelize icon indicating copy to clipboard operation
feathers-sequelize copied to clipboard

Support withSchema [with PR]

Open Artaud opened this issue 10 months ago • 5 comments

I'd like to dynamically change schema based on request params, which can be done in Sequelize by calling the model with withSchema, similarly to withScope usage which is already supported by feathers-sequelize.

Is that something you would accept in feathers-sequelize? I can propose a PR.

Artaud avatar Apr 18 '24 08:04 Artaud

At the currently supported version of sequelize, it would be just schema which is supposed to be deprecated by withSchema in sequelize 7

Artaud avatar Apr 18 '24 21:04 Artaud

I also see that you are currently getting to a new major release. I'd love to slip this addition in :)

Artaud avatar Apr 19 '24 06:04 Artaud

I'm not sure how to structure the addition though. I see that applyScope is deprecated in favor of ModelWithScope, which is being used as a model in all DB methods. If I wanted to add schema support, I either can:

  1. add a new function similar to applyScope, like applySchema
  2. alter the ModelWithScope to be loosened to ModelWithScopeAndSchema which I really don't like for its smell, or
  3. assume that support for more sequelize props can be added in the future, and make the Model function more generic such as:
ModelWithSequelizeParams(params: ServiceParams) {
  const Model = this.getModel(params)
  # apply scope if present in params
  # apply schema if present in params
  # here be space for future additions
  return Model
}

and then use this in all DB methods.

What do you think?

Artaud avatar Apr 19 '24 06:04 Artaud

I am sorry for the slow response on this! I will investigate Sequelize's withSchema options and see if we can get that added.

DaddyWarbucks avatar Jun 19 '24 20:06 DaddyWarbucks

Thanks! I can see that you've got a lot on your hands (don't we all :D) so I completely understand, no rush :) I've been already using the change i proposed in the PR in production (using patch-package) and it's been working good so far.

Artaud avatar Jun 20 '24 08:06 Artaud