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] - JsonNode type #59

Closed
hardy925 opened this issue Oct 16, 2020 · 5 comments · Fixed by #61
Closed

[RFC] - JsonNode type #59

hardy925 opened this issue Oct 16, 2020 · 5 comments · Fixed by #61
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers Hacktoberfest Please see our CONTRIBUTING.md file as well! help wanted Extra attention is needed

Comments

@hardy925
Copy link
Contributor

hardy925 commented Oct 16, 2020

JsonNode type

add a type for JsonNode that consists of the correct types supported by the JSON Notation standard

text (see "3. Values")

Note: if updated please follow the Obsoleted by: tag in the header:
Screen Shot 2020-10-16 at 5 42 04 PM

Motivation

To properly type our strategic directions.

ToJsonStrategy and FromJsonStrategy make use of any, by providing this type we can set up directional strategies.

Design Detail

The supported RFC types should be | (Unioned) into a JsonNode type

/** Functions used when hydrating data */
export type FromJsonStrategy = (value: any) => any;

/** Functions used when dehydrating data */
export type ToJsonStrategy = (value: any) => any;

Becomes

/** Functions used when hydrating data */
export type FromJsonStrategy<T> = (value: JsonNode) => T;

/** Functions used when dehydrating data */
export type ToJsonStrategy <T>= (value: T) => JsonNode;
@hardy925 hardy925 added v1.0.0 roadmap enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed Hacktoberfest Please see our CONTRIBUTING.md file as well! labels Oct 16, 2020
@hardy925 hardy925 modified the milestone: Public Release Oct 18, 2020
@ms1111
Copy link
Collaborator

ms1111 commented Oct 31, 2020

This is how the lead architect of TypeScript suggests defining it with a recursive type reference, supported in TS 3.7+: microsoft/TypeScript#33050

type Json = string | number | boolean | null | Json[] | { [key: string]: Json };

TS 3.7 was released in Nov 2019.

Here is another solution he posted a few years back for earlier TS versions:

microsoft/TypeScript#3496 (comment)

type JSONValue = string | number | boolean | JSONObject | JSONArray;

interface JSONObject {
    [x: string]: JSONValue;
}

interface JSONArray extends Array<JSONValue> { }

@hardy613
Copy link
Collaborator

Hello @ms1111 👋

Thanks for looking into this (I had no idea TBH)

type Json = string | number | boolean | null | Json[] | { [key: string]: Json };

Is more along the lines of what I was thinking. Which now I am wondering about the object ({}) definition if we should use Record instead?

type Json = string | number | boolean | null | Json[] | Record<string, Json>;

@ms1111
Copy link
Collaborator

ms1111 commented Oct 31, 2020

Which now I am wondering about the object ({}) definition if we should use Record instead?

type Json = string | number | boolean | null | Json[] | Record<string, Json>;

Type alias 'Json' circularly references itself.ts(2456)

For some reason the Record throws it off in a way that { [key: string]: Json } doesn't.

What would you think about this:

export type JsonObject = { [key: string]: JsonValue };
export type JsonValue = null | boolean | number | string | JsonValue[] | JsonObject;

/** Functions used when hydrating data */
export type FromJsonStrategy<T> = (value: JsonObject) => T;

/** Functions used when dehydrating data */
export type ToJsonStrategy <T>= (value: T) => JsonObject;

... explicitly specifying the return type of ToJsonStrategy as JsonObject (an object) rather than just any JSON value. I think that's how it's always used, right? The top-level thing you serialize with ts_serialize has to be an object, not an array or a scalar?

@hardy925
Copy link
Contributor Author

I think that's how it's always used, right? The top-level thing you serialize with ts_serialize has to be an object, not an array or a scalar?

Yea, the expected usage is for authors to define how an object is serialized to JSON - this does make sense to me.

Thanks!

@hardy613
Copy link
Collaborator

actually, sorry, this is my mistake,

in this use case, we can expect an array or scalar value because we are not serializing the top level object here, this would be the description of how a specific property is serialized.

The top-level thing you serialize with ts_serialize has to be an object

is correct, however, at this point, we are describing the properties of said object.

ms1111 added a commit that referenced this issue Oct 31, 2020
Define explicit types for legal plain JSON values to avoid
double-deserialization shenanigans.

Fixes #59.
@ms1111 ms1111 mentioned this issue Oct 31, 2020
3 tasks
ms1111 added a commit that referenced this issue Oct 31, 2020
Define explicit types for legal plain JSON values to avoid
double-deserialization shenanigans.

Fixes #59.
ms1111 added a commit that referenced this issue Oct 31, 2020
Define explicit types for legal plain JSON values to avoid
double-deserialization shenanigans.

Fixes #59.
@GameBridgeAI GameBridgeAI deleted a comment from hardy613 Nov 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers Hacktoberfest Please see our CONTRIBUTING.md file as well! help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants