elm / json

Work with JSON in Elm
https://package.elm-lang.org/packages/elm/json/latest/
BSD 3-Clause "New" or "Revised" License
90 stars 25 forks source link

Using == on Json.Decode.Values can return `True` for values that aren’t equal #30

Open lydell opened 3 years ago

lydell commented 3 years ago

SSCCE

> import Json.Encode as JE
> JE.object [ ("a", JE.list identity []) ] == JE.object []
True : Bool
> JE.object [ ("a", JE.object []) ] == JE.object []
True : Bool
> JE.object [ ("a", JE.null) ] == JE.object [ ( "a", JE.null ), ( "b", JE.null ) ]
True : Bool

Edit: This Discourse post by Evan explains that (==) shouldn’t be used with JSON values from elm/json and an idea for improving this in the future: https://discourse.elm-lang.org/t/function-equality/7538/24

Edit2: It’s even documented that one shouldn’t use (==) with JSON values: https://package.elm-lang.org/packages/elm/core/latest/Basics#equality 😅 But who reads the docs for (==), right?

harrysarson commented 3 years ago
> JE.object [ ("a", JE.list identity []) ] == JE.object []
True : Bool
> JE.object [ ("a", JE.object []) ] == JE.object []
True : Bool

These two happen because we (after some recursion into the objects) call __Utils_eq([], undefined, depth, stack). We do not enter the if here https://github.com/elm/core/blob/65cea00afa0de03d7dda0487d964a305fc3d58e3/src/Elm/Kernel/Utils.js#L34 because x is an object (y isn't). Then we look at all the properties of [] and there are not any so we say the two objects are equal.

The fix is to check if y is an object in https://github.com/elm/core/blob/65cea00afa0de03d7dda0487d964a305fc3d58e3/src/Elm/Kernel/Utils.js#L34.

> JE.object [ ("a", JE.null) ] == JE.object [ ( "a", JE.null ), ( "b", JE.null ) ]
True : Bool

This one happens because we ignore any properties of the right hand side that are not also properties of the left hand side [1].

The fix is to compare the number of properties of x and y (using an Object.keys polyfill probably).

[1]

The code was probably written only thinking about elm records where the compiler ensures the properties of the left and right hand sides are the same or elm custom types where either the properties are the same or the value '$' of will be different.

harrysarson commented 3 years ago

There is one more case

> JE.object [ ] == JE.list identity []
True : Bool

Maybe it is okay to say these two are equal? After all they have all the same properties (!) Otherwise we would need a defensive check Array.isArray(x) == Array.isArray(y) (polyfilled).

All of this does seem wasteful; we would pay a cost for every (not special cased by compiler) equality in elm to make Json.Encode.Value equality work. I suspect no one much uses Json.Encode.Value equality anyway although I have no data to back that theory up.