stvkoch / graphqlizejs

Generate data types and resolvers from models of sequelizejs
MIT License
9 stars 3 forks source link

[Feature] Add JSON support to the list of sequelize's supported types. #22

Closed chirvo closed 3 years ago

chirvo commented 3 years ago

Hi there,

First and foremost, thank you for your work. I really appreciate it.

TL;DR:

Add support to the JSON DataType either by a) adding it to the Object of types mapSequelizeToGraphql in types.js, or b) removing the absolute switch you pass to mapTypes() in schemas.js, in the body of generateInputOperators().

A deeper explanation

In some of my models I need to use JSON fields. According to the sequelize docs the JSON data type is supported.

I tried to check if my models would work with your library, so I took the code from your example folder and tried it along with my models; then got this error:

SyntaxError: Syntax Error: Expected Name, found ":". (99:3)
   97 | input _inputOperator {
   98 |             eq:
>  99 | ne:
      |   ^
  100 | gte:
  101 | gt:
  102 | lte:
    at e (/home/chirvo/sequelize2/node_modules/prettier/parser-graphql.js:1:319)

I know that's a sequelize error, so I started tracing the bug. Long story short, it took me to schemas.js, function generateInputOperators(). In it, you import mapTypes() from types.js and pass it a switch for it to return exactly a value of a existing key in an object constant mapSequelizeToGraphql. declared in the same types.js. Because of this, when you pass JSON as a datatype, the returned value is an empty string.

Now the possible solutions I propose are quite simplistic, and since I haven't dig deep enough in your code they may prove not viable, but here they are anyway: Either add the JSON datatype to the mapSequelizeToGraphql, or remove the absolute switch from the mapTypes call in generateInputOperators. I saw that the only place you call it using the switch is right there; other calls to the same function are not using the switch. Probably the later one may introduce bugs, or break any edge cases you foresaw that I didn't.

If you want me to do a PR with any of these proposed solution let me know.

Thanks!

EDIT: Formatting.

stvkoch commented 3 years ago

Hi Irving,

thank you for your message and contribution. I'm sorry for not support all sequelizejs data types.

Let me try to change it and add support to JSON fields. :pray:

My concern is how the graphql server will handle the input of schemaless data, such as your "JSON" unserialized, and also when server try resolve it.

But let me implement some tests and see what is missing

chirvo commented 3 years ago

Just to let you know how I was trying to enable JSON support in the schema, I imported graphql-type-json, then added the needed references to the schema (via the extend var from the example, and also extending the resolvers.

const GraphQLJSON = require('graphql-type-json');
const extend = `
scalar JSON
`;
const schemaGenerated = prettier.format(schema(sequelize, extend), {
  parser: "graphql",
});
const resolversGenerated = resolvers(sequelize, pubsub, (sequelize) => ({
  JSON: GraphQLJSON,
  mutation: {
  .
  .
  .

EDIT: Fix typo

stvkoch commented 3 years ago

Now you can use JSON and JSONB and then query like:

  # query over JSONB structures
  overJsonb: services(where: { meta: { path: "meta.baz.foo", where: { eq: "baz" } } }) {
    id
    meta
    countryId
    country {
      id
      name
    }
  }

remember that you can only run queries over structure if the type was JSONB

stvkoch commented 3 years ago

https://github.com/stvkoch/graphqlizejs/tree/v0.4.0

stvkoch commented 3 years ago

thank you @bigchirv it's was a great suggestion

chirvo commented 3 years ago

Amazing! Thank you! 👏🏼👏🏼👏🏼