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

[Proposal] map-of index should be part of :in during a walk #906

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

green-coder
Copy link
Member

(defprotocol LensSchema
  (-keep [this] "returns truthy if schema contributes to value path")

I think that the index in :map-of should be part of :in during a walk as it contributes to the value path, otherwise we don't differentiate between the key and the value.

@green-coder
Copy link
Member Author

Other proposal: to replace the 0 and 1 indices by something like ::m/key and ::m/val for this schema, similarly to how ::m/in is used for collections.

@ikitommi
Copy link
Member

ikitommi commented Jun 6, 2024

Thanks for reporting. I think this is even harder than that. There should be conversion from explain result of :in to :path and vice versa and that doesn't hold for :map-of.

(defn e! [schema value]
  (let [{:keys [schema] :as explain} (m/explain schema value)]
    (for [{:keys [path in] :as error} (-> explain :errors)]
      {:schema schema
       :error error
       :get-in (mu/get-in schema path)
       :path->in (mu/path->in schema path)
       :in->paths (mu/in->paths schema in)})))

(e! [:map ["kikka" :keyword]] {"kikka" "x"})
;({:schema [:map ["kikka" :keyword]],
;  :error {:path ["kikka"], :in ["kikka"], :schema :keyword, :value "x"},
;  :get-in :keyword,
;  :path->in ["kikka"],
;  :in->paths [["kikka"]]})

(e! [:map-of :keyword :keyword] {:kikka :a, :kukka "b"})
;({:schema [:map-of :keyword :keyword],
;  :error {:path [1], :in [:kukka], :schema :keyword, :value "b"},
;  :get-in :keyword,
;  :path->in [1], <-- this is wrong!
;  :in->paths []}) <-- this too

I think there are many ways to fix this. First that comes to mind is to make m/-explain contain both schema index (e.g. :path) + the value itself, wrapped in a vector. This should be possible as the :path is fully managed via the Schema instance, so it can write anything + read it back via the LensSchema protocol.

In this the above case, we would not conj just 1 to the path when explaining, but a vector of index + value instead, e.g. [1 :kikka]. This would be unwrapped in the LensSchema.

The example would return:

(e! [:map-of :keyword :keyword] {:kikka :a, :kukka "b"})
;({:schema [:map-of :keyword :keyword],
;  :error {:path [[1 :kukka]], :in [:kukka], :schema :keyword, :value "b"},
;  :get-in :keyword,
;  :path->in [:kukka]
;  :in->paths [[1 :kukka]})

would this work for you?

PR welcome.

@ikitommi ikitommi added the help wanted Help most welcome label Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Help most welcome
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

2 participants