Multiple fields of type ID attempt to create multiple mst props of type Types.identifier (which is illegal)

[gitfish]

Multiple fields of type ID attempt to create multiple mst props of type Types.identifier - this is illegal as per https://github.com/mobxjs/mobx-state-tree/blob/master/packages/mobx-state-tree/src/types/complex-types/model.ts line 362.

A schema that produces this is (this is an amplify generated example):

type Blog @model { id: ID! name: String! posts: [Post] @connection(keyName: "byBlog", fields: ["id"]) }

type Post @model @key(name: "byBlog", fields: ["blogID"]) { id: ID! title: String! blogID: ID! blog: Blog @connection(fields: ["blogID"]) comments: [Comment] @connection(keyName: "byPost", fields: ["id"]) }

type Comment @model @key(name: "byPost", fields: ["postID", "content"]) { id: ID! postID: ID! post: Post @connection(fields: ["postID"]) content: String! }

Recommended solution:

Generate mst fields of type String for fields other than id (if id is not present, then possibly generate mst fields of type string once an ID type has been encountered).

[chrisdrackett]

I don’t believe this is valid GraphQL, I believe the ID scalar is meant for the unique identifier for a object. When doing a reference can you just do the following in GraphQL:

type Post {
id: ID
title: String!
blog: Blog
comments: [Comment]
}

[gitfish]

Yep, that'd definitely be nice graphql, but this particular schema is generated by amplify and the custom directives are used to generate indexes in dynamo and also allows access to the raw foreign key (of type ID).

I don't believe I've come across anything in the spec that says a type can't contain two fields of type ID - http://spec.graphql.org/June2018/#sec-ID. I understand that a field 'id' of type ID is typically used as a unique identifier, but I'm not sure that there's a restriction on the usage of the ID type. You might be able to point me to some information that indicates so.

[gitfish]

I guess the easiest option is to just change the types on the schema and see how I fair. It's not particularly accurate semantically though - not that I really care ;)

[gitfish]

Unfortunately I get InvalidDirectiveError: id field is not of type String when trying to change it.

[chrisdrackett]

hm, I'm not familiar with amplify so this is all new to me. Is this an input format like a prisma schema? I'm wondering if you can have amplify output a graphQL schema that you can then ingest in mst-gql.

[gitfish]

Hi,

The sample I posted was a graphql schema. The @s are called directives as outlined here: https://graphql.org/learn/queries and http://spec.graphql.org/June2018/#sec-Language.Directives

There are a couple of standard and then it’s up to how the server etc interprets them (in this case AWS AppSync).

Cheers,

On Fri, 1 May 2020 at 2:09 pm, Chris Drackett notifications@github.com wrote:

hm, I'm not familiar with amplify so this is all new to me. I don't see anything in the graphQL spec about decorators (@). Is this an input format like a prisma schema? I'm wondering if you can have amplify output a graphQL schema that you can then ingest in mst-gql.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/mobxjs/mst-gql/issues/222#issuecomment-622238414, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEVACGBERNHKN6OI6TSBPLRPJDQNANCNFSM4MWZHBIA .

[gitfish]

Directives aren’t relevant to the issue I raised. However, graphql builds schema will fail because it can’t understand the directives - there is some configuration/arts that can be passed to avoid this. I’m generating off the json version that amplify generates.

On Fri, 1 May 2020 at 2:09 pm, Chris Drackett notifications@github.com wrote:

hm, I'm not familiar with amplify so this is all new to me. I don't see anything in the graphQL spec about decorators (@). Is this an input format like a prisma schema? I'm wondering if you can have amplify output a graphQL schema that you can then ingest in mst-gql.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/mobxjs/mst-gql/issues/222#issuecomment-622238414, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEVACGBERNHKN6OI6TSBPLRPJDQNANCNFSM4MWZHBIA .

[chrisdrackett]

from the graphql site:

The core GraphQL specification includes exactly two directives, which must be supported by any spec-compliant GraphQL server implementation: @include(if: Boolean) Only include this field in the result if the argument is true. @skip(if: Boolean) Skip this field if the argument is true.

I see one more in the working spec, @deprecated

@model, @key, @connection are not standard and I don't think mst-gql can take on understanding or supporting these directives, at least until they are part of the graphql spec.

[chrisdrackett]

when you are querying your server is it possible to get back the postID or blogID field? or do these just have post and blog?

[gitfish]

Every framework uses several custom directives - e.g. Apollo - this is not unique to AppSync. Also, directives in this case aren’t the issue. The issue is that two Types.identifier props are being produced in mst. I’ve manually modified the output files to use Types.string vs Types.identifier on the blogID etc fields on the generated base model (should not be touching that) and it works fine. Anyway, I’ll go to another framework - I can see there is a fair bit of resistance. I can raise a pul request so this issue is at least clear.

[gitfish]

And, yep, naturally the postID and blogID fields are available to query as they're on the schema.

[chrisdrackett]

Sorry if I'm coming off as resistive, I'm just trying to understand the problem space to be able to help work to a solution.

Seems like it would be possible for us to add a way via config to specify the field to use as Types.identifier in the case that more than one exist, and I'm sure we would be happy to look at a PR.

I just want to make sure that I both understand the use case and that it is valid before working to figure out the best solution.

[special-character]

The issue I see right now with having two ID fields is that the mst-gql generation will have no way to determine which should be the identifier.

When I typically have GraphQL that references another table, the resolver returns the full data as a part of the graph instead of just the id. I might be missing something but I don't quite understand why two IDs would be needed since the whole point of GraphQL is to use fewer requests to build out the whole graph instead of just returning the id to look up later.

I think it would be possible to add a way to specify the identifier instead of it being based on ID but I am curious about the use case and reasoning for having multiple IDs. It feels confusing on more levels than just mst-gql generation to me.

[chrisdrackett]

I think the whole Blog object is available via blog but blogID also exists. It seems to me this exists because amplify is using the graphql schema both for generating the database and defining what is returned. This is different from something like prisma where you define in one place and the generated schema is simpler.

[special-character]

Ahh, I see. It does seem like mst-gql should be able to support the flexibility

[chrisdrackett]

@gitfish added https://github.com/mobxjs/mst-gql/issues/223 to cover this, FYI