Generate 'possibleTypes' from implemented 'interfaces' field

[th3coop]

This PR Fixes #607.

Instead of using the possbileTypes field on an ObjectType, this PR instead looks at the interfaces field for every InterfaceType and ObjectType definition to see what Interface it implements then add it to the Interfaces possibleTypes list.

Summary of changes:

• We're now decoding interfaces field out of the gql schema response where applicable
• Several helper functions added to Type.elm

[th3coop]

Ok, now that I understand the build checks I'm going to jump back into this. I plan to, at the very least, rename interfacePossibleTypesDict to something else so it doesn't seem like I'm still using that field.

I also acknowledge that it's likely a big nono that I've now exposed that function in order to make the tests work so I'm going to revisit that and see if there is a higher lvl I can put all the data together at or something.

[dillonkearns]

Sounds good! Could you also add an example that shows up in the end-to-end code?

This would be a good place to add it:

https://github.com/dillonkearns/elm-graphql/blob/master/ete_tests/edge-cases.gql

That way we'll have the new logic covered in the end-to-end output to make sure the output is looking good, and prevent it from accidentally changing in the future.

[th3coop]

Ok, found some issues with the generated code once I got the ete stuff added that I didn't notice when I was running things before (so good thing it's there!). I have an idea that I'm working through but it's going to take longer than I thought. Will be back.

Question: I see that for the ete test, it hits a server running on Heroku. Do I need to do anything to update that or does it update itself when I push the ete schema change?

[th3coop]

@dillonkearns K I think this is ready.

I'm not sure how you usually do these reviews but I'll point out the two major functional changes

• Totally changed how this function works AND had to expose it to get tests working. I can't see a way to un-expose it. It seems to fall under the elm-test black hole of having to expose some inappropriate stuff to get all the testing you want. https://github.com/dillonkearns/elm-graphql/pull/608/files#diff-d159547b90259873b0b0043d8b48fe75042b7711a4279f630b70f176d03f746bL45
• Quite a few added and exposed functions to Type.elm: https://github.com/dillonkearns/elm-graphql/pull/608/files#diff-ff2f69b1959a2df15054d6b51f99305aca05d377a6b7ce768e295c2e92c2495dR12

[th3coop]

Morning @dillonkearns. Could I get approval for the workflows to run? I promise I didn't commit and Debug.logs

[th3coop]

And this time I ran npm run build before pushing the code 🤕 Sorry about that @dillonkearns

[th3coop]

Resolved: https://github.com/dillonkearns/elm-graphql/pull/610

@dillonkearns, to be able to run elm-review locally I had to updated it so there are some changes in this PR now that aren't directly related to the root issue it's addressing. I could easily do that in a separate branch if you like to keep things nice and tidy.

- This change is due to the new version of Elm-review and catching an unused module in the repo: https://github.com/dillonkearns/elm-graphql/pull/608/files#diff-d4b4331a1a542ce28ac90a7d59d1cf6f04eb34df3ae695d33c4f740c2c6c19c6L29 - suggested by jfmengels here , ran npx elm-json upgrade: https://github.com/dillonkearns/elm-graphql/pull/608/files#diff-9df456f94279a3c092f3f3011d527594b850731bc4e6f04191ab79e6ba3243b8

[th3coop]

OK, now I've run elm-review, npm run build and npm run test. It workflows should complete meow.,

[th3coop]

@dillonkearns ping for workflow run.

[th3coop]

Hey @dillonkearns, have you had time to take a look at this? Do you feel like this is a change you'll be interested in merging?

[dillonkearns]

Hey @th3coop, definitely interested in merging! I've been a bit busy with travel lately, sorry for the delayed response.

The only things I need before merging this are:

• To understand why API changes to the published package are necessary, and consider whether there is any way to avoid (or minimize that)
• I'd like to understand the before picture. It would be really helpful if I could compare the generated code before and after. Something that would help with that would be to see the end-to-end test results before this change, ideally in a git diff. I can work on that when I have some spare cycles.

[th3coop]

Awwwww, but I was going to play Star Craft tonight.

Kidding! Absolutely I can address these points. I can get that diff of the before/after.

Follow up question:

• For the first point, clarifying API change; you are looking for an explicit statement as to why that change was necessary, why it's not possible to avoid it and what I've done my best to try to minimize it's effect as much as possible?

[dillonkearns]

you are looking for an explicit statement as to why that change was necessary, why it's not possible to avoid it and what I've done my best to try to minimize it's effect as much as possible?

Yeah, exactly, basically that's something I need to understand before merging and releasing this, and anything you can do to help me understand that will speed things up.

[th3coop]

I can do this 👍

[th3coop]

API changes:

generator/src/Graphql/Parser/Type.elm

• All of these additions are helpers functions which remove the need to perform a case of within a lambda, or implement the same function in a let block outside of the Type.elm file.
• These functions operator on types that are already exposed so I interpret this as making those types easier to use.
• I removed one function that is only performing a decomposition of the TypeDefinition type to get at the ClassCaseName which is easy enough to do in a lambda, eg. (\(TypeDefinition n _ _) -> ClassCaseName.raw n)

generator/src/Graphql/Generator/Group.elm

• interfacePossibleTypesDict -> interfaceImplementorsDict and exposing the new function.
• This is the core of the change.
• When processing Interfaces now we need to do some post decode processing on the decoder Result to look at all of the ObjectType and InterfaceType fields and see what interface they implement if any then add them to the Interface they implement.
• Because of the above, to get tests working I had to expose Group.interfaceImplementorsDict and run it on the decode Result in the test otherwise InterfaceTypes won't contain all the other types that implement it. ie. we can't just run the interfaceDecoder and get the correct results. We have to run interfaceDecoder and objectDecoder then run the results through interfaceImplementorsDict so all the interfaces get their implementors set.

generator/src/Graphql/Generator/Context.elm

• Changing InterfaceLookup type alias was required to have the right data available when generating Module paths in generator/src/Graphql/Generator/Interface.elm:70. ie. knowing if the fragment was in the Interface folder or the Object folder.

Hopefully that clarifies why I think these API changes are necessary/required. I acknowledge that the changes to Type.elm are not required but I really do think that that is the best place for those functions rather than one offs in let blocks where they are called. Having said that, I'll be happy to move them to a let block if you'd rather that.

[th3coop]

And here's a diff of the generated ete code that shows what the changes in this PR result in:

diff --git a/ete_tests/src/EdgeCases/Interface/Character.elm b/ete_tests/src/EdgeCases/Interface/Character.elm
index dafe3217..9b83f662 100644
--- a/ete_tests/src/EdgeCases/Interface/Character.elm
+++ b/ete_tests/src/EdgeCases/Interface/Character.elm
@@ -20,7 +20,8 @@ import Json.Decode as Decode

 type alias Fragments decodesTo =
-    { onHuman : SelectionSet decodesTo EdgeCases.Object.Human
+    { onSpecies : SelectionSet decodesTo EdgeCases.Interface.Species
+    , onHuman : SelectionSet decodesTo EdgeCases.Object.Human
     }

@@ -31,7 +32,8 @@ fragments :
     -> SelectionSet decodesTo EdgeCases.Interface.Character
 fragments selections____ =
     Object.exhaustiveFragmentSelection
-        [ Object.buildFragment "Human" selections____.onHuman
+        [ Object.buildFragment "Species" selections____.onSpecies
+        , Object.buildFragment "Human" selections____.onHuman
         ]

@@ -40,7 +42,8 @@ update syntax to add `SelectionSet`s for the types you want to handle.
 -}
 maybeFragments : Fragments (Maybe decodesTo)
 maybeFragments =
-    { onHuman = Graphql.SelectionSet.empty |> Graphql.SelectionSet.map (\_ -> Nothing)
+    { onSpecies = Graphql.SelectionSet.empty |> Graphql.SelectionSet.map (\_ -> Nothing)
+    , onHuman = Graphql.SelectionSet.empty |> Graphql.SelectionSet.map (\_ -> Nothing)
     }

I made a full PR for comparison and the specific changes is here: : https://github.com/th3coop/elm-graphql/pull/1/files#diff-0222bf4407ccb0114ee199ecfe42d21d7720f3a3422c5746096f26a4caf0d224L23

[dillonkearns]

Hey @th3coop thanks so much for giving me the diff and explanation of all these changes. I realized that these changes don't effect the published Elm package at all (neither the public interface or the code that runs within the Elm package) because all of the changes are within the generator/ folder, so that makes it even simpler! Thanks for outlining things, that made it a lot easier to navigate.

It's looking great, I went ahead and published this so it's now available with the latest NPM package, version 4.3.1. Thanks again for your work on this, I really appreciate it! 🙏

[th3coop]

Awesome! I've got a Jira ticket waiting for me to update our code to use this new version! It was my pleasure to help! Amazing setup in this package. So easy to add to it and test it for such a complicated paradigm.