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

Params for multiple instances of foreign resolver don't get merged properly #222

Open
jacobobryant opened this issue Dec 14, 2024 · 1 comment

Comments

@jacobobryant
Copy link

jacobobryant commented Dec 14, 2024

I'm using the graphql integration and did a query like [(:foreign-resolver {:a 1}) :local-resolver], where :local-resolver includes :foreign-resolver in its input. When :local-resolver was included in the query, the params weren't passed to :foreign-resolver. I tracked the issue down to pf.eql/merge-ast-children, which was being called by combine-foreign-ast. This fixed the issue (UPDATE: it caused other problems though):

diff --git a/src/main/com/wsscode/pathom3/format/eql.cljc b/src/main/com/wsscode/pathom3/format/eql.cljc
index 8726cee..e63f4a3 100644
--- a/src/main/com/wsscode/pathom3/format/eql.cljc
+++ b/src/main/com/wsscode/pathom3/format/eql.cljc
@@ -304,14 +304,18 @@
                      (update idx key merge-ast-children node)
                      (assoc idx key node))))
                idx
-               (:children ast2))]
+               (:children ast2))
+        params (merge (:params ast1) (:params ast2))]
     (-> (or ast1 ast2)
         (cond->
           (seq idx')
           (assoc :children (map-children->children idx'))
 
           (and (seq idx') (not (contains? #{:join :root} (:type ast1))))
-          (assoc :type :join))
+          (assoc :type :join)
+
+          (seq params)
+          (assoc :params params))
         (dissoc :query))))
 
 (defn merge-asts

Let me know if more info/setting up a PR/etc would be helpful.

@jacobobryant
Copy link
Author

Update: this started causing an exception in pcp/merge-ast-children when I load a certain page in our app. I see that function is comparing params for two nodes to ensure they're equal. So I can see the connection to my patch... perhaps there's a different place where the graphql params need to be merged.

clojure.lang.ExceptionInfo: Incompatible placeholder request {:source-node {:type :prop, :dispatch-key :ops.RealTimeDispatchResult/id, :key :ops.RealTimeDispatchResult/id, :params {:com.wsscode.pathom3.connect.operation/optional? true}}, :conflicting-node {:type :prop, :dispatch-key :ops.RealTimeDispatchResult/id, :key :ops.RealTimeDispatchResult/id}}
	at com.wsscode.pathom3.connect.planner$merge_ast_children$fn__56862.invoke(planner.cljc:1582)
        ...

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

No branches or pull requests

1 participant