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

How should the toJSON method be used? #116

Closed
littledan opened this issue Dec 4, 2018 · 15 comments
Closed

How should the toJSON method be used? #116

littledan opened this issue Dec 4, 2018 · 15 comments
Assignees

Comments

@littledan
Copy link
Contributor

JavaScript's JSON.stringify function calls into the .toJSON() method on objects, if it exists, to find out how to serialize them. toJSON is inherently lossy: there is no way to figure out what type it came from originally, since it serializes just like some other value would've serialized. How should we deal with this?

Date

Date was the first class to have toJSON defined--it's the ISO 8601 representation.

Map/Set

Later, TC39 considered a proposal by @DavidBruant, presented by @ljharb, on a toJSON method for Map and Set. In the notes, there is skepticism from the committee, so the proposal did not move forward. JSON.stringify(new Set([1, 2, 3])) remains, uselessly, "{}". I was under the impression that this is our new pattern going forward.

BigInt

We discussed BigInt JSON serialization in tc39/proposal-bigint#24 . Based on the history with Map and Set, and worries about people misinterpreting things, BigInt adopted semantics of throwing on toJSON, to avoid silent bugs and misinterpretation. This hasn't been a very popular decision, e.g., as re-raised in this issue.

Temporal proposal

In the temporal proposal, objects have a JSON serialization which is like their toString behavior, roughly analogous to Date, as discussed at tc39/proposal-temporal#74 . I feel guilty about my participation here--I was pushing back on the toJSON method, but it seems like many people want the behavior, so maybe I have been mistaken. In this thread, an analogy with URL was pointed out.

URL and WebIDL support

URL originally lacked a toJSON method, but it was added in whatwg/url#137 after initial resistance.

To support URL's toJSON, WebIDL was changed from an unnecessarily complicated "serializer" specification to simply allowing specifications to overload toJSON in whatwg/webidl#323 . @tobie assembled a list of affected interfaces which implement toJSON.

Recommendation?

Where should we go from here? Should we develop a consistent pattern for toJSON overloading across JS as a whole, or does it make sense to make different decisions case-by-case? What's a good guideline for deciding?

@littledan
Copy link
Contributor Author

littledan commented Dec 4, 2018

It seems like it'd be ideal if we can come to a consensus among the people who participated in the decision-making processes on the specifications leading to this point, as well as the TAG, cc @annevk @domenic @tobie @allenwb @erights @ljharb @DavidBruant @pipobscure @kaizhu256 @gibson042

@domenic
Copy link
Member

domenic commented Dec 4, 2018

This is a good thing to discuss, and I don't have a fully formed opinion. Except, I think it's important to encourage toJSON() for "struct like" classes that are just data holders. Often these classes could be dictionaries, but need some slight additional tweak, such as being read-only, or having a convenience method or computed getter. (Example of the latter: DOMRect.) I think it's probably a good idea for these sorts of classes to behave as similar to POJSOs as possible. I.e., as if they were just defined as Web IDL dictionaries.

@ljharb
Copy link

ljharb commented Dec 4, 2018

Some of the problems with json are indeed toJSON, but also the usability of parse/stringify when you want customizations (iow, you basically don’t get any ergonomic customization on either). What we have with Map/Set is the worst possible outcome to me, because it leaves them useless for use with json - but the alternative (coming up with a meaningful output for toJSON for every new type of thing) is also unpalatable.

Is there any desire to improve the ergonomics of providing custom parsing and revivification at the top level parse/stringify operation that doesn’t suffer from the lossiness of JSON? If not, what should we do to ensure that new things aren’t just unusable in a world with JSON?

My opinion here is also very much not fully formed, but both “no toJSON at all on a new thing” and “let’s relitigate toJSON every time there’s a new thing” seem like undesirable outcomes :-/

@annevk
Copy link
Member

annevk commented Dec 5, 2018

I think if an object maps to something that can reasonably be expressed in JSON, it's fine to do so if there's demand. Set/Map seem like a bad fit as they can contain many things that cannot be expressed. BigInt can be expressed, though not in JavaScript's JSON, so if you care about roundtripping in JavaScript you might opt for a string there, which I guess is a little weird, so I can see not wanting to support it at all.

@littledan
Copy link
Contributor Author

I don't really see how the BigInt case and the URL case differ. You could represent 123n as "123" (with quotes) and be no more lossy than URL.

@annevk
Copy link
Member

annevk commented Dec 5, 2018

BigInt has a more ideal JSON serialization (number) if you don't care about roundtripping in JavaScript and URL doesn't.

@annevk
Copy link
Member

annevk commented Dec 5, 2018

So for BigInt it might make sense to consider an option to JSON.parse() to get BigInt out of numbers, perhaps (though maybe it's worth waiting on decimal support).

@DavidBruant
Copy link

My opinion remains the same as for Set and Map : i think most objects could have a reasonable default JSON representation (for sets and maps, [...mySet] and [...myMap] seemed appropriate to me especially as they can be re-feed to the corresponding constructor on the receiving end)
Super-accurate/specific representations can always be added as opt-in

The current situation where any JSON representation is opt-in doesn't seem to serve anyone's interest

My preference in order :

  • reasonable JSON representation by default
  • throw on toJSON()
  • current situation

@kaizhu256
Copy link

@annevk there's a proposal to give JSON.parse's reviver more contextual-arguments at https://github.com/gibson042/ecma262-proposal-JSON-parse-with-source, tho i suspect bigint is the only real motivation driving it.

My preference in order (least cognitive-load):

  • reasonable JSON representation by default
  • else fallback to "{}" (keeping precedent with regexp, map, set, ...)
  • hope no more primitives will be introduced to language (maybe exception for BigDecimal).

Map could've been represented as plain-object, had we disallowed [uncommon-use] non-string keys (and the pathological serialization issues it created).

@kaizhu256
Copy link

also if it isn't obvious; in keeping with above preference, i would want bigint represented as string with quotes rather than throwing:

JSON.stringify({ "number": 1234n })
// {"number":"1234"}

@plinss plinss added this to the 2020-05-04-week milestone Mar 30, 2020
@kenchris kenchris self-assigned this May 5, 2020
@a2sheppy
Copy link

I wonder. What's the important thing here? Getting an object of whatever type into JSON or serializing it into a textual form that can then later be reformed back into the original object? That seems to be a fundamental question that needs to be answered before any final decision is made. If an object can't be represented in JSON, it may be worthwhile to instead have a serialization method you can use to obtain a string representation that will correctly return the original object upon deserialization.

It would be ideal if everything could be represented exactly using JSON, but that doesn't appear to be a realistic goal for all types, so it may be worth considering the alternative.

@torgo torgo removed this from the 2020-07-06-week milestone Jul 7, 2020
@cynthia
Copy link
Member

cynthia commented Jul 7, 2020

@littledan we're sort of stuck here. We've been sitting on this for quite a bit, and have come to the realization that it's not easy to decide and recommend something just by ourselves due to this being blocked by missing functionality. What should we do here?

@annevk
Copy link
Member

annevk commented Jul 7, 2020

If there's a natural mapping to JSON and there's some precedent in "user space" we should add it. In general though I'd hold off, especially as TC39 doesn't seem to have broad agreement on this either. Otherwise we might end up doing things that turn out to later go against the JavaScript language.

@littledan
Copy link
Contributor Author

To clarify, nothing is blocked by this issue, since we're continuing to make case-by-case calls. The Temporal currently does contain toJSON methods that convert Temporal types to strings (following the pattern from Date), and the (much less mature) Decimal proposal would probably follow BigInt and throw on toJSON. If there are no concerns about this direction, then maybe this thread has served its purpose.

@torgo
Copy link
Member

torgo commented Jul 8, 2020

Ok we decided in the interest of trying to focus our energies we're going to close this. We can reopen if necessary later on.

@torgo torgo closed this as completed Jul 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests