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.

Sign up for GitHub

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

Jump to bottom

Jsonable to support interfaces #1816

Closed
wants to merge 1 commit into from
Closed

Jsonable to support interfaces #1816

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 8 additions & 1 deletion packages/runtime/runtime-definitions/src/jsonable.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ export type JsonableObject<T> = {

export type JsonableArray<T> = Jsonable<T>[];

export type JsonablePrimitiveArrayOrObject<T = JsonablePrimitive> = T | JsonableArray<T> | JsonableObject<T>;


/**
* Used to constrain a value to types that are serializable as JSON. The `T` type parameter may be used to
* customize the type of the leaves to support situations where a `replacer` is used to handle special values.
Expand All @@ -26,4 +29,8 @@ export type JsonableArray<T> = Jsonable<T>[];
* - Non-finite numbers (`NaN`, `+/-Infinity`) are also coerced to `null`.
* - (`null` always serializes as `null`.)
*/
export type Jsonable<T = JsonablePrimitive> = T | JsonableArray<T> | JsonableObject<T>;
export type Jsonable<T> = {
[P in keyof T]: T[P] extends JsonablePrimitiveArrayOrObject
Copy link
Contributor

Choose a reason for hiding this comment

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

i just pushed a PR for basically the same thing 😋 check it out #1823

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this might be too loose. Does it fail if an interface isn't jsonable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it covers all the scenarios I could think of

link to all the testing

Copy link
Contributor Author

@skylerjokiel skylerjokiel Apr 17, 2020 •
edited

Choose a reason for hiding this comment

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

Something something... great minds think alike. lol

I think I like this one because the consumption syntax is much simpler and only uses the one type:

function foo<T extends Jsonable<T>>(a: T): void { 
}

Copy link
Contributor

Choose a reason for hiding this comment

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

nested interfaces

Copy link
Contributor

Choose a reason for hiding this comment

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

i think we need type recursion to make this work

Copy link
Contributor

Choose a reason for hiding this comment

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

mine fails with that too

Copy link
Contributor

Choose a reason for hiding this comment

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

I approve of both. :-)

Slight preference for end-user ergonomics of the grand unified Jsonable<T>approach, if the results are comparable.

? T[P]
: Pick<T, P>;
};