-
-
Notifications
You must be signed in to change notification settings - Fork 736
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
[New] parse
: add allowSparse
option for collapsing arrays with missing indices
#312
Conversation
…ssing indices Fixes ljharb#181.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will definitely need some tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've rebased this for you.
As for PR requirements, this would also need the readme to be updated.
However, #181 is marked "question" because I'm still not convinced that it's a good idea to create sparse arrays for any reason, and I still believe that all the use cases listed in that issue are better solved by altering the field names.
Codecov Report
@@ Coverage Diff @@
## master #312 +/- ##
=======================================
Coverage 99.85% 99.85%
=======================================
Files 8 8
Lines 1348 1363 +15
Branches 164 166 +2
=======================================
+ Hits 1346 1361 +15
Misses 2 2
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've rebased this, and added stringify tests to match the parse tests that you added (thanks).
I continue to think producing sparse arrays is a bad idea, but since stringify accepts them, it seems useful to enable a round trip.
parse
: add allowSparse
option for collapsing arrays with missing indices
36250a4
to
b04febd
Compare
Fixes #181