mickhansen / graphql-sequelize

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

Relay createConnectionResolver orderBy depends on GraphQLEnumType #639

Closed intellix closed 5 years ago

intellix commented 5 years ago

I'm currently in the process of trying to rewrite the JS Based schema creation methods into SDL/Resolvers.

user.type.ts

import gql from 'graphql-tag';

export const types = gql`
  type Query {
    user(id: ID!): User
    users(
      after: String,
      before: String,
      first: PaginationAmount,
      last: PaginationAmount,
      order: UserOrderBy
    ): UserConnection
  }

  type User {
    id: ID!
    createdAt: SequelizeDate
    updatedAt: SequelizeDate
    email: String
    name: String
  }

  type UserConnection {
    pageInfo: PageInfo!
    edges: [UserEdge]
    total: Int
  }

  type UserEdge {
    node: User
    cursor: String!
    index: Int
  }

  enum UserOrderBy {
    ID
    BALANCE
  }
`;

query.resolver.ts

export const QueryResolver = {
  Query: {
    user: (source: any, args: any, context: any, info: any) => {
      args.id = fromGlobalId(args.id).id;
      return resolver(() => User)(source, args, context, info);
    },
    users: relay.createConnectionResolver({
      target: User,
    }).resolveConnection,
  },
};

This is all good until I need to pass in the orderBy options (UserOrderBy specifically). It's dependant on using JS to create that type like so:

new GraphQLEnumType({
  name: 'UserOrderBy',
  values: {
    ID: { value: ['id', 'ASC'] },
    BALANCE: { value: ['balance', 'ASC'] },
  },
});

https://github.com/mickhansen/graphql-sequelize/blob/b036ef39b4831f682e62edd6654fcf7b4bea92ab/src/relay.js#L207-L209

I'm guessing we can refactor this to just take the values like so:

orderBy: {
  ID: { value: ['id', 'ASC'] },
  BALANCE: { value: ['balance', 'ASC'] },
}

Any thoughts about this?

mickhansen commented 5 years ago

Is the schema converted to a set of javascript objects internally? I sort of assume it is, maybe we can figure out to pass the schema type somehow.

intellix commented 5 years ago

Not sure about that, will have a check.

Another thing I've found is that NodeTypeMapper depends on the JS Objects as well like so:

// Map Node GraphQL Types to Sequelize Models
nodeTypeMapper.mapTypes({
  [User.name]: UserType,
});

Any thoughts on how you could get around that? Maybe it could take the type name by string and if there's a registry of types we can look that up for it

mickhansen commented 5 years ago

Any thoughts on how you could get around that? Maybe it could take the type name by string and if there's a registry of types we can look that up for it

That could work.

How do interfaces work when you define your schema externally? You still have to plug in type resolvers somewhere i assume?

intellix commented 5 years ago

Researched this a little. Enum internal values are defined within resolvers like so:

export const UserResolver = {
  User: {
    level: (user: User) => user.level,
  },
  UserOrderBy: {
    ID: ['id', 'ASC'],
    XP: ['xp', 'ASC'],
    BALANCE: [sequelize.literal('(SELECT SUM("Wallet"."amount") FROM "Wallet" WHERE "Wallet"."userId" = "User"."id")'), 'ASC'],
  },
  Query: {
    user: () => ({ level: 1 }),
  },
};

Enums can be fetched by name from the schema and return the AST and their values:

info.schema.getType('UserOrderBy').getValues();

screenshot 2018-11-14 16 56 52

mickhansen commented 5 years ago

What does info.schema.getType('UserOrderBy') return? It seems ideal if createConnection could take info.schema.getType('UserOrderBy') as an argument for orderBy

intellix commented 5 years ago

It returns the GraphQLEnumType: screenshot 2018-11-14 17 43 10

The problem with providing info.schema.getType('UserOrderBy') as an argument is that there's a circular dependency between creating the schema and the schema itself.

To create a schema you need to provide both SDL and Resolvers (after creating the connections) like so:

export const schema = makeExecutableSchema({
  typeDefs: importSchema(`${__dirname}/definitions/schema.graphql`),
  resolvers: [
    AchievementResolver,
  ],
};

I think we'd have to pass in either a GraphQLEnumType or string of the Enum type and if it's a string then we fetch it from AST

mickhansen commented 5 years ago

The problem with providing info.schema.getType('UserOrderBy') as an argument is that there's a circular dependency between creating the schema and the schema itself.

Right. But i suppose you wouldn't really want to be using createConnection to create the schema if you are already doing that by hand, maybe we want to refactor the code to allow just returning the resolver.

mickhansen commented 5 years ago

To create a schema you need to provide both SDL and Resolvers (after creating the connections) like so:

Oh, you can't get the types via the importSchema though?

intellix commented 5 years ago

importSchema returns a huge SDL string with all of the types stitched together:

screenshot 2018-11-15 09 57 58

makeExecutableSchema returns the schema that allows querying the AST etc it seems:

screenshot 2018-11-15 09 59 31

mickhansen commented 5 years ago

Okay, i guess the best approach is to remove the requirement of passing orderBy and then providing an alternative like orderByArg where by the library will apply it's logic via the AST when it sees an arg that matches the orderByArg