-
Notifications
You must be signed in to change notification settings - Fork 349
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
Body.json reviver #104
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. P.S. I understand this is a polyfill, see whatwg/fetch#104
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 |
I understand, I was mislead by the polyfill using |
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. |
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. |
What is the processing model that you are proposing? |
@domenic your answer is dismissing this issue purely on implementation details. The idea that API How it is implemented is up to browsers. At "worse", a browser could have a C++ path with no argument and fallback to The point is:
The work-around Plus, some browsers might be able to offer more optimized implementations? An optimized C++ API is not as useful if it's unusable (using |
How is it the same core function?
|
@annevk 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. Unless you consider revivers an obsolete feature? It's rather useful so I hope you don't. |
I don't see why you think it doesn't justify a different API. |
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 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 Here's some fan-fiction: jods upgrades his old code. nope -- can't do. New APIs have a smaller feature-set than old ones. |
The rationale is that the parsing and object allocation can happen in parallel to some extent, which cannot happen if there's callbacks involved. |
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. People, who need a reviver, will not suddenly not need it anymore just because you don't expose an API for it. |
+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
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. |
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.
|
@saas2813 are there libraries that can perform JSON parsing incrementally (i.e., on a |
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:
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. |
@saas2813 Instantiating a user-supplied class would call its constructor, which is essentially the same as a callback. |
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. |
Body.json()
should take an optional parameter to be passed toJSON.parse(json[, reviver])
function to use a reviver callback to parse JSON data.See: JSON.parse(): Using the
reviver
parameter at the MDN.The text was updated successfully, but these errors were encountered: