JSON

[benjamingr]

Hi, I've noticed you use .toJSON() to convert to JavaScript objects.

JSON is a data serialization format, converting it toJSON would have been to also call .stringify on it.

The objects produced by this call are also not always valid JSON or are even encodable/decodable as JSON.

I would recommend renaming .toJSON. I don't have any particularly good ideas on what to though.

[fkling]

I also don't like this naming convention, but it actually has nothing to do with this library. toJSON is a special method that is used by JSON.stringify if it is available (spec). Built-in objects, like Date implement such a method as well.

[leebyron]

Yeah I will probably hide toJSON and expose toJS which similarly does a deep conversion to plain JS objects, but doesn't muck with JSON. —
Sent from Mailbox

On Wed, Jul 30, 2014 at 11:32 AM, Felix Kling notifications@github.com
wrote:

I also don't like this naming convention, but it actually has nothing to do with this library. toJSON is a special method that is used by JSON.stringify if it is available (spec). Built-in objects, like Date implement such a method as well.

---

Reply to this email directly or view it on GitHub:
#15 (comment)

[fkling]

@leebyron : This was actually more of a reply to @benjamingr :) Not sure what you mean by "hiding" in this case. The method should definitely exist to make JSON conversion easier.

I just dislike the choice of the method name in general ;)

[leebyron]

Hide as in not document beyond "supports JSON.stringify"—
Sent from Mailbox

On Wed, Jul 30, 2014 at 11:40 AM, Felix Kling notifications@github.com
wrote:

@leebyron : This was actually more of a reply to @benjamingr :) Not sure what you mean by "hiding" in this case. The method should definitely exist to make JSON conversion easier.

I just dislike the choice of the method name in general ;)

Reply to this email directly or view it on GitHub:
#15 (comment)

[benjamingr]

@fkling but .toJSON expects you to return a JSON string, that's what Date does in the spec.. this is also what a lot of libraries do with it. Returning an object just happens to work since it'll recursively run the serialization function again.

[fkling]

@benjamingr: No, Date.prototype.toJSON does not return a string containing JSON. It returns the value null or the return value of toISOString, which returns a string, but it doesn't contain JSON.

I'm relatively sure that .toJSON() is supposed to return a JSON encodable value, especially given how the spec process the return value. If you were to return JSON, you would get undesired results:

var foo = [{
  toJSON: function() { return JSON.stringify({bar: 42}); }
}];
JSON.stringify(foo); // "["{\"bar\":42}"]" instead of "[{"bar":42}]"

Again, I don't like the naming convention either, but it's not the fault of this library.

---

Other libraries / frameworks such as Backbone.js, do the exact same thing. I was about to create an issue about that, until I realized that .toJSON is just messed up from start.

[benjamingr]

It appears that you are right @fkling, this is what the proposal page says too. It's a poor naming choice.

[sophiebits]

The name toJSON is confusing, but I don't think it's so bad that we need a separate method toJS.

[leebyron]

toJSON is no longer described at all in the type definitions file, but continues to exist as an alias for toJS so that JSON.stringify still behaves correctly. I also changed fromJSON to fromJS to solve for the same ambiguity.

Thanks for highlighting this point of confusion, @benjamingr!

[yavuzmester]

Hi, toJS is confusing to me that we are writing in JS already. A class instance with methods is still a JS object, isn't it?