Skip to content
This repository has been archived by the owner on Jun 15, 2020. It is now read-only.

Replace json4s-ast with custom AST #62

Merged
merged 6 commits into from
Jun 24, 2016
Merged

Conversation

OlivierBlanvillain
Copy link
Collaborator

This PR replaces json4s-ast with a custom AST

  • The AST is in validation-json-ast/shared/src/main/scala/Ast.scala
  • This also and adds functions to do back and forth conversion with the play AST, and the native JavaScript AST
  • The vaW defined in the demo is now included in the lib
  • AstSpec cross compiles (which might be surprising)

- The AST is in `validation-json-ast/shared/src/main/scala/Ast.scala`
- This also and adds functions to do back and forth conversion with the play AST, and the native JavaScript AST
- The vaW defined in the demo is now included in the lib
- AstSpec cross compiles (which might be surprising)
)(User.apply, unlift(User.unapply))
}
implicit val format: Format[js.Dynamic, js.Dynamic, User] = Format(
Rule(j => User.format.validate(Ast.from(j))),
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Si tu as un meilleur idée pour ça je suis preneur.

- Change the build to publish-local validation, which will be required to compile the example until v2.0 is released
case JObject(value) => value.mapValues(to).toJSDictionary
case JNumber(value) =>
val d = value.toDouble; if (d.isNaN || d.isInfinity) null else d
}).asInstanceOf[js.Dynamic]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a big fan of this cast. It seems to be avoidable.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could model everything with js.Any, but then we got into the problem than String !<: js.Any, so we would have to cast anyways...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Equivalent code in circe, which goes through an input: Any: https://github.com/travisbrown/circe/blob/master/scalajs/src/main/scala/io/circe/scalajs/package.scala

I agree this could be reworked to look a bit nicer

@jto jto mentioned this pull request Jun 22, 2016
@coveralls
Copy link

coveralls commented Jun 23, 2016

Coverage Status

Coverage increased (+0.08%) to 82.358% when pulling e46f04b on OlivierBlanvillain:json-ast into 96d46ab on jto:v2.0.

@jto jto merged commit 14fb41c into jto:v2.0 Jun 24, 2016
@jto
Copy link
Owner

jto commented Jun 24, 2016

Just a little doc and we're good :)

@OlivierBlanvillain
Copy link
Collaborator Author

What do you think need documentation? Something like

We can only cross compile our own custom JSON AST cuz
scala/slip#28. To share Rule/Write
between different platforms you would need to cross compile them with the
shared AST, and comvert these to platform specific Rule/Write, for instance
by using the Rule[js.Dynamic, JValue] / Write[JValue, js.Dynamic]
provided here. See play-scalajs-example project for a complete
example.

?

On Fri, Jun 24, 2016 at 10:17 AM, Julien Tournay notifications@github.com
wrote:

Just a little doc and we're good :)


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#62 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/ABCPcYIPOS1RqnAClLbPWBzKuIcEMprOks5qO5KHgaJpZM4IruwN
.

@jto
Copy link
Owner

jto commented Jun 24, 2016

Yes :)

@OlivierBlanvillain OlivierBlanvillain deleted the json-ast branch June 24, 2016 08:28
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants