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

JSONValue type is overly wide #52

Closed
nedgar opened this issue Apr 3, 2023 · 10 comments
Closed

JSONValue type is overly wide #52

nedgar opened this issue Apr 3, 2023 · 10 comments
Assignees
Labels

Comments

@nedgar
Copy link

nedgar commented Apr 3, 2023

The type definition of JSONValue allows object and Array<object> (in addition to string | number | boolean) which seems overly wide.

Should it instead be: type JSONValue = string | number | boolean | JSONObject | Array<JSONObject>?

@vmidyllic
Copy link
Collaborator

vmidyllic commented Apr 6, 2023

can you suggest what are you going to achieve with such a change?
how does object work wrong for your use case?
We will have a detailed look on this problem.

@nedgar
Copy link
Author

nedgar commented Apr 6, 2023

@vmidyllic I'm not hitting it as a real issue, since my examples using it are valid JSON, but type object allows values that aren't supported in JSON, e.g. functions.

With the current typing a function is allowed:

image

With the type changed (corrected from my original comment) to type JSONValue = string | number | boolean | JSONObject | Array<JSONValue>, it flags the function as invalid:

image

@nedgar
Copy link
Author

nedgar commented Apr 6, 2023

Hrm, my proposed change doesn't allow interface types, dates, or other objects that implement toJSON().

image

image

Not allowing dates may be a feature, though, since they get serialized as an ISO 8601 string not as seconds from epoch.
I guess it depends what the intent is here. Do you want to allow any object that can be stringified to JSON? If not, how should it be restricted?

There's a good discussion here: microsoft/TypeScript#1897

@github-actions
Copy link

This issue is stale because it has been open for 14 days with no activity.

@github-actions github-actions bot added the stale label Apr 27, 2023
@vmidyllic
Copy link
Collaborator

@nedgar we add this task to the backlog. But it's not first priority.

@nedgar
Copy link
Author

nedgar commented Apr 27, 2023

No worries, it's not urgent at all, thanks.

@github-actions github-actions bot removed the stale label Apr 28, 2023
@github-actions
Copy link

This issue is stale because it has been open for 14 days with no activity.

@github-actions github-actions bot added the stale label May 13, 2023
@sanketsaagar
Copy link

Hi @nedgar , As our team has acknowledged that we have added this task to our backlog and it will be resolved, You may close the issue.

@github-actions github-actions bot removed the stale label May 18, 2023
@github-actions
Copy link

github-actions bot commented Jun 1, 2023

This issue is stale because it has been open for 14 days with no activity.

@github-actions github-actions bot added the stale label Jun 1, 2023
@github-actions
Copy link

This issue was closed because it has been inactive for 7 days since being marked as stale.

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

No branches or pull requests

4 participants