Add missing instances (Decode, Encode)

[theobat]

No description provided.

[nalchevanidze]

I think it should fail.

[theobat]

right, it's more coherent with the other behaviors (i.e. non empty)

[nalchevanidze]

or directive @Uniq will validate duplicates. and with assumption that there are no dups, decode will dedupe silently.
what do you think?

[nalchevanidze]

with this way Graphql client can't distinguish difference between set, non-empty and regular list.

that why may it will be better approach to add them directives? for example:

 data MyInput = MyInput {  
    list1 :: NonEmpty Int,
    list2 :: Set Int,
    list3 :: [int] 
 }

that can be represented as:

directive @NonEmty () on FIELD_DEFINITION | INPUT_FIELD_DEFINITION
directive @Uniq () on FIELD_DEFINITION | INPUT_FIELD_DEFINITION


input MyInput {  
    list1: [Int!]! @NonEmpty
    list2: [Int!]! @Uniq
    list3: [int!]! 
 }

[nalchevanidze]

you can define custom directives at morpheus-graphql-core/src/Data/Morpheus/Schema/Directives.hs

to assign dirctives to fields see:

• src/Data/Morpheus/Server/Deriving/Introspect.hs.
• src/Data/Morpheus/Server/TH/Declare/Introspect.hs

[theobat]

with this way Graphql client can't distinguish difference between set, non-empty and regular list.
that why may it will be better approach to add them directives? for example:

Yeah that's a really good idea ! I was annoyed by the missing differences on the client too !

[nalchevanidze]

no. introspection does not render directives. so. we can't assign them directives. :(

[nalchevanidze]

only way would be:

data MyInput = MyInput {  
    list1 :: NonEmpty Int,
    list2 :: Set Int,
    list3 :: [int] 
 }

to wrapp and generate custom types for them:

input InputNonEmpty_Int {
   elems : [Int!]!
}

input InputSet_Int {
   elems : [Int!]!
}

input MyInput {  
    list1: InputNonEmpty_Int
    list2: InputSet_Int
    list3: [int!]! 
 }

is kind a too verbose but, what we can do?

[theobat]

Is it explicitly out of the introspection query or is it just "not mentioned" in the spec ? (I didn't find any reference to schema directives presence or absence in the introspection query).
Anyway, I think there are 4 options here:

• drop support for list-like structures (handle the list to Set or list to NonEmpty in your resolvers).
• support those structures "silently" (no information on the client, considered the same as lists).
• create a superset of the spec and implement new wrapper types.
• use custom directives on the schema, but still without information on the client (or maybe this is where we can create a superset of the spec)

At least that's the only options I see, for input types. Personally, I'm ok with 2, 3, 4 options and like 3 and 4 better.
It seems like other languages are doing 4 with no introspection info.
As far as other languages are concerned it seems the 1st option is the rule (Java).
I don't think "augmenting" the language with custom wrapper types is bad (like type script "augmented" javascript), but maybe I'm missing something...

[nalchevanidze]

for custom specifications compatibility problems with all graphql clients may occur. but i still think that, GraphQL should support this datatypes some way (for both Input and Output types).

one solution can be custom wrappers:

# is kind of wrapper
# rights side can of "=" :
#   - only can  appear wrapper types (like lists and custom wrappers)
#   - wrapper "!" is not allowed. e.g [a!] is invalid
wrapper NonEmpty a = [a]
wrapper Set a = [a]
wrapper Pair a b = [a, b]
wrapper Map a b = [Pair a b]

wrapper NonEmpty a =  [a]
wrapper Set a = [a]

input MyInput {
  list1: (NonEmpty Int!)!
  list2: (Set Int!)!
  list3: [int!]!
  map: (Map String! Int!)!
}

type MyOutput {
  list1: (NonEmpty Int!)!
  list2: (Set Int!)!
  list3: [int!]!
  map: (Map String! Int!)!
}

we can formalize it . and make a proposal as an issue for it on: https://github.com/graphql/graphql-spec/issues

[nalchevanidze]

@theobat what do you think?

[nalchevanidze]

server must define serialization methods for wrappers.

wrapper NonEmpty a = [a]

haskell

class Wrapper wrapper  where
    fromList :: [a] -> Either String (wrapper a)
    toList ::  wrapper a -> [a]

instance Wrapper NonEmpty where
   fromList x = ...
   toList x = 

js

const fromList = xs => {}
const toList = nonempty => [....]

const resolverMap = {
  NonEmpty: new GraphQLWrapperType({
    name: 'NonEmpty',
    description: 'NonEmpty list',
    parseValue: fromList,
    serialize: toList,
  })
}

[theobat]

Yeah it feels like the right way to do it, besides the main graphql implementation is in javascript and they already have added Set so, it feels like the way to go.

[theobat]

So let's work on the proposal to graphql then, cause it seems like the way to go.

[theobat]

For the record, I'm working on this:
https://gist.github.com/theobat/beccb49724250103535229f423b9084c

I think it would be interesting to reach people at https://github.com/hasura/graphql-engine, https://github.com/graphql-java/graphql-java, https://github.com/sangria-graphql/sangria and maybe even https://github.com/graphql-rust/juniper, to get more leverage; but let's waait for a better version of the proposal (in particular I'd like to tackle the actual spec changes before submitting and requesting comments).

[theobat]

@nalchevanidze I'd like your review on my draft proposal when you have time. If you find it OK, I'll probably talk about the idea in other graphql libraries before submitting it to the graphql-spec process.

[nalchevanidze]

@theobat can you merge your version with my draft.
https://github.com/morpheusgraphql/graphql-proposal-custom-wrapper-type

[theobat]

@nalchevanidze I think we should do the server-side only (simple Encode, Decode like lists) for Vector, because it's semantically equivalent to Lists, I can fix the PR for this if you want

That is to say, I don't think calling a list [Person] or Vector Person on the client really important introspection wise, they behave the same... (but maybe that's just me though).

[nalchevanidze]

@theobat for now we can just add missing instances without waiting of proposal wrapper types. and update library when the proposal is accepted.

[theobat]

Right, I'm going to rebase and clean this

[theobat]

@nalchevanidze, This should be ready now !

[nalchevanidze]

@theobat hi thanks, sorry for late response. could you update changelog too?

[theobat]

Hi yeah sure, I guess I should add a test too

[nalchevanidze]

i approved. you don't have to, but would be great if you would. you can add them in different merge request.