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

Make Yojson dependency optional #22

Open
yallop opened this issue Aug 5, 2016 · 3 comments
Open

Make Yojson dependency optional #22

yallop opened this issue Aug 5, 2016 · 3 comments

Comments

@yallop
Copy link
Contributor

yallop commented Aug 5, 2016

Dynamic records are useful even without JSON serialisation.

It would be convenient to be able to use the records library without Yojson.

@emillon
Copy link
Contributor

emillon commented Sep 21, 2016

Hello,

I think that there are several points that may help use records without yojson:

  1. Remove Record.format. It is the only function that actually relies on yojson.
  2. Redefine the json type so that it is not necessary to depend on yojson just for the type.
  3. Make of_yojson and to_yojson arguments to Record.Type.make optional.

(1.) is straightforward and I'll deprecate the function in the next release (or maybe replace it with a opaque formatter compatible with ppx_deriving.show conventions).

I'm not sure (2.) is too useful, but does not hurt.

(3.) is a bit tricky to do in a safe way. My current plan is to change the type of Record.Type.make to:

val make:
  name: string ->
 ?to_yojson: ('a -> Yojson.Safe.json) ->
  ?of_yojson: (Yojson.Safe.json -> ('a, string) result) ->
  unit -> 'a t

And add an UnserializableType exception that gets raised when calling yojson functions on a type that has been built without converters (combinators such as list would need to cope with this too and build the resulting converters in a monadic way).

Another way could be to add a phantom type parameter like type ('a, with_json) t to type differently the Type.t with and without converter, but this may add too much complexity.

I'm not sold on either of this approaches, I'll have to try a bit.

Thanks for this suggestion!

@bbc2
Copy link
Member

bbc2 commented Sep 21, 2016

Is there a need for serialization to be in JSON? If not, maybe choosing an approach that makes serialization independent from the format would be nice, no?

@emillon
Copy link
Contributor

emillon commented Sep 21, 2016

A problem with that is that with the current approach, a reference to the serialization functions needs to be kept in each Field.t. I can see two approaches to do that:

  • add a type variable to Type.t so that it contains the type it can be serialized to. That pollutes the API, and not very flexible (since types can only serialize to one format)
  • move the concern of associating types to serializers away from Type and Field. That would mean passing a "polid -> serialization functions" mapping to converters. That seems difficult to maintain since you'll need to manually register converters for all the libraries you use.

For these reasons I think that sticking to json is good enough :)

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

No branches or pull requests

3 participants