corka149 / jsonpatch

A implementation of RFC 6902 in pure Elixir.
MIT License
43 stars 11 forks source link

Support map atom keys in Jsonpatch.diff #24

Closed Valian closed 5 months ago

Valian commented 5 months ago

Is your feature request related to a problem? Please describe. Hi :wave: I'm building https://github.com/Valian/live_vue, a LiveView & Vue integration library. I'm checking if it's possible to minimize payload sent to the browser by sending JSON patches instead of whole objects.

I'm working with maps very often having atom keys.

Describe the solution you'd like I'd love to have diff working with atom keys. Eg.

Jsonpatch.diff(
  %{atom: [1, 2]}, 
  %{atom: [1, 2, 3]}
)

Currently I'm getting

** (FunctionClauseError) no function clause matching in Jsonpatch.escape/1    

    The following arguments were given to Jsonpatch.escape/1:

        # 1
        :atom

    Attempted function clauses (showing 2 out of 2):

        defp escape(fragment) when is_binary(fragment)
        defp escape(fragment) when is_integer(fragment)

    (jsonpatch 2.2.0) lib/jsonpatch.ex:253: Jsonpatch.escape/1
    (jsonpatch 2.2.0) lib/jsonpatch.ex:218: Jsonpatch.do_diff/5
    iex:1: (file)

Describe alternatives you've considered I can deeply convert all keys to strings, but I'm not sure if it's optimal?

I can try doing a contribution on my own, just wanted to align first before diving into the code. Maybe you could also give some hints?

Additional context Related issue about atom keys: https://github.com/corka149/jsonpatch/issues/8

corka149 commented 5 months ago

Hi @Valian

thx for the detailed report. Luckily the mentioned function is the only issue. The fix is on the way.

Valian commented 5 months ago

Wow thank you a lot! Such a quick resolution, much appreciated! 🙏🙌

corka149 commented 5 months ago

No problem! The fix was released as version 2.2.1.