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

Comparing nested arrays stored in Json.Value can result in runtime error #945

Open
Widdershin opened this issue Feb 28, 2018 · 2 comments
Open
Labels

Comments

@Widdershin
Copy link

Widdershin commented Feb 28, 2018

Hello,

I work in a team that's using Elm and we're very happy with it, thank you all for your hard work.

We have encountered a runtime error when attempting to compare Json.value objects with different structures. After a bit of digging, it became clear that the underlying issue was around comparing arrays with different levels of nesting:

$ elm-repl
> import Json.Decode as Json
> Json.decodeString Json.value "[[1]]" == Json.decodeString Json.value "[]"
TypeError: Cannot read property '0' of undefined

I appreciate that comparing Json.value objects is not very good form, and we'll workaround this issue by not doing that. It's not desirable for us to decode these particular objects, as they can be very large and the Elm part of our application does not actually need to have a model of the contents.

In any case, it seems desirable to remove this class of runtime errors, since it seems quite feasible to do so.

The error is coming from the eqHelp function. It appears that extending the null check to cover undefined would resolve this issue.

I do not have currently have time to prepare a quality PR, but perhaps something like this?

-	if (typeof x !== 'object' || (x === null || y === null))
+	if (typeof x !== 'object' || (x === null || y === null || y === undefined))
 	{
 		typeof x === 'function' && __Error_throw(5);
 		return false;
         }

Unsure if this should apply to x as well, but I'm pretty sure the above fix would be sufficient to avoid this runtime error.

I hope this is enough to go on. Happy to answer any questions.

Elm version: 0.18
Tested on: Firefox and Chrome latest on Mac OS

@process-bot
Copy link

Thanks for the issue! Make sure it satisfies this checklist. My human colleagues will appreciate it!

Here is what to expect next, and if anyone wants to comment, keep these things in mind.

@Widdershin
Copy link
Author

Also just noticed another curiosity:

> Json.decodeString Json.value "[]" == Json.decodeString Json.value "[[1]]"
True : Bool

Should probably return False. Happy to open a separate issue if desired.

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

No branches or pull requests

3 participants