Clojurescript patch issue

[Folcon]

Just to check, is this lib supposed to work with clojurescript?

I've been tinkering with records and have managed to find an error if you diff/patch within a record. The diff is:

(edit/diff (map->TEST {:a 1 :b (map->TEST {:c 2})}) (map->TEST {:a 1 :b (map->TEST {:c 3})}))

Clojure:

(require '[editscript.core :as edit]
         '[editscript.edit :refer [edits->script]]
         '[clojure.edn :as edn])
(defrecord TEST [])

(let [last-state (edn/read-string {:readers {'example_ns.example_test.TEST map->TEST}} "#example_ns.example_test.TEST{:a 1, :b #example_ns.example_test.TEST{:c 2}}")
      updates [{:diff [[[:b :c] :r 3]]}]]
  (reduce (fn [prior {:keys [diff] :as edit}]
            (edit/patch prior (edits->script diff)))
          last-state
          updates))
#example_ns.example_test.TEST{:a 1, :b #example_ns.example_test.TEST{:c 3}}

Clojurescript:

(require '[editscript.core :as edit]
         '[editscript.edit :refer [edits->script]]
         '[clojure.edn :as edn])

(defrecord TEST [])

(let [last-state (edn/read-string {:readers {'example_ns.example_test.TEST map->TEST}} "#example_ns.example_test.TEST{:a 1, :b #example_ns.example_test.TEST{:c 2}}")
      updates [{:diff [[[:b :c] :r 3]]}]]
  (reduce (fn [prior {:keys [diff] :as edit}]
            (edit/patch prior (edits->script diff)))
          last-state
          updates))

#object[Error Error: No matching clause: val]

I could be doing something incorrectly, but not sure what. EDIT:

(let [last-state (edn/read-string {:readers {'example_ns.example_test.TEST map->TEST}} "#example_ns.example_test.TEST{:a 1, :b 2}")
      updates [{:diff [[[:b] :r 3]]}]]
  (reduce (fn [prior {:keys [diff] :as edit}]
            (edit/patch prior (edits->script diff)))
          last-state
          updates))
#object[Error Error: No matching clause: val]

Even non-nested doesn't work.

[Folcon]

Figured it out:

(extend-protocol IType
    TEST
    (get-type [_] :map))

I just needed to extend your protocols to my defrecord types.

It might be worthwhile documenting this, as it's pretty surprising behaviour, it also works perfectly fine in clojure, probably because clojure records type differently to clojurescript ones. (This extending either needs to be done on the clojurescript side or in the cljc file where the records are declared.)

[huahaiy]

This library is designed to work with plain Clojure/Clojurescript data literals.

I would not consider defrecord as plain data in general, as their semantics may go beyond list/vector/map/set that we currently support. As you have already figured out, you can still use this library if you extend your type to implement our protocols, like most libraries in the Clojure world allow. However, you have lost the type information in the process, as our library does not know anything about your type, i.e. patch will not return a TEST, but just a plain map.

In any case, if you want to preserve the type information, you can convert your type into regular Clojure/Clojurescript maps, e.g. {:type :test, ...}, However, marshaling/unmarshaling customized types into regular Clojure/Clojurescript data literals goes beyond the scope of this library, and I do not plan to add this functionality.

Philosophically, this library is fully onboard with the Clojure's data oriented programming ideology, and this ideology encourages universal data literals, and discourages specialized types. defprotocol, deftype, defrecord and so on are in the language mostly for implementation and performance purposes, i.e. for library authors. Most application programmer should use regular maps/vectors for the most part.

[Folcon]

That's fair, I'm not certain that the general perspective is that clear cut, but in general I'm fully on board with your philosophy =)...

It appears that I can utilise the library as is, the only reason I'm using records where I'd normally be using maps is performance considerations, at some point I'll be doing a profiling comparison of the current approach and the standard data literals, if the hit isn't too big, the intention is to swap =)...

Either way, thanks for writing the lib, it's been very helpful.

My only comment is that on the diffing side, I didn't realise how big a performance difference the quick algorithm made over the default A* one. It might be worth highlighting that depending on the usecase (data streaming), one should try the other first.