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

JsonObject doesn't allow optionals #64

Closed
SamVerschueren opened this issue Oct 8, 2019 · 8 comments · Fixed by #65
Closed

JsonObject doesn't allow optionals #64

SamVerschueren opened this issue Oct 8, 2019 · 8 comments · Fixed by #65

Comments

@SamVerschueren
Copy link
Contributor

SamVerschueren commented Oct 8, 2019

I want to type something as JsonObject, because the interface defines a request from an API.

So then I use extends JsonObject, but my API has some optionals.

interface Unicorn extends JsonObject {
	unicorn: string;
	rainbow?: string;
}

The compiler will fail with

Property 'unicorn' of type 'string | undefined' is not assignable to string index type 'JsonValue'

The reasoning behind this is that undefined is not serializable and doesn't exist in JSON. But how would someone solve this issue? Or should we change something in the JSON definitions of type-fest?

A possible solution would be to allow undefined in JsonObject.

export type JsonObject = {[key: string]: JsonValue | undefined};

This allows me to add optionals to something which is a JsonObject.

interface Unicorn extends JsonObject {
	unicorn: string;
	rainbow?: string;
}

To me, this kinda makes sense because JSON.stringify({unicorn: '🦄', rainbow: undefined}) results in {"unicorn":"🦄"}, thus allowing undefined in JSON like objects.

@SamVerschueren
Copy link
Contributor Author

We could also add undefined to JsonValue. But wasn't sure about that approach.

@yxliang01
Copy link
Contributor

@SamVerschueren I think it's weird to add undefined to JsonValue since in JSON, we only have null but no undefined. From my understanding, JsonObject is good for casting JSON.parse return value but not input to JSON.stringify because a non-JSON directly translatable object can be stringified well. E.g. a custom object implementing their own toJSON() method.

@SamVerschueren
Copy link
Contributor Author

Adding undefined to JsonValue isn't my preferred approach either. But currently, the use case explained in the docs of JsonObject itself, isn't very usuable. The extends JsonObject doesn't allow optional properties which enforces you to do something like extends Partial<JsonObject> which is meh... That's why I think adding | undefined to the JsonObject definition could make sense.

@yxliang01
Copy link
Contributor

Then, how about simply make all the keys optional? @SamVerschueren it's pretty clear in this way I think.

@SamVerschueren
Copy link
Contributor Author

By doing export type JsonObject = {[key: string]: JsonValue | undefined}; the keys become optional. You can't do [key: string]?: JsonValue if I'm not mistaken.

@yxliang01
Copy link
Contributor

yxliang01 commented Oct 8, 2019

@SamVerschueren Nope you can do export type JsonObject = {[key in string]?: JsonValue};. Tested under TypeScript 3.6.3 . Although this one will have the same effect with the one you wrote, this is clearer I think.

EDIT: weirdly, export type JsonObject = {[key: string]?: JsonValue}; doesn't work as you suggested. Anyone knows the difference between these two?

@SamVerschueren
Copy link
Contributor Author

SamVerschueren commented Oct 9, 2019

Another thing with JsonObject is that actually, the use case it mentions, extends JsonObject is not best practice.

The reason is that JsonObject has a index signature. So if you want to make something a JsonObject, you can do this

interface Unicorn extends JsonObject {
	unicorn: string;
	rainbow: string;
}

So now we can create objects following this contract

const unicorn: Unicorn = {
	unicorn: '🦄',
	rainbow: '🌈'
};

But because JsonObject is a index signature, we can remove the rainbow property from the interface and it will still work because it falls back to the index signature. We can off course also add extra properties to Unicorn, which we don't want either.

I made quite a few changes to the JsonObject/JsonValue types which work in the opposite direction.

// Made all these types generic
type JsonObject<T = any> = {[key: string]: JsonValue<T>};
interface JsonArray<T = any> extends Array<JsonValue<T>> { }
type JsonValue<T = any> = string | number | boolean | null | Pick<JsonObject<T>, Extract<keyof T, string>> | JsonArray<T>;

// EXAMPLE

interface Unicorn {
	unicorn: string;
	rainbow: string;
}

const unicorn: Unicorn = {
	unicorn: '🦄',
	rainbow: '🌈'
};

const jsonObject: JsonObject<Unicorn> = unicorn;

It will fail if you change the rainbow type to something like Date because Date is not supported by JsonValue.

This works much better than using extends JsonObject for us because we still have very strict typings now.

I'm not saying that these changes should be made to type-fest though. It's just how we solved the problem we had. There might be perfect valid use cased for the current implementation.

@sindresorhus
Copy link
Owner

The reason is that JsonObject has a index signature. So if you want to make something a JsonObject, you can do this

Is there a way we can make it not have an index signature?

This works much better than using extends JsonObject for us because we still have very strict typings now.

Are you using your generic JSON type as input type or return type?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants