Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How to create an optimisticResponse for a mutation returning union type? #321

Closed
reck753 opened this issue Mar 26, 2022 · 5 comments
Closed

Comments

@reck753
Copy link
Contributor

reck753 commented Mar 26, 2022

Hi!

Question setup

type TextPost {
  id: ID!
  text: String!
  liked: Boolean!
} 

type ImagePost {
  id: ID!
  url: String!
  liked: Boolean!
}

union Post = TextPost | ImagePost

type Mutation {
  like(id: ID!): Post!
  dislike(id: ID!): Post!
}
// MyComponent.res

module PostFragment = %relay(`
  fragment MyComponent_post on Post {
    ... on TextPost {
      id
      liked
    }
    ... on ImagePost {
      id
      liked
    }
  }
`)

module LikeMutation = %relay(`
  mutation MyComponent_likeMutation($id: ID!)
  @raw_response_type {
    __typename
    ...MyComponent_post
  }
`)

Question

As you already know, union type is translated to polymorphic variant and hence the optimisticResponse, of a mutation returning union type, expects the same thing. However, by doing so:

like(
  ~variables={
    id: id,
  },
  ~optimisticResponse={
    like: #TextPost({
      __typename: #TextPost,
      __isPost: #TextPost,
      id: id,
      liked: true,
    })
  },
  (),
)

we would get this error:

Warning: Expected `optimisticResponse` to match structure of server response for mutation `MyComponent_likeMutation`, please define fields for all of
{
  "like": {
    "__typename": "<scalar>",
    "id": "<scalar>"
  }
}
Warning: Expected `optimisticResponse` to match structure of server response for mutation `MyComponent_likeMutation`, please remove all fields of
{
  "like": {
    "NAME": {},
    "VAL": {}
  }
}

which completely makes sense since NAME/VAL object is how polymorphic variant is being translated to JS.

We also tried doing something unsafe like this:

like(
  ~variables={
    id: id,
  },
  ~optimisticResponse={
    like: {
      "__typename": "TextPost",
      "__isPost": "TextPost",
      "id": id,
      "liked": true,
    }->Obj.magic
  },
  (),
)

but that would return this error:

Warning: RelayResponseNormalizer: Payload did not contain a value for field `like: like(id:"SOME_NODE_ID")`. Check that you are parsing with the same query that was used to fetch the payload.

We are wondering if there is any type safe solution for this?

Apart from that having it is pretty hard to make optimisticResponse as well:

let post = PostFragment.use(post)

...

~optimisticResponse: {
  like: switch post {
    | #TextPost({ id }) => #TextPost({ id: id, liked: true }) // because they have different `record` types
    | #ImagePost({ id }) => #ImagePost({ id: id, liked: true })
    | #UnselectedUnionMember(x) => #UnselectedUnionMember(x)
  }
}

As a solution, we could change Post be an interface, which probably makes sense for this example, and take id and liked as common fields and avoid having inline fragments at all. However, this would be a workaround, since we might stumble on a case where we wouldn't have any common fields (#315), and we might get the same result as with union since, in that case, interface would be translated to polymorphic variant as well.

Thanks in advance and sorry for yet another long question/issue...

@reck753
Copy link
Contributor Author

reck753 commented Mar 26, 2022

I just saw #316, so I don't know if this that I wrote was affected by that PR...

@zth
Copy link
Owner

zth commented Mar 26, 2022

@reck753 haha, the timing! You are absolutely correct that this was indeed a bug that #316 fixed. The fix has been released in beta.16, so please try that, it should work just like you expect. To clarify - this should work just as you expect, values should be translated automatically.

Sorry for the inconvenience!

@reck753
Copy link
Contributor Author

reck753 commented Mar 26, 2022

The timing is incredible! My bad for not checking what was done in the meantime 😅
It feels like you read my mind since I was experiencing this issue yesterday...

Thanks a lot! Will try it and let you know!

@zth
Copy link
Owner

zth commented Mar 26, 2022

@reck753 yes please try it and let me know! It's embarrassing this bug has been sitting around for this long 😬

And thank you for posting good questions with great background info - my goal is for RescriptRelay to be as ergonomic as I can make it, so you're definitively encouraged to keep posting things like this any time you find something that feels hard/cumbersome/unclear. Thanks!

@reck753
Copy link
Contributor Author

reck753 commented Mar 26, 2022

@zth I just tried it and it works like a charm! Thank you!

It's not embarrassing at all. The thing you do for us is pretty hard, so we can't be mad if something is missing out, especially when you are quick to fix it!

It is easier for everyone to make precise and well explained issues, even if it takes some time doing so. So, definitely, if there is something to be asked, I will be descriptive! 😄

@reck753 reck753 closed this as completed Mar 26, 2022
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

No branches or pull requests

2 participants