Skip to content

Conversation

Secretmapper
Copy link

@Secretmapper Secretmapper commented May 2, 2016

The current customMutations and customQueries are powerful, but the types aren't exposed so it is impossible to use the generated graphQLTypes on customQueries/Mutations.

example:

const customMutations = {
  addMaterialChapter: mutationWithClientMutationId({
    name: 'addMaterialChapter',
    inputFields: {
      id: { type: new GraphQLNonNull(GraphQLID) },
      title: { type: GraphQLString },
      content: { type: GraphQLString },
    },
    outputFields: {
      changedMaterial: { type: ??? }                                // no type
    },
    mutateAndGetPayload: async ({ id, title }) => {
      // resolver
    }
  })
}

export default getSchema([Material], {hooks, customMutations})

is not possible since the changedMaterial doesn't have the necessary type, since the type will only be exposed once getSchema is called and by then it'd already be too late.

What I propose is to pass in a function for both customMutations and Queries, so that the types can be passed dynamically:

const customMutations = (types) => ({
  addMaterialChapter: mutationWithClientMutationId({
    name: 'addMaterialChapter',
    inputFields: {
      //resolve
    },
    outputFields: {
      changedMaterial: { type: types.Material }                     // type obtained from types passed
    },
    mutateAndGetPayload: async ({ id, title }) => {
      //resolve
    }
  })
})

I realize this is a breaking change, (and I'm not even sure if the above is actually impossible without my PR), so wanted to see what you think @tothandras . If you think the approach is good I'll write tests for it.

}, {
queries: customQueries,
mutations: customMutations
queries: customQueries(types),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about: _.isFunction(customQueries) ? customQueries(types) : customQueries?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it won't be a breaking change then

@Secretmapper
Copy link
Author

@tothandras Ah, that's even better, thanks!

I'm about to hit the sack but I'll submit another commit tomorrow with tests.

@tothandras
Copy link
Contributor

@Secretmapper I had a few minutes :) hope you don't mind!
tothandras@0cb244f
Can you try it out?

@Secretmapper
Copy link
Author

@tothandras Thanks!

Another thing I forgot, we also needed to expose the processId function. This way the user can write his/her own resolver using the ids generated by graffiti-mongoose. I've also added in the function checks!

@tothandras
Copy link
Contributor

Sure, if you need that, but we are about to process every field, because it seems we would need that for #112! :)
Also, take a look at my commit, we need to convert the object types to input types.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants