refactor: retain null values in array #149
Conversation
test/index.test.ts
Outdated
| expect(removeUndefinedObjects(obj)).toStrictEqual({ | ||
| d: [1234], | ||
| f: null, | ||
| g: [null, null], |
There was a problem hiding this comment.
yeah i guess i'll defer to @erunion on the expected default behavior here — should null values be filtered out of arrays by default? or should we add an add'l option for this?
i do agree that the current JSON.parse(JSON.stringify(obj)) behavior does occasionally yield unexpected results (which we kinda sorta account for):
|
Yea I just want to make sure there aren't any regressions from how this library behaves currently as we use it in a number of areas within the application. |
Yeah that was my concern as well and why I thought about cloning the functionality to the oas repo. To reduce the chance of regression, I agree with Kanad's approach on creating an optional option to preserve nulls. Setting the option on would use the custom remove undefined objects function, instead of stringifying, and not removing nulls in array. This will be set in |
There was a problem hiding this comment.
ok after giving it a further think through, i think this is technically fixing unexpected behavior and i'm ok with updating the default behavior as opposed to introducing another flag? based on the README, it looks like we are NOT supposed to remove null values:
i do share @erunion's concerns for regressions though, will defer to him on the final call!
README.md
Outdated
| ```js | ||
| import removeUndefinedObjects from 'remove-undefined-objects'; | ||
|
|
||
| console.log(removeUndefinedObjects({ key1: [null, undefined], key2: 123 }, { preserveArrayNulls: true })); |
There was a problem hiding this comment.
can you add another example that's using the exact same object but with preserveArrayNulls set to false so it illustrates the difference?
|
Update: Changed current approach to put preserving array nulls under an optional flag to reduce chances of regression. Happy to revert it back to make it the default behaviour as kanad pointed out. |
|
Published to v7.0.0 |
🧰 Changes
🧬 QA & Testing