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

Documentation for JSON.Decode.Value is misleading #957

Closed
arowM opened this Issue May 8, 2018 · 3 comments

Comments

Projects
None yet
3 participants
@arowM

arowM commented May 8, 2018

In master branch, documentation about JSON.Decode.Value is as follows.

A JSON value.

But I guess it is misleading because Value is now not only for JSON objects.
(Though it is defined in JSON.Decode module...)
For example, it is also used for Html.Events.on to handle Event object, which is not exactly the JSON object.
In another instace, we use Value as an input/output type of ports to handle some JS objects that Elm cannot handle such as File object.
(In fact, I have not realized that Value can be used to pass File object to ports at first.)

P.S.
I'm not sure that this is the right place to report about JSON.Decode module because it seems to be moving to independent package, so if this is not the right place, I'll apologize.

@toraritte

This comment has been minimized.

Show comment
Hide comment
@toraritte

toraritte commented May 14, 2018

Value is originally defined in Json.Encode and it is only aliased in Json.Decode. On the other hand, Json.Encode.Value's documentation is more general by saying that it "represents a JavaScript value".

https://github.com/elm-lang/json/blob/master/src/Json/Encode.elm#L35
https://github.com/elm-lang/json/blob/master/src/Json/Decode.elm#L405

@arowM

This comment has been minimized.

Show comment
Hide comment
@arowM

arowM May 14, 2018

Thanks. So, how about using the same documentation as Json.Encode.Value to avoid confusion.

arowM commented May 14, 2018

Thanks. So, how about using the same documentation as Json.Encode.Value to avoid confusion.

toraritte referenced this issue in toraritte/json May 14, 2018

Update documentation for `Value` type alias
It aliases `Json.Encode.Value` therefore it would be prudent to use the same description. See issue at https://github.com/elm-lang/core/issues/957
@evancz

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz May 14, 2018

Member

Making the PR about it the canonical one for this

Member

evancz commented May 14, 2018

Making the PR about it the canonical one for this

@evancz evancz closed this May 14, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment