wilkerlucio / pathom3

Interface with complex data via graph mapping.
https://pathom3.wsscode.com
Eclipse Public License 2.0
376 stars 31 forks source link

Different `pco/params` behavior in Clojure vs. ClojureScript #211

Closed dehli closed 2 months ago

dehli commented 11 months ago

Hey @wilkerlucio 👋 Hope you're doing well!

I've been away from pathom for a little bit but I'm revisiting it again and I ran into a bug around pco/params, ClojureScript, and the parallel parser. Below is a minimal reproducible example (full example is located here) showcasing what I'm seeing. It looks like when :email/body is passed into the email-valid? resolver it is "resetting" the params. Will follow up here if I make any progress on it!

(ns dev.dehli.pathom3.missing-params
  (:require [com.wsscode.pathom3.connect.indexes :as pci]
            [com.wsscode.pathom3.connect.operation :as pco]
            [com.wsscode.pathom3.interface.async.eql :as p.a.eql]
            [promesa.core :as p]))

(pco/defresolver email-body [env _]
  {:email/body (if (:text? (pco/params env))
                 "Hello there"
                 "<h1>Hello there</h1>")})

(pco/defresolver email-subject []
  {:email/subject "Clojure Rocks"})

(pco/defresolver email-valid?
  [{:email/keys [body subject]}]
  {:email/valid? (and (some? body) (some? subject))})

(def env
  (-> (pci/register [email-body email-subject email-valid?])
      (assoc ::p.a.eql/parallel? true)))

(comment
  (letfn [(process [tx]
            (-> env
                (p.a.eql/process tx)
                (p/then prn)))]
    (p/do
      (process [:email/body
                :email/valid?])

      (process `[(:email/body {:text? true})
                 :email/valid?])))

  ;; Clojure (correct)
  ;; #:email{:body "<h1>Hello there</h1>", :valid? true}
  ;; #:email{:body "Hello there", :valid? true}

  ;; ClojureScript (incorrect)
  ;; #:email{:body "<h1>Hello there</h1>", :valid? true}
  ;; #:email{:body "<h1>Hello there</h1>", :valid? true}
  )
wilkerlucio commented 2 months ago

Hi @dehli ! I'm doing well, how about you? Sorry for the long due here, just got to it. The issue that the inputs for :email/valid? ends up in a different part of the graph than the direct request to :email/body:

CleanShot 2024-08-24 at 02 52 55@2x

I notice this graph is optimizable (the first end part is a subset of the second, so this could be merged).

But for now, and to make the params part for resilient, I'm doing a post-process on the graph to ensure all equal resolvers have the same params, so we can avoid the issue of different params on different parts completely.

dehli commented 2 months ago

Hey @wilkerlucio, no need to apologize! Happy to hear you're doing well! Doing good over here too! That makes a lot of sense. I appreciate the fix! Will get latest pulled down. Have a good week!