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

Use JsonValue as JSON.parse Return Type? #10

Closed
joealden opened this issue Feb 20, 2023 · 10 comments
Closed

Use JsonValue as JSON.parse Return Type? #10

joealden opened this issue Feb 20, 2023 · 10 comments

Comments

@joealden
Copy link

Perhaps this would go further that what is wanted from this library, but I think it'd be nice if JSON.parse could return a type similar to (or the same as) JsonValue from type-fest (see https://github.com/sindresorhus/type-fest#json).

I've never used the optional reviver function in JSON.parse, but it looks like it can modify the return type beyond JsonValue, so perhaps it'd make sense to have a return type of JsonValue if no reviver is passed, then unknown if a reviver is passed?

@mattpocock
Copy link
Collaborator

I think unknown here is safer, but I'll leave this issue open!

@j

This comment was marked as off-topic.

@joealden
Copy link
Author

@j if the aim is type safety, that isn't safe

@mattpocock
Copy link
Collaborator

@j Hiding this as not relevant to the issue. Please also take a look at the readme, where I mention in detail the reason why we won't be implementing this.

@j
Copy link

j commented Feb 20, 2023

@mattpocock

true; understood. I live in a world where I consume self-owned APIs and messages that are type-safe at the point of having to parse it, so my brain went there.

with that said, leaving as unknown and forcing narrowing or casting is better.

@m10rten
Copy link

m10rten commented Feb 23, 2023

Same applies to JSON.stringify:reviver btw ^.^

@wmertens
Copy link

I'd like to see JsonValue when no reviver too. I can't see how it's less safe?

@mattpocock
Copy link
Collaborator

mattpocock commented Mar 1, 2023

OK - I think I prefer unknown here instead of JSONValue. The reason is that narrowing based on unknown is actually more productive. With JSONValue you actually lose a lot of value in the return type, preventing you from narrowing properly:

https://tsplay.dev/WKpQKw

@mattpocock mattpocock closed this as not planned Won't fix, can't repro, duplicate, stale Mar 1, 2023
@aaditmshah
Copy link

I created a PR, #123, for a type-safe and sound version of JSON.parse if anybody is interested. I also wrote a good description of the problem statement and the solution that I implemented.

@aaditmshah
Copy link

@mattpocock Consider the following playground example. With noUncheckedIndexedAccess you can see that the type of result.whatever is just JsonValue, but the type of result.awdkjanwdkjn is JsonValue | undefined. You may not get autocomplete for whatever and an error for awdkjanwdkjn, but this is the next best thing.

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

No branches or pull requests

6 participants