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

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

Open
lydell opened this issue Mar 30, 2021 · 2 comments

Comments

@lydell
Copy link

lydell commented Mar 30, 2021

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
Copy link

> 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
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants