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

Type safety for JSON serializable things #136

Closed
DLehenbauer opened this issue Sep 24, 2019 · 9 comments
Closed

Type safety for JSON serializable things #136

DLehenbauer opened this issue Sep 24, 2019 · 9 comments
Assignees
Labels
api area: dds Issues related to distributed data structures resolution: won't fix

Comments

@DLehenbauer
Copy link
Contributor

From Tyler's Soduku feedback:

Mistake #1: Trying to store objects in a DDS that are not safely JSON round-trippable

This is approximately the solution:

export type JsonPrimitive = undefined | boolean | number | string;
export type JsonObject = { [index: string]: JsonPrimitive | JsonArray | JsonObject };
export type JsonArray = (JsonPrimitive | JsonObject)[];
export type Json = JsonPrimitive | JsonArray | JsonObject;

As I've mentioned before (#3145), we should also call Object.freeze(..) on these objects.

@DLehenbauer
Copy link
Contributor Author

Typescript complains abt. the circular reference if I attempt to support nested arrays like this (e.g., [[[ . ]]]):

export type JsonArray = (JsonPrimitive | JsonObject | JsonArray)[];

(Perhaps @skylerjokiel, @anthony-murphy or @jack-williams have a trick.)

@anthony-murphy
Copy link
Contributor

it can be worked around with more interfaces, but recursive types are coming in typescript 3.7:
https://httptoolkit.tech/blog/5-big-features-of-typescript-3.7/

@jack-williams
Copy link
Contributor

Yeah until microsoft/TypeScript#33050 lands it has to be done with an interface:

export interface JsonArray extends Array<JsonPrimitive | JsonObject | JsonArray> {}

@DLehenbauer
Copy link
Contributor Author

Note: Json introduced in #165

@DLehenbauer
Copy link
Contributor Author

DLehenbauer commented Sep 28, 2019

The Jsonable definition works well for anonymous objects, but requires casting through 'unknown' in both directions when casting to/from a compatible interface. :-/

    interface ICell {
        text: JsonablePrimitive;
        cache: JsonablePrimitive;
    }

    private loadCell(row: number, col: number): ICell {
        return this.matrix.getItem(row, col) as unknown as ICell;
    }

    private storeCell(row: number, col: number, cell: ICell) {
        return this.matrix.setItems(row, col, [cell as unknown as Jsonable]);
    }

@jack-williams
Copy link
Contributor

Yeah this is abit of a pain. You can always do:

type ICell = {
    text: JsonablePrimitive;
    cache: JsonablePrimitive;
}

or

interface ICell {
    text: JsonablePrimitive;
    cache: JsonablePrimitive;
    [other: string]: unknown
}

The latter is probably more honest unless you're explicitly filtering out properties before serialising.

Could also do:

private loadCell(row: number, col: number): ICell {
    return this.matrix.getItem(row, col) as Pick<ICell, keyof ICell>;
}

private storeCell(row: number, col: number, cell: Pick<ICell, keyof ICell>) {
    return this.matrix.setItems(row, col, [cell as Jsonable]);
}

@okjodom
Copy link

okjodom commented Oct 18, 2019

tagging myself for future developments on this issue

@curtisman curtisman added the api label Jan 21, 2020
@curtisman curtisman added the area: dds Issues related to distributed data structures label Jan 31, 2020
@curtisman curtisman added this to the Future milestone Feb 2, 2020
anthony-murphy added a commit that referenced this issue Apr 20, 2020
This is basically taking advantage of duck typing. We are generating a type we know is jsonable from the input type, but setting anything not serializable to never, which will cause compile time issues with objects of the original type.

related #1816 #136 #363 #364
@curtisman curtisman modified the milestones: Future, November 2020 Oct 25, 2020
@curtisman curtisman modified the milestones: November 2020, December 2020 Nov 2, 2020
@curtisman curtisman modified the milestones: December 2020, January 2021 Dec 1, 2020
@danielroney danielroney modified the milestones: January 2021, Next Jan 7, 2021
@tylerbutler
Copy link
Member

We have had a lot of discussion about this over the past year, and some things were implemented, like AsJsonable. However, the general consensus has been that this requires too much "fancy type handling" and ultimately is more confusing than helpful.

The current plan is to close this, and possibly remove the things we put in place already with follow up issues.

@ghost
Copy link

ghost commented Feb 22, 2021

Because this issue is marked as "won't fix" and has not had activity for over 3 days, we're automatically closing it for house-keeping purposes.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api area: dds Issues related to distributed data structures resolution: won't fix
Projects
None yet
Development

No branches or pull requests

7 participants