haskell-graphql / graphql-api

Write type-safe GraphQL services in Haskell
BSD 3-Clause "New" or "Revised" License
406 stars 35 forks source link

Move Name to Value module and make invalid Objects impossible #24

Closed jml closed 7 years ago

jml commented 7 years ago

I started out moving Name to the Value module with the intent of not adding any new TODOs. I ended up adding three, with a much bigger PR than I expected.

That's because I tried to eliminate the TODO that asked What the heck does unionObject do? I wrote a test to figure it out and found that it constructed invalid objects--objects with duplicate field names. I really don't want to ever have that bug again, so I jotted down a property and updated the types to give Object smart constructors so it's now only possible to construct Objects with unique field names.

To be honest, I still don't understand why unionObjects is the right behaviour in the union resolver, but I'm certain that its old behaviour was definitely wrong.

jml commented 7 years ago

PTAL

jml commented 7 years ago

Sure, I meant that if we validate the query there would be no need to check for duplicate fields. Even if the Object defines duplicate fields (which is another interesting bug we should catch, see discussion below) then we'd never hit this condition.

Ah, yes. I make a comment about this in #26.