Fork me on GitHub
#pathom
<
2021-02-18
>
jmayaalv09:02:16

We got asked today the possibility to expose our pathom api as a graphql api. i know this has been discussed several times and there are attempts like https://github.com/denisidoro/graffiti but do you think it would make sense to relay on pathom3's smart maps as lacinia resolvers? so basically we would create the graphql schema define the root queries attaching resolvers that use smartmaps

wilkerlucio12:02:07

I would avoid smart maps for this use case, doing large queries one attribute at a time isnt very efficient, and some queries will pay more than others

wilkerlucio12:02:25

I think there is some work to be done, like Grafitti to improve that story

wilkerlucio12:02:27

not on my radar at this time, but I can help if anyone wants to try that direction

jmayaalv19:02:08

Thanks for the input wilker. It's probably something we will need to implement at some point. I can start looking at it, is the graffiti code a good place to start ?

wilkerlucio00:02:55

@jmayaalv cool, I just started this discussion in the project, if anyone would like to bring points on how to make a GraphQL interface out of Pathom, we can concentrate resources here: https://github.com/wilkerlucio/pathom3/discussions/18

👍 4
markaddleman19:02:49

(please disregard the namespace name - I'm not sure this is a bug 🙂 )

markaddleman19:02:53

I have a resolver called "dimension-attributes" which can produce an array of raw data which another resolver formats into something - in this example, that formatting has trivial logic in the id-event-level resolver. Finally, I have a resolver field-list which reformats the data into something suitable for the pivot table UI widget

markaddleman19:02:24

The dimension-attributes resolver can produce an empty array

markaddleman19:02:33

When the dimension-attributes resolver produces an empty array, the reformatting resolver (field-list) is not invoked and, so, the field-list resolver is not invoked. This results in the wrong data sent to the UI

markaddleman19:02:01

I've tried various combinations of pco/? but I cannot find the right combination to get the result I'm looking for.

markaddleman19:02:07

Am I going about this the wrong way?

dehli23:02:24

the problem is when you return an empty array you’re not returning those keys (`:key`, :some , and :data) so pathom won’t be able to invoke the id-event-level resolver (which is how your field-list’s attribute-id input can be fulfilled). what would you like the response of (:>/input {:key "empty"}) to be?

dehli23:02:50

Instead of returning a vector would this work?

(pco/defresolver dimension-attributes [{:keys [key]}]
  {::pco/input  [:key]
   ::pco/output [{:attributes/dimensions [:key :some :data]}]}
  {:attributes/dimensions (when (not= key "empty")
                            {:key "key"
                             :some "a"
                             :data "b"})})

(pco/defresolver id-event-level [input]
  {::pco/input [(pco/? :key) (pco/? :some) (pco/? :data)]}
  {:attribute/id input})

;; Keep the rest the same
When you return an empty array, there’s nothing for pathom to iterate on which is the real issue with your original code (and why using a combination of (pco/?) probably wasn’t working).

markaddleman00:02:08

Thanks. I may have oversimplified the real use case. In real life, dimensions-attributes pulls data from a elastic search which may have zero, one or many documents that match a query

markaddleman00:02:55

I feel like there ought to be a solution on the field-list resolver by using pco/? but I can't find a magic combination to get the results that I want

👀 4
wilkerlucio00:02:54

@U2845S9KL just did tried here

wilkerlucio00:02:01

I believe this has the change you want:

(pco/defresolver field-list [input]
  {::pco/input  [{:attributes/dimensions [(pco/? :attribute/id)]}]
   ::pco/output [{:pivot-table/fields [:pivot-table.fields/list
                                       :pivot-table.fields/count]}]}
  {:pivot-table/fields {:pivot-table.fields/count (count (:attributes/dimensions input))
                        :pivot-table.fields/list  (mapv (comp (fn [raw] {:pivot-table.field/id raw}) :attribute/id) (:attributes/dimensions input))}})

wilkerlucio00:02:13

try with this resolver modified, and let me know if that's expected result now

markaddleman00:02:31

Thx. I'll give it a try right now

wilkerlucio00:02:16

ops. I just noticed this broken the first example ("not empty")

markaddleman00:02:00

I thought I had tried that 🙂

markaddleman00:02:16

It's as if pco/? is causing Pathom to give up too early

dehli00:02:59

Is it expected that we’d need to make it optional? I would think that {:attributes/dimensions []} would still fulfill:

[{:attributes/dimensions [:attribute/id]}]

wilkerlucio00:02:10

its different specifications

wilkerlucio00:02:26

I guess in his case, the list is mandatory, but the some parts of the list content are optional

wilkerlucio00:02:17

Pathom will only consider a sub-query valid if the required fields there are fulfilled

wilkerlucio00:02:28

in that sense, the Pathom behavior is correct in the initial example

wilkerlucio00:02:33

because the "empty" case can't fulfill the item detail

wilkerlucio00:02:03

altough, this seems a special case up to discussion, over what to do when the collection is empty

wilkerlucio00:02:11

in this case, should the input be considered valid, or invalid?

wilkerlucio00:02:05

@U2845S9KL changing from required to optional and stop working is for sure a bug, going to work on that in a bit

markaddleman00:02:42

Thanks! This is one of the last bits for me to switch completely to Pathom3.

markaddleman00:02:04

(to be clear, this is new functionality - I haven't tried this in Pathom2)

wilkerlucio00:02:25

no nested inputs in Pathom 2, you are living the edge here 🙂

dehli00:02:53

I have many comments in our pathom2 code about where we will benefit from nested inputs 🙂

wilkerlucio00:02:36

nice, and Im really asking your opinion here

wilkerlucio00:02:47

in case of empty collections, what you think would be the most sane default?

wilkerlucio00:02:52

consider the input valid, or invalid?

wilkerlucio00:02:11

(currently is invalid)

wilkerlucio00:02:22

and to make a case for valid, the @U2845S9KL code would work without need for optionality on the fields, if valid was the answer

dehli00:02:09

In my opinion an empty collection is valid b/c it does fulfill the contract. For every item in the collection (zero items) they have an :attribute/id

wilkerlucio00:02:20

yeah, I'm tending to that side too

wilkerlucio00:02:40

most nested inputs are mostly about aggregation, and aggregation on empty collections most commonly expected to be valid

markaddleman00:02:57

From my standpoint, it took a little bit of staring to understand why the empty collection was not triggering the id resolver.

markaddleman00:02:02

It would have been more natural if it had

wilkerlucio00:02:16

yeah, ok, expect that to change, soon 🙂

🙏 8
dehli00:02:08

awesome! what happens if some of the elements fulfill the nested input requirement but others don’t? I don’t have a real life example, but just curious

wilkerlucio00:02:19

in that case its considered a match, but some items may have missing requirements

wilkerlucio00:02:41

wonder if would be sane to filter it out

markaddleman00:02:33

From my standpoint, it would be sane to filter those out. I conceptualize pathom's behavior as a database JOIN operation. If some attributes don't exist on one side of the join, they aren't in the result set

☝️ 4
dehli01:02:11

ya, i also think filtering out those missing the required key is sane behavior. if you didn’t want them filtered out i’d imagine you could make the nested key an optional key instead of required

wilkerlucio01:02:51

I agree with you guys

wilkerlucio01:02:15

valid empty inputs just landed on master! @U2845S9KL you may wanna give a try if you still up 🙂

🚀 7
markaddleman01:02:39

right after dinner. Thanks!

wilkerlucio01:02:15

nested optional inputs fixed on master

🎉 8
markaddleman01:02:37

Works perfectly!

wilkerlucio02:02:21

you'r welcome 🙂

wilkerlucio02:02:41

and to finish this up today, nested input filtering just landed on master!

🎉 8
wilkerlucio02:02:07

so now in cases of collections and parts of the items are missing the requirement, these items are filtered out

markaddleman02:02:44

Yep. I just updated my code to remove pco/? . Makes it easier to understand, I think

wilkerlucio02:02:58

yeah, and is the correct expression in your case

dehli03:02:46

that’s awesome! thanks for the quick updates! excited to play around with the changes

wilkerlucio01:02:15

nested optional inputs fixed on master

🎉 8
wilkerlucio02:02:41

and to finish this up today, nested input filtering just landed on master!

🎉 8