Fork me on GitHub
#graphql
<
2021-11-14
>
thumbnail10:11:51

I noticed a subtle difference in the wrap-error between lacinia <1.0 and the 1.1 alpha's; Previously multiple errors passed into wrap-error would give yield a WrappedValue with both errors:

(with-error "test" [{:message "first error"}
                    {:message "second error"}])
=>
#com.walmartlabs.lacinia.selector_context.WrappedValue{:value "test",
                                                       :behavior :error,
                                                       :data [{:message "first error"} {:message "second error"}]}
In the alpha's (specifically the latest commit d5a17b49e1cf46d77fc0e446396ca359752adce4) the behaviour is different:
(with-error "test" [{:message "first error"}
                    {:message "second error"}])
=>
#com.walmartlabs.lacinia.select_utils.WrappedValue{:value #com.walmartlabs.lacinia.select_utils.WrappedValue{:value "test",
                                                                                                             :behavior :error,
                                                                                                             :data {:message "second error"}},
                                                   :behavior :error,
                                                   :data {:message "first error"}}
Is this intentional? Because if so I'll adjust our internal lib to handle these cases differently.

thumbnail10:11:14

FWIW the original behaviour can be restored by using

(wrap-value value :error (map assert-error-map errors))
on https://github.com/walmartlabs/lacinia/blob/d5a17b49e1cf46d77fc0e446396ca359752adce4/src/com/walmartlabs/lacinia/resolve.clj#L58 Happy to submit a PR if desired btw, but not sure about the intend of the original change.

hlship20:11:18

Multiple error maps are a very rarely used feature; the new approach changes how they are represented but doesn't change the behavior (not even the order). Now, each WrappedValue has just a single error map and a few checks elsewhere for map vs. seq of maps goes away.

hlship20:11:03

Minor, but I was trying to hit everything that affects performance. Is there a problem with this approach w.r.t. your application or tests?

thumbnail21:11:22

There was an internal helper to build a relay nodes resolver out of multiple normal resolvers. It unwraps multiple resolver results (or values) into 1 resolver result. Merging the multiple (potential) error results too. The merging logic had to be changed now that the value doesn't contain a collection of errors anymore. Nothing big but we needed to update it to make sure our clients could handle both current and future versions of lacinia :)