juji-io / editscript

A library to diff and patch Clojure/ClojureScript data structures
Eclipse Public License 1.0
485 stars 23 forks source link

diffing for nested MapEntry replaces the top level entry #20

Open den1k opened 3 years ago

den1k commented 3 years ago

This contains an update path:

(e/diff [1 [2 [3 4]]]
        [1 [2 [3 "🤔🤔🤔"]]])
=> [[[1 1 1] :r "🤔🤔🤔"]]

This replaces the top level MapEntry:

(ns user
  (:import (clojure.lang MapEntry)))

(e/diff (MapEntry/create 1 (MapEntry/create 2 (MapEntry/create 3 4)))
        (MapEntry/create 1 (MapEntry/create 2 (MapEntry/create 3 "🤔🤔🤔"))))
=> [[[] :r [1 [2 [3 "🤔🤔🤔"]]]]]

Running into this issue using https://github.com/metosin/malli#parsing-values where parts of the parse-tree are MapEntries.

huahaiy commented 3 years ago

Currently, a MapEntry is treated as a data value :val.

Is it desirable to treat it as a data type of its own? If the use case is common enough, we can do that.

den1k commented 3 years ago

Context for my case is that https://github.com/metosin/malli#value-transformation returns a sequence of MapEntries: https://github.com/metosin/malli/blob/4671ea3a670dda6289f550ff557e10abea799329/src/malli/core.cljc#L1747

I think it's desirable solely for the reason that it would be more correct the diffing for MapEntry is currently broken. If this is low effort and performance hit is negligible I think it would be great to have this.

huahaiy commented 3 years ago

Sounds reasonable. I will get to it when I get some time.

huahaiy commented 3 years ago

Maybe think about ways to make this extensible.