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

Add union support #30

Closed
wants to merge 1 commit into from
Closed

Add union support #30

wants to merge 1 commit into from

Conversation

DmitryTsepelev
Copy link
Owner

No description provided.

@DmitryTsepelev DmitryTsepelev force-pushed the union-support branch 2 times, most recently from 1ffd5a0 to c58808d Compare October 1, 2020 19:36
@jeromedalbert
Copy link
Contributor

jeromedalbert commented Apr 28, 2021

I guess that you didn't manage to make this workaround PR work, or the specs never passed?

We are using a few unions in our graphql app, so I am getting those Failed to look ahead the field errors. Since the impacted cache_fragment: fields are sometimes used in vastly different queries and selections, I don't think I can safely pass a custom (mostly static) query_cache_key as a workaround, since the respective fragments could look wildly different and this could result in serving wrong cached fragments. Unless there is some way to fingerprint a graphql query selection without using lookaheads, but I don't know of one.

I tried using rmosolgo/graphql-ruby#3007 's branch in my app and still got the Failed to look ahead the field error. I am guessing that GraphQL::FragmentCache would need to somehow adapt to rmosolgo/graphql-ruby#3007 if it gets merged one day.

@DmitryTsepelev
Copy link
Owner Author

The problem is that lookahead does not support unions, and we build a cache key based on it. I feel like it's easier to find a different reliable way to build a key without the lookahead rather than try to fix it's behaviour.

@hendie
Copy link

hendie commented Nov 16, 2022

Any progress on this PR @DmitryTsepelev ? We also have quite a few instances of unions in our data model. I would love to use your cache offering, but that's kinda limiting us from further evaluation at this time.

@jeromedalbert
Copy link
Contributor

jeromedalbert commented Nov 17, 2022

FYI I worked around the issue in my app by not using union types, and using interfaces instead. It seemed to mostly work.

Unions and interfaces in GraphQL are very similar, so migrating required minimal effort. No changes were needed for the frontend / API consumer, since the ... on syntax in queries remained the same.

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.

3 participants