-
Notifications
You must be signed in to change notification settings - Fork 2
feat: add preserve empty array option #148
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
feat: add preserve empty array option #148
Conversation
test/index.test.ts
Outdated
test('should not remove empty arrays when preserveEmptyArray is true', () => { | ||
expect(removeUndefinedObjects({ value: [] }, { preserveEmptyArray: true })).toStrictEqual({ value: [] }); | ||
expect(removeUndefinedObjects({ value: [undefined] }, { preserveEmptyArray: true })).toStrictEqual({ value: [] }); | ||
expect(removeUndefinedObjects({ value: [null] }, { preserveEmptyArray: true })).toStrictEqual({ value: [] }); |
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.
Can you add another case in here for an array with non-falsy values to assert that it is left alone. Also can you add some more examples for arrays in a nested object?
Can you also update the README for this new option? |
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.
echoing @erunion's feedback here on the test coverage and README docs, otherwise LGTM!
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 believe tests are failing due to my changes in #150, sorry about that 😅 mind fixing that up? running npm run prettier
locally should do the trick!
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'm happy with this, will defer to @erunion for final approval + merge!
README.md
Outdated
import removeUndefinedObjects from 'remove-undefined-objects'; | ||
|
||
console.log( | ||
removeUndefinedObjects({ key1: [], key2: [undefined], key3: { key4: 'a', key5: [] } }, { preserveEmptyArray: true }), |
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.
similar comment to here — can you add another example that's using the exact same object but with preserveEmptyArray
set to false so it illustrates the difference?
Published to v7.0.0 |
🧰 Changes
oas
repopreserveEmptyArray
option & separate logic to check if the current value in an object is an array.🧬 QA & Testing
preserveEmptyArray
in the removalOptions & pass in an object that has empty arrays, or arrays that contain undefined & null values