fetch icon indicating copy to clipboard operation
fetch copied to clipboard

Body's json(): add reviver argument

Open heruan opened this issue 10 years ago • 20 comments

Body.json() should take an optional parameter to be passed to JSON.parse(json[, reviver]) function to use a reviver callback to parse JSON data.

See: JSON.parse(): Using the reviver parameter at the MDN.

heruan avatar Aug 05 '15 10:08 heruan

This doesn't make sense. The whole point of body.json() is that the parsing can happen entirely in C++ without calling back in to JavaScript. If you want to do the parsing in JavaScript, you should just use body.text() and JSON.parse it yourself.

domenic avatar Aug 05 '15 13:08 domenic

I understand, I was mislead by the polyfill using JSON.parse(). Why not extend the concept of reviver to the underlying mechanism? This would permit to instantiate JavaScript object from their JSON representation with some logic (e.g. dealing with Dates or returning an existing object instance where the JSON was already parsed before).

heruan avatar Aug 05 '15 15:08 heruan

The underlying mechanism is C++. Are you proposing allowing authors to supply C++ code to the browser to execute?

Again, just use .text() and JSON.parse.

domenic avatar Aug 05 '15 15:08 domenic

I'm proposing the idea of a way to manipulate the returned object; for example passing a JSON schema to the C++ function which will produce the JavaScript object for validation. For other uses, I will use Body.text() and JSON.parse().

heruan avatar Aug 05 '15 15:08 heruan

What is the processing model that you are proposing?

annevk avatar Aug 06 '15 13:08 annevk

@domenic your answer is dismissing this issue purely on implementation details.

The idea that API .json(reviver) should exist seems appealing to me.

How it is implemented is up to browsers. At "worse", a browser could have a C++ path with no argument and fallback to .text().then(json => JSON.parse(json, reviver)) when there's an argument.

The point is:

  • Body.json() is not consistent with JSON.parse(text[, reviver]). It's the same core function but it doesn't have the same API, which is surprising (in a bad way).
  • JSON being what it is, revivers are quite useful. E.g. to replace strings with Date, since in 2018 automatically serializing dates is still an unsolved problem; or to create typed objects based on $type.

The work-around fetch("api/hello").then(r => r.text()).then(text => JSON.parse(text, reviver)) is not very pretty, there's no reason that we shouldn't be able to do fetch("api/hello").then(r => r.json(reviver))

Plus, some browsers might be able to offer more optimized implementations?

An optimized C++ API is not as useful if it's unusable (using .text() instead) or if you have to post-process your object graph after it's been deserialized (which moots the performance win).

jods4 avatar Jun 06 '18 13:06 jods4

How is it the same core function?

  1. It doesn't share the name.
  2. It operates on a stream of bytes, not a string.
  3. It returns a promise for an object, not an object.

annevk avatar Jun 06 '18 13:06 annevk

@annevk Both methods are inside the core web platform and both parse serialized json into a JS object.

The key difference is (2) and implies (1) and (3): one operates on a stream of bytes (hence different name and returns a Promise) the other operates on a string.

The fact that their input is different doesn't justify that the API surface of parsing is different.

It's like saying: if you serialize JSON to a file you can choose some properties to omit, but not when you serialize JSON to a memory string where you can transform values, unlike when you serialize JSON to an HTTP body, in which case you can choose to transform camel-case to PascalCase.

Good API design is consistent and predictable. Wherever the web platform exposes JSON parsing, it should expose the same parsing options.

Unless you consider revivers an obsolete feature? It's rather useful so I hope you don't.

jods4 avatar Jun 06 '18 14:06 jods4

I don't see why you think it doesn't justify a different API.

annevk avatar Jun 06 '18 14:06 annevk

Same platform feature (JSON parsing), so default should be same/similar API and capabilities, see https://en.wikipedia.org/wiki/Principle_of_least_astonishment The justification is on you: why a different API?

The only justification I can see in this thread is "code is written in C++ so can't use a JS callback", which is not enough in my opinion. You can have a .json() C++ implementation and .json(reviver) in JS.

Here's some fan-fiction: jods upgrades his old code. "Hey there's this cool new API fetch to grab data from the server, it even has some deserialization built-in! I'm gonna replace my ugly XmlHttpRequest code (includes a reviver) with a one liner, so cool."

nope -- can't do. New APIs have a smaller feature-set than old ones.

jods4 avatar Jun 06 '18 14:06 jods4

The rationale is that the parsing and object allocation can happen in parallel to some extent, which cannot happen if there's callbacks involved.

annevk avatar Jun 06 '18 14:06 annevk

How does that preclude the platform from exposing a compatible API?

function cpp_json(body) { /* native optimised C++ code */ }

Body.prototype.json = function(reviver) {
  return reviver ?
    this.text().then(text => JSON.parse(text, reviver)) : 
    cpp_json(this);
}

There, you have a consistent API. It has a fast path for users not needing a reviver and an easy to use API for users that do.

People, who need a reviver, will not suddenly not need it anymore just because you don't expose an API for it. Instead they will get frustrated, not use your function and fallback to something else.

jods4 avatar Jun 06 '18 15:06 jods4

+1 for consistent API design. I found this issue when googling how to pass a reviver to json() and this made my eyes roll so hard it hurt

Again, just use .text() and JSON.parse.

The word just should be reserved for solutions that are actually obvious to most people. I would never have thought to use text() because I'm working with objects.

sliekens avatar Nov 10 '18 13:11 sliekens

Actually what is needed is not really the same callback. What is needed is a way to define transformations and Class assignments to do while parsing the JSON string. Something like:

  • Value matches this RegExp assign this class.
  • Value contains this tag assign this class.
  • Values matching this tag should not be included. Basicly an array of { matcher: "RegExp", regexp: "...", stdclass:"Date"}, { matcher: "Tag", tag: "@type", value:"Address", userclass:"Address"} { matcher: "Tag", tag: "internal", delete: "true"}

saas2813 avatar Apr 08 '19 07:04 saas2813

@saas2813 are there libraries that can perform JSON parsing incrementally (i.e., on a ReadableStream) while doing that which have seen somewhat non-trivial adoption? That'd make a good case for some kind of change to the standard library.

annevk avatar Apr 08 '19 07:04 annevk

I found this thread after attempts on stackoverflow. My #1 use of reviver is to reduce the size of the parsed object when working with very huge json files, eg:

(k,v) => (k==="a" || k==="b")? v : undefined; // discarding c, d, ...

It should be done easily on a stream. It is a pity to have to pass through response.text(), or to use the entire object after response.json() before removing all the useless properties. I don't understand why it should be so difficult to implement.

jeansoulin avatar Oct 27 '19 12:10 jeansoulin

@saas2813 Instantiating a user-supplied class would call its constructor, which is essentially the same as a callback.

bergus avatar Oct 27 '19 20:10 bergus

Why just don't traverse object after its creation? This way you always use optimized c++ for parsing but can you reviver to convert certain values to Date or BigInt objects. This won't use less memory for big json if someone skips some values, but at least it solves type conversion problem, with consistent syntax.

Somnium7 avatar Aug 02 '22 15:08 Somnium7

Ability to support parsing BigInts would be a huge benefit of this proposal (compared to other transformations supported via the Reviver mechanism), because without some sort of a solution big ints are practically lost when using Body.json()

pspraveenkr avatar Nov 24 '25 12:11 pspraveenkr

Now that TC39 has standardized reviver this is probably worth reconsidering.

There's a number of places that expose json() (Push API, File API, …) and we should probably update all of them if we decide to do this, making this a somewhat non-trivial undertaking.

annevk avatar Nov 24 '25 12:11 annevk