Skip to content

Conversation

@ms1111
Copy link
Collaborator

@ms1111 ms1111 commented Oct 31, 2020

Fixes

#59 - JsonNode type

Description

Define explicit types for legal plain JSON values to avoid double-deserialization shenanigans.

Fixes #59.

Proposed changes in this PR

Adds JsonValue and JsonObject types to make ToJsonStrategy and FromJsonStrategy more specific.

Things to look at

  • Test coverage
  • Code Style
  • Documentation (READMEs etc)
@ms1111 ms1111 requested a review from hardy925 October 31, 2020 21:59
Define explicit types for legal plain JSON values to avoid double-deserialization shenanigans. Fixes #59.
@hardy613 hardy613 added documentation Improvements or additions to documentation v1.0.0 roadmap labels Nov 1, 2020
Copy link
Collaborator

@hardy613 hardy613 left a comment

Choose a reason for hiding this comment

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

I will probably pull this down to look into some things, but this is a great start! thanks @ms1111

Copy link
Collaborator

@ChrisDufourMB ChrisDufourMB left a comment

Choose a reason for hiding this comment

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

Looks great! Some comments around JsonValue vs JsonObject

@hardy925 hardy925 changed the base branch from support-ts-isolated-modules to develop November 4, 2020 08:38
@ms1111 ms1111 marked this pull request as ready for review November 8, 2020 04:10
hardy925
hardy925 previously approved these changes Nov 9, 2020
Copy link
Contributor

@hardy925 hardy925 left a comment

Choose a reason for hiding this comment

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

Thanks @ms1111 LGTM

@hardy925 hardy925 merged commit f50ed71 into develop Nov 9, 2020
@hardy925 hardy925 deleted the json-type branch November 9, 2020 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

5 participants