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

Change the type of json from JSONValue to unknown #371

Merged
merged 7 commits into from
Dec 15, 2023

Conversation

josdejong
Copy link
Owner

BREAKING CHANGES IN THE TYPE DEFINITIONS

The JSONValue type is too limited, for example when using the lossless-json parser instead of JSON, there can be instances of LosslessNumber. Other parsers may generated BigInt instances, etc. Therefore, defining json as unknown is more "accurate".

So the following (core) type definitions:

export type TextContent = { text: string } | { json: undefined; text: string }
export type JSONContent = { json: unknown } | { json: unknown; text: undefined }
export type Content = JSONContent | TextContent

Will now become:

export type TextContent = { text: string }
export type JSONContent = { json: unknown }
export type Content = JSONContent | TextContent

Also, the type of JSONParser is changed from type JSONParser = JSON to an explicit description, which has one difference with JSON.stringify namely that it can return string | undefined instead of string. The current official type definition of JSON is lacking in this regard: JSON.stringify(undefined) returns undefined, but this is not reflected in the type, which can lead to a false sense of null-safety. See: https://stackoverflow.com/questions/74461780/is-the-official-type-definition-for-json-stringify-wrong.

All in all, this should result in a more smooth TypeScript experience and less need for type casting.

@josdejong josdejong merged commit dc4671a into main Dec 15, 2023
3 checks passed
@josdejong josdejong deleted the feat/remove-jsonvalue branch December 15, 2023 08:42
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 this pull request may close these issues.

1 participant