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

[RFC] Reduce the amount of different Yojson variants #105

Open
Leonidas-from-XIV opened this issue Jul 31, 2020 · 2 comments
Open

[RFC] Reduce the amount of different Yojson variants #105

Leonidas-from-XIV opened this issue Jul 31, 2020 · 2 comments
Labels
rfc Request for Comments on future direction

Comments

@Leonidas-from-XIV
Copy link
Member

Current state & Rationale

Yojson currently supports parsing into three different variants of JSON: Basic (which is mostly "normal JSON"), Safe (JSON extended with variants and tuples and making sure numbers don't get truncated; what most Yojson users seem to have settled on) and Raw (which parses numbers into strings and does not do string quoting).

This creates friction when different codebases use different types: library A has a hard time interoperating with library B if library A uses Basic.t and library B uses Safe.t.

Proposal

When picking Yojson for parsing JSON there is very little guidance on which of these variants to use. When parsing people want to parse JSON correctly into some mostly sensible format and not be confronted with minutia about different ways to parse JSON.

As such this RFC proposes to rethink the way the Yojson variants are divided up. There should be one variant that will fulfill 80% of peoples uses, presumably similar to what Safe is today. There should probably also be one that does as little interpretation of values as possible, e.g. never assuming numbers are `Int.

Reducing the amount of variants also makes it easier to remove code-generation via CPPO, which is currently preventing Merlin from working properly on the codebase and making reasoning over the functionality harder by adding a metalevel to the code.

The exact resulting variants are subject to discussion that I want to open up in this RFC.

Impact

Yojson.Safe.t is a type that is used by ppx_deriving_yojson and ppx_yojson_conv and changing the representation there might cause breakage. The upside is that fixing the PPXes will also automatically fix the users of it.

Roadmap

This would be planned for a 3.0.0 release at the earliest.

@Leonidas-from-XIV Leonidas-from-XIV added the rfc Request for Comments on future direction label Jul 31, 2020
@cemerick
Copy link
Contributor

Given the recent release, I wonder if the plan as stated here (tl;dr: re-center on Safe, removing Basic?) remains the plan?

@Leonidas-from-XIV
Copy link
Member Author

I think so, but for the fallout we'd need to check the revdeps and see how much breakage this will cause. After doing a couple of PRs to fix (json -> t) breakage at least the codebases that I have contributed to use mostly Safe, so it might not be too bad.

What would probably be the best is to create one release where Basic would be marked as deprecated so people can prepare. However I think at first we should tackle #104 given next to nobody uses these variants and most comments about this are positive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rfc Request for Comments on future direction
Projects
None yet
Development

No branches or pull requests

2 participants