Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

docs($resource): document Resource#toJSON #14725

Closed
wants to merge 1 commit into from
Closed

docs($resource): document Resource#toJSON #14725

wants to merge 1 commit into from

Conversation

jrvidal
Copy link
Contributor

@jrvidal jrvidal commented Jun 7, 2016

Document the toJSON method of Resource instances as part of its public API.

@gkalpak
Copy link
Member

gkalpak commented Jun 7, 2016

Thx for submit this! Could you expand a little bit on when toJSON is used and what it does? Maybe add a link to MDN.

@jrvidal
Copy link
Contributor Author

jrvidal commented Jun 7, 2016

Could you expand a little bit on when toJSON is used and what it does? Maybe add a link to MDN.

Will do 👍

I have just realized that toJSON does not remove $cancelRequest, so strictly speaking, it doesn't return a "simple object without the extra methods". Do you think it is pertinent to modify toJSON to do that, and if so, is it OK to do it in this PR?

@gkalpak
Copy link
Member

gkalpak commented Jun 7, 2016

toJSON is not a general purpose function - it's sole purpose is facilitating proper conversion to JSON. As such, it makes sense to do the least amount of work necessary. Functions are not serialized to JSON, so $cancelRequest is not deleted on purpose.

@jrvidal
Copy link
Contributor Author

jrvidal commented Jun 7, 2016

Gotcha, I will update the docs accordingly.

@jrvidal
Copy link
Contributor Author

jrvidal commented Jun 7, 2016

Notice however how this behavior means that the workaround I mentioned in #14637, where I tried to use a toJSON call as a way of getting "just the data", is not valid. In any case, documenting this method improves the situation so at least we know where we stand.

Document the `toJSON` method of Resource instances as part of its public API.

(#14637)
@jrvidal
Copy link
Contributor Author

jrvidal commented Jun 7, 2016

I rebased my commit with a better description.

petebacondarwin pushed a commit that referenced this pull request Jun 9, 2016
Document the `toJSON` method of `Resource` instances as part of its public API.

See #14637
Closes #14725
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants