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

Composing Selections #64

Closed
Jayshua opened this issue Jul 2, 2018 · 10 comments
Closed

Composing Selections #64

Jayshua opened this issue Jul 2, 2018 · 10 comments

Comments

@Jayshua
Copy link
Collaborator

Jayshua commented Jul 2, 2018

The Use Case

I've found it's quite convenient to define a data-type without an id field. For example, say I have a person entity.

type alias Person =
    { firstName: String
    , lastName: String
    , age : Int
    }

This way I can reuse (potentially very large) record types for both creation code as well as get/updates. On the surface this seems like a small win - is it really that big of a deal to define a second type for the create function? However, it also allows me to reuse the views and update functions as well - not an insignificant amount of code. (I've considered relying on type-inference to construct the view/update signatures and passing the two different records. I haven't explored this enough to know whether it's a viable alternative.)

Of course, I still need the ID if it exists. So the get signature looks something like ID -> SelectionSet (ID, Person), and the update something like (ID, Person) -> SelectionSet (ID, Person).

The Problem

There is no easy way to go from an ID-less selection set to a tuple with the ID.

Here's how I do it now

type alias Person =
    { firstName: String
    , lastName: String
    , age : Int
    }

type alias PersonWithId =
   { id : ID
   , firstName : String
   , lastName : String
   , age : Int
   }

attendeeFragment : SelectionSet (Int, Person) Api.Object.Person
attendeeFragment =
    let
        selection =
            Api.Object.Person.selection PersonWithId
                |> with Api.Object.Person.id
                |> with Api.Object.Person.firstName
                |> with Api.Object.Person.lastName
                |> with Api.Object.Person.age

        toTuple person =
            ( person.id
            , { firstName = person.firstName
              , lastName = person.lastName
              , age = person.age
              }
            )
    in
    Graphqelm.SelectionSet.map toTuple selection

You can imagine a selection with 20 or 30 fields getting quite repetitive.

Here's how I'd like to do it

type alias Person =
    { firstName: String
    , lastName: String
    , age : Int
    }

attendeeFragment : SelectionSet (Int, Person) Api.Object.Person
attendeeFragment =
    let
        bodySelection =
            Api.Object.Person.selection Person
                |> with Api.Object.Person.firstName
                |> with Api.Object.Person.lastName
                |> with Api.Object.Person.age
        idSelection =
            Api.Object.Person.selection identity
                |> with Api.Object.Person.id
    in
    Graphqelm.SelectionSet.map2 (,) idSelection bodySelection

Notice that the bodySelection can be extracted and reused across all three create/get/update functions. There are probably more ways to do it, but that's what I had in mind.

Other Thoughts

This use-case is purely imaginary at the moment. I'll be building something like it in the not-to-distant future though, so I may be able to come back and let you know how it goes.

Imagine having a drop-down list that allows the user to select a person. It requires the firstName and lastName fields. Ideally, it wouldn't need to know anything about the context it is placed in. Say an Invoice entity has a list of Person indicating the contacts it is associated with. In GraphQL this could be composed like this (Age is required for other reasons):

invoice {
    id
    note
    contacts {
      age
       ...dropdownFragment
    }
}

Again, it may be best to simply have the drop-down not know anything at all about the query. I haven't gotten to this part of my project yet, so probably best not to base any big decisions on this use case.

@xtian
Copy link
Contributor

xtian commented Aug 29, 2018

@Jayshua You have probably struck upon this workaround as well but I have handled a similar case in my app by making an additional query:

Api.Query.selection (Maybe.map2 Tuple.pair)
  |> Graphql.SelectionSet.with idSelection
  |> Graphql.SelectionSet.with bodySelection

@dillonkearns
Copy link
Owner

@Jayshua for the particular use case you mention, have you considered using extensible records? Richard gives a great description of some ways you can use extensible records to decouple from certain fields in a record in this Elm Europe 2017 talk (tons of other great tips in there, too).

So instead of depending on Person or PersonWithId in your functions, you could have annotations like this:

helperFunction :   { person |
     firstName : String
   , lastName : String
   } -> String
helperFunction { firstName, lastName } =
  firstName ++ " " ++ lastName

I'd be curious what your (and other people's) thoughts are about use cases where this syntax would still come in handy given @xtian's trick and the extensible records syntax. I'm not saying that there aren't examples, just that I'd like to know about them so that I can drive API improvements based on real-world use.

Thanks everyone for the helpful conversation!

@Munksgaard
Copy link

Munksgaard commented Nov 1, 2018

I ran into the same issue today, but extensible records don't really do the trick for me.

For my example, I have defined two types: Asset and Quote, and they have the following selectionSets defined:

assetDecoder : SelectionSet Asset Project.Object.Asset
assetDecoder =
    Asset.selection Asset
        |> with Asset.name

quoteDecoder : SelectionSet Quote Project.Object.Quote
quoteDecoder =
    Quote.selection Quote
        |> with (Field.map (\(Scalar.Date date) -> date) Quote.date)
        |> with (Field.map (\(Scalar.BigFloat value) -> value) Quote.value)         

Now, I want to execute the following graphql query and get a (Asset, List Quote):

{
  assetById(id: 500) {
    name
    quotesByAssetId {
      nodes {
        date
        value
      }
    }
  }
}

The problem is that I cannot actually use those two decoders that I defined above without altering my query to something like

{
  assetById(id: 500) {
    name
  }
  assetById(id: 500) {
    quotesByAssetId {
      nodes {
        date
        value
      }
    }
  }
}

which, for complicated queries, could have a negative impact on the performance. Instead, I would need to define an additional decoder assetAndQuotes, but then I lose composability of my decoders and have to define lots of individual ones for individual use-cases.

What I would like to see is a compose or combine function like the following:

combine : (a -> b -> c) -> SelectionSet a typeLock -> SelectionSet b typeLock -> SelectionSet c typeLock

that would allow me to combine two SelectionSets in the same query. Then I could simply write

combinedQuery : AssetId -> SelectionSet (Maybe ( Asset, List Quote )) RootQuery
combinedQuery (AssetId assetId) =
    Query.selection identity
        |> with
            (Query.assetById { id = assetId }
                (combine
                    Tuple.pair
                    assetDecoder
                    quotesDecoder
                )
            )

which I think is a lot nicer.

@Munksgaard
Copy link

So I think I managed to create my own combinator:

map2 :
    (a -> b -> value)
    -> SelectionSet a typeLock
    -> SelectionSet b typeLock
    -> SelectionSet value typeLock
map2 combine (SelectionSet objectFields1 objectDecoder1) (SelectionSet objectFields2 objectDecoder2) =
    SelectionSet (objectFields1 ++ objectFields2)
        (Decode.map2 combine
            objectDecoder1
            objectDecoder2
        )

However, I'm not sure if I should handle duplicates in the objectFields lists. @dillonkearns you probably have a better understanding of how it works, what do you think?

@dillonkearns
Copy link
Owner

That's right @Munksgaard, I wish it was that simple but the thing that makes it difficult is preventing duplicate collisions (which would result in errors). The issue is that these are built up into decoders, so you can't fix it without some sort of deeper design change. See the code in the SelectionSet.with function: https://github.com/dillonkearns/elm-graphql/blob/master/src/Graphql/SelectionSet.elm. I should have mentioned that earlier on this thread, I guess I only said it in the original Slack thread. Thanks anyway though!

@dillonkearns
Copy link
Owner

dillonkearns commented Nov 5, 2018

Good news! I came up with a really simple solution to make SelectionSets more "pure" and less stateful so that we can safely join together multiple SelectionSets without worrying about the aliasing. In a nutshell, the idea is to make the aliases hash-based rather than index-based so that you don't need to know about any surrounding context to know whether there will be an alias or what the alias will be.

The index-based aliases looked like this:

query {
  human(id: "1001") { name }
  human1: human(id: "1004") { name }
  human2: human(id: "1004") { id }
}

This approach would increment the index for each previous sibling. No index was used for the 0th lookup of a field.

The new solution uses hash-based aliases to avoid duplicate conflicts, like this:

query {
  human1213318493: human(id: "1001") { name }
  human3685532794: human(id: "1004") { name }
  human3685532794: human(id: "1004") { id }
}

Notice that the two human(id: "1004") fields share the same alias name. The hash is determined purely based on two things, 1) the field's name (human in the aforementioned example), and 2) the field's arguments ((id: "1004") in the aforementioned example). And that's it! Super simple! We actually don't need to look at the field's children (i.e. nested SelectionSets as in human(id: 1004) { friends { name } }) because if those nested fields within a given Field's SelectionSet contain any arguments, then they will have a hashed alias! So the solution is quite robust.

And since the hash is only based on those two things, if there are no arguments for a given field, then there is no alias at all! So the queries for the most part stay pretty readable. I think this approach is fairly unlikely to cause bugs as well because of its simplicity.

This query is valid because it leverages a GraphQL feature called Field Selection Merging (there's a nice human-readable explanation here, and the formal spec is here). Field merging means that this query (you can run it with this link) will merge the two queries together into a single response like so:

query {
  myAlias: hero { id }
  myAlias: hero { name }
}
"myAlias": {
  "name": "Luke Skywalker",
  "id": "1000"
}

So with the hash-based alias names, we can safely merge SelectionSets together! I will post another update soon.

@Munksgaard
Copy link

That's great to hear @dillonkearns! I'm looking forward to seeing the final implementation :-)

@dillonkearns
Copy link
Owner

Alright, I've added SelectionSet.withFragment and SelectionSet.map2 in version 1.2.0 of dillonkearns/elm-graphql. Thanks everyone for the code examples and feedback!

Let me know on Slack or Github if you have any feedback for this change! Here's an example of SelectionSet.withFragment for reference.

@Jayshua
Copy link
Collaborator Author

Jayshua commented Nov 11, 2018

@dillonkearns Just had a chance to use this and it worked exactly like I hoped it would. Thank's so much!

@dillonkearns
Copy link
Owner

Fantastic, @Jayshua, it's really good to hear real feedback from actual usage. And I'm really glad that helped with your use case 😄 Thank you for reporting back, and thanks for the initial feedback!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants