Skip to content

Why does the gateway expand interface fragment into multiple implementations fragments? (federation) #3253

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

Closed
vitramir opened this issue Sep 5, 2019 · 11 comments · Fixed by #3529
Assignees
Labels

Comments

@vitramir
Copy link
Contributor

vitramir commented Sep 5, 2019

For example we have SDL with interface and multiple implementations:

interface Node{
    id: ID!
}

type Brand implements Node{
    id: ID!
    title: String!
}

type Department implements Node{
    id: ID!
    title: String!
}

type Query{
    brand: Brand
}

Then query

{
    brand {
        ... on Node {
            id
        }
    }
}

will be transformed into

Fetch(service: "data") {
    {
      brands {
        ... on Brand {
          id
        }
        ... on Department {
          id
        }
    }
}

So, the result will be error "Fragment cannot be spread here as objects of type "Brand" can never be of type "Department"."

Why don't we pass fragments as is?

@vitramir
Copy link
Contributor Author

vitramir commented Sep 6, 2019

Ok, I think I can answer the question myself. We should do it because we don't want to define all interfaces in every service.

@trevor-scheer
Copy link
Contributor

@vitramir my apologies for not responding to your question. This is actually a bug within query planner that we'll be working to fix. The query is fine, and the query planner should just remove the type condition Node since every Brand is also a Node. I currently have a simple reproduction and am building one that's a bit more complex with nesting. @martijnwalraven and I will be working on this Monday!

@vitramir
Copy link
Contributor Author

vitramir commented Sep 7, 2019

@trevor-scheer Check my PR, please.
I store all parent types from fragments (and nested fragments too), find intersection of implementations and expand fragment into possible types only.

@vitramir
Copy link
Contributor Author

My team found one more case. Interface inside interface.

interface Item{
    id: ID!
    content: Content
}

enum ContentType{
    text
    number
}

interface Content{
    type: ContentType
}

type Text implements Content{
     type: ContentType
     value: String
}

type Number implements Content{
     type: ContentType
     value: Int
}

type Brand implements Item{
    id: ID!
    content: Text!
}

type Department implements Item{
    id: ID!
    content: Number!
}

type Query{
    item: Item
}

Then query

{
  item {
    content {
      type
    }
  }
}

will be transformed into

Fetch(service: "data") {
    {
      item {
        ... on Brand {
           content {
              ... on Text {
                type
              }
              ... on Number {
                type
              }
           }
        }
        ... on Department {
           content {
              ... on Text {
                type
              }
              ... on Number {
                type
              }
           }
        }
    }
}

But Department's content can't be of type Text and Brand's content can't be of type Number

@vitramir
Copy link
Contributor Author

Looks like I have fixed this in last commit

@Pruxis
Copy link
Contributor

Pruxis commented Sep 12, 2019

Great @vitramir I also noticed this today at work 😄

@rtymchyk
Copy link

I don't follow this conversation - is this still a TODO @trevor-scheer?

@rtymchyk
Copy link

@vitramir @Pruxis what is the fix? Would be great if you could share...

@Pruxis
Copy link
Contributor

Pruxis commented Sep 24, 2019

For me bumping to the latest version solved my issues.

@vitramir
Copy link
Contributor Author

@rtymchyk did you check the PR #3257

@rtymchyk
Copy link

@vitramir Ah ok, my bad, got confused thinking there was an app-specific fix. The PR makes sense and fixes the problem in our use case.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants