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

I think I found a bug in your Jsonify type? #466

Closed
Evertt opened this issue Sep 16, 2022 · 2 comments · Fixed by #497
Closed

I think I found a bug in your Jsonify type? #466

Evertt opened this issue Sep 16, 2022 · 2 comments · Fixed by #497
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@Evertt
Copy link

Evertt commented Sep 16, 2022

The title ends with a question mark, because I've literally never installed this package before. I was just reading the source code out of interest.

And while reading the code for Jsonify, I saw something that looked like a bug to me. Namely here:

https://github.com/sindresorhus/type-fest/blob/main/source/jsonify.d.ts#L103

At first I was like "oh really? I never knew that Map and Set are converted to empty objects by JSON.stringify(), that's interesting, let's try that!". And indeed:

image

However, that line that I linked to, is still a bug, I believe. Because the {} there is not representing an empty object value, but instead it represents the {} type, which as far as I know means any non-nullish type. So instead you should use your own EmptyObject type, don't you think?

@sindresorhus
Copy link
Owner

Seems like a bug to me. It was introduced in 8ca2959.

// @frontsideair

@sindresorhus sindresorhus added bug Something isn't working help wanted Extra attention is needed labels Sep 29, 2022
Evertt added a commit to Evertt/type-fest that referenced this issue Sep 30, 2022
@frontsideair
Copy link
Contributor

Yeah, sounds like a bug. It looks like there's already a PR addressing it, otherwise I'd be looking at it. Thanks for taking the initiative!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants