Provide reviver instead of parseImmutable

[devsnek]

I couldn't find an issue about this, but it would be interesting if, instead of adding a new JSON.parseImmutable, we did something like JSON.parse(source, Record.JSONSourceReviver).

[nicolo-ribaudo]

I don't think that this is related to the parse-with-source proposal: we can provide a custom reviver even if that proposal doesn't happen, like we are doing in the polyfill.

Related: in #120 we are discussing semantics and use cases for the reviver function when using parseImmutable.

[devsnek]

@nicolo-ribaudo yeah fair enough. I would definitely be interested in champion's opinions on this then.

[ljharb]

so is the only purpose then of JSON.parseImmutable that you don't have to specify the reviver explicitly?

[littledan]

Part of this is ergonomics. This feature is very frequently requested. Another part is optimizability: engines can avoid generating the intermediate objects if this is a built-in function, and it may be harder if it's just a parameter to JSON.parse (may be possible to optimize if a reviver is provided as a built-in function, but probably causes an unfortunate performance cliff in that case).

[bakkot]

On the point of ergonomics, another major downside with only providing a reviver is that revivers do not compose easily, meaning that it would be difficult to simultaneously parse to immutable data structures and do any sort of custom reviving logic.

[devsnek]

I don't think there's any perf component to it, the engine can tell the reviver is Record.jsonReviver the same way it can tell the callee is JSON.parse.

I think the main question is how hard/easy it is to compose revivers. if it's difficult we probably shouldn't do this. if it's easy this pattern might be worth exploring. My understanding of the reviver API is that it should be pretty easy to compose them:

JSON.parse(s, (k, v) => reviver1(k, reviver2(k, v))

[bakkot]

The complexity with composing revivers is mostly around the rest of the API surface: that returning undefined results in omitting the property, and that the receiver is the parent object of this key/value pair (which doesn't necessarily make sense in an immutable context; see #120).

[devsnek]

Yeah the receiver is awkward I guess. I think this could still be an interesting path to take given interest in finding answers to the various problems. Of course if I'm the only one who finds this pattern worthwhile we could choose not to pursue it as well.

[rricard]

For the reasons listed by @littledan I think we want to keep JSON.parseImmutable as-is. If we think of something better or find an issue with the current reviver handling in the proposed spec change please ping me so I can reopen that issue or reopen a new issue!