mickhansen / graphql-sequelize

GraphQL & Relay for MySQL & Postgres via Sequelize
MIT License
1.9k stars 172 forks source link

Support for Symbol operators #585

Closed RichAyotte closed 6 years ago

RichAyotte commented 6 years ago

String operators have been deprecated in Sequelize since version 4 and they'll be completely dropped in version 5. Would you consider making the minimum Sequelize version to 4 and accept a patch for operator Symbols?

https://github.com/sequelize/sequelize/pull/9278

mickhansen commented 6 years ago

I'd hate to drop Sequelize v3 support as i have a major production system running on it. We could definitely look at operator symbols though.

RichAyotte commented 6 years ago

Would a version check from require('sequelize').Sequelize.version in https://github.com/mickhansen/graphql-sequelize/blob/d39d0ce48ea248831480161f1aeff68f240f7b5c/src/replaceWhereOperators.js#L38 be acceptable?

RichAyotte commented 6 years ago

Something like

 if (version >= 4) {
  return replaceKeyDeep(where, {
    and: Op.and,
    or: Op.or,
    gt: Op.gt,
    gte: Op.gte,
    lt: Op.lt,
    lte: Op.lte,
    ne: Op.ne,
    between: Op.between,
    notBetween: Op.notBetween,
    in: Op.in,
    notIn: Op.notIn,
    notLike: Op.notLike,
    iLike: Op.iLike,
    notILike: Op.notILike,
    like: Op.like,
    overlap: Op.overlap,
    contains: Op.contains,
    contained: Op.contained,
    any: Op.any,
    col: Op.col
  });
}
mickhansen commented 6 years ago

Regarding replaceWhereOperators i'd generally love to see it moved to a plugin instead, i don't like throwing away the type strictness of GraphQL so i'd ideally look to remove it from core, or atleast expose it as a typed object of sorts.

RichAyotte commented 6 years ago

Are you suggesting a new type like whereType that would replace JSONType?

mickhansen commented 6 years ago

Something like that, i'd like to make the entire where functionality pluggable, and then leave it to somebody else to device if they want types or not.

mickhansen commented 6 years ago

Happy to merge a PR adding operator support though, won't remove where untill there's a strategy in place and a major is done.