Jsonify correctness #410
Merged
Jsonify correctness #410
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- it now handles transformation of Number, String and Boolean to primitive counterparts - it correctly turns function and symbol to null as array members - it omits entries in objects where values are function, symbol or undefined; and where keys are symbols - it fails on BigInt with never
|
You need to fix the lint errors. |
What's the difficulty with supporting TypedArrays? |
Both |
|
Sorry for the lint errors point, I thought they were there before but I was mistaken. I fixed them all. I'll investigate the typed arrays and update the readme about limitations. |
|
@sindresorhus I think it should be ready for review. I fixed newly introduced lint errors as well. |
1 task
|
Thanks for contributing 🙏 |
skarab42
pushed a commit
to skarab42/type-fest
that referenced
this pull request
Jul 3, 2022
skarab42
pushed a commit
to skarab42/type-fest
that referenced
this pull request
Aug 16, 2022
skarab42
pushed a commit
to skarab42/type-fest
that referenced
this pull request
Aug 24, 2022
skarab42
pushed a commit
to skarab42/type-fest
that referenced
this pull request
Aug 24, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is my first attempt at fixing Jsonify problems I mentioned in #407.
I started with tests first, then proceeded to fix those tests. Tests are based on MDN docs and my own tests.
I had to delete a faulty test case, where an object had a function value. The spec tells that those entries are omitted from the output.
I checked the validity of the types with the provided
testscript. It also output a lot of lint warnings that were there before I picked it up, so I left them alone. I can fix those errors if you want me to, but it will end up destroying the git blame.I'm not sure if I documented everything I did as best as I could, since this was a first attempt. I can fix anything where you thing it needs more explanation.
I also commented out the Infinity and NaN checks. Infinity is detectable with this library, but it seemed like a minor edge case. NaN is not detectable as far as I can understand, see #406.
Also missing is
TypedArrays. I'm not sure how to approach them, any guidance is welcome, or we can get this merged and leave it as future work.I can also add a warning about NaNs in the readme, or leave it as future work as well.
Sorry for a lot of text, I just wanted to cover everything.
Fixes #407