gqlgen icon indicating copy to clipboard operation
gqlgen copied to clipboard

Aliased field not being returned if asked for in a fragment

Open ryanwmoore opened this issue 4 years ago • 12 comments

What happened?

Aliased fields are not returned when asked for in a fragment.

This query works as expected:

query MyQuery {
  hero {
    name
    aliasedName: name
  }
}

This query does not. My understanding is they should be equivalent:

query MyQuery {
  hero {
    ...SomeFragment
  }
}

fragment SomeFragment on Character {
  name
  aliasedName: name
}

What did you expect?

In both cases, I expect to see

{
  "data": {
    "hero": {
      "name": "R2-D2",
      "aliasedName": "R2-D2"
    }
  }
}

Instead, in the broken query (the one using a Fragment), I see that aliasedName is not returned:

{
  "data": {
    "hero": {
      "name": "R2-D2"
    }
  }
}

Minimal graphql.schema and models to reproduce

See Star Wars example in gqlgen repo. These queries work as I expect when pasted into the textboxes at https://graphql.org/learn/queries/

versions

  • gqlgen version? master head (39a12e0f1b6d9833f516f0271db0dbfa45c5ec45)
  • go version? go1.14.6 darwin/amd64 but I saw similar behavior in go1.14.7 on Linux
  • dep or go modules? gqlgen is using go modules

ryanwmoore avatar Aug 07 '20 17:08 ryanwmoore

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Nov 06 '20 02:11 stale[bot]

Nope.

frederikhors avatar Nov 06 '20 02:11 frederikhors

FWIW, I'm seeing the same bug happen with aliased fields, unrelated to being inside of a fragment. Instead of not showing up in the result at all, it shows up but gets null. Probably related.

BrianGenisio avatar Nov 06 '20 18:11 BrianGenisio

Clarification: we're using federation so it turns out the federation server is re-writing the query to our GraphQL server to use a fragment. So yeah, we're seeing exactly this problem as well.

BrianGenisio avatar Nov 06 '20 19:11 BrianGenisio

Ok, I understand why it is happening but I don't have enough context within the code of this project to fix it.

In this code, it is determining that an alias of the name of the field already exists, so it is returning the first field on the aliased field. This is because in the field structure, the Alias is set to be the Name if an alias is not specified.

In the example above, the name field is not aliased, so the field is defined as {Name: "name", Alias: "name", ...}. The next time through this function for the alias field, we are looking for a field that is defined as {Name: "name", Alias: "aliasedName"}. Since the first field has an alias as name and it is now looking for name, it lumps the two fields together and the rest is history.

It does seem odd to me that the field structure would default Alias to be the same as the Name, but there's probably a reason. I must confess, I don't know what the best way to fix this is. I just looked at this code for the first time an hour ago. But hopefully by pointing out the why of the bug, a maintainer might be able to suggest a best way to fix?

BrianGenisio avatar Nov 06 '20 20:11 BrianGenisio

This seems related to the behavior observed in this ticket: https://github.com/99designs/gqlgen/issues/83.

The fix for that issue (https://github.com/99designs/gqlgen/pull/84) included a test for the working case that @ryanwmoore mentioned above. This may explain the insistence of a later commenter that the bug wasn't fixed, despite there being a clear test case for it: the behavior is different depending on whether it's a direct query or a fragment.

While not explicitly mentioned in the older ticket, its title suggests a possible workaround: aliasing all usages of the field that needs to get used in different contexts. I haven't investigated this thoroughly but it did work in my particular use case. In the Star Wars example it might look something like this:

query MyQuery {
  hero {
    ...SomeFragment
  }
}

fragment SomeFragment on Character {
  aliasedName1: name
  aliasedName2: name
}

hardlyknowem avatar Nov 30 '20 22:11 hardlyknowem

@hardlyknowem Yeah, aliasing all fields fixes the problem. But we can't always control the clients that make the query. And this should work from a GraphQL schema perspective. Consumers will just be scratching their heads wondering why they aren't getting any data. And it can be more hidden inside polymorphic types... for example:

fragment SomeFragment on Character {
  name
 ... on SuperCharacter {
    superName: name(super:true)
  }
}

BrianGenisio avatar Mar 16 '21 10:03 BrianGenisio

Looks like this may have been fixed in 130ed3f7d? (Edit: Brian and I are following up offline and will report back.)

benjaminjkraft avatar Mar 16 '21 17:03 benjaminjkraft

Ok, we believe this was fixed, or at least the case of it that we ran into was.

benjaminjkraft avatar Mar 16 '21 19:03 benjaminjkraft

Agreed with @benjaminjkraft (my colleague). I tested 130ed3f under a couple of different scenarios and it fixed the problem. We're very excited. It looks like it was too late for 0.13.0 but we might patch it for our usage until there is a new, official, release. Does anyone know when that will be?

BrianGenisio avatar Mar 17 '21 09:03 BrianGenisio

Oh good! We're still stuck on v11.0 for the time being (see #1293, #1317, #1391, #1392). The quick gist is an error was introduced in the codegen for v12. There have been some two PRs to fix it, but the PRs aren't being accepted without automated tests, but automated tests would require extensive refactoring. But it's good to know that this issue will be fixed when we upgrade. Fortunately we can still use the workaround described above.

hardlyknowem avatar Mar 18 '21 16:03 hardlyknowem

Now that v0.14 is out, this appears to be resolved, at least in the form the original issue describes.

hardlyknowem avatar Oct 03 '21 13:10 hardlyknowem