Skip to content

[node-core-library] Add ignoreUndefinedValues option to JsonFile serialization APIs #2696

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

Merged
merged 6 commits into from
May 18, 2021

Conversation

dmichon-msft
Copy link
Contributor

@dmichon-msft dmichon-msft commented May 13, 2021

Summary

Adds a configuration option ignoreUndefinedValues to the JsonFile serialization APIs to make their behavior consistent with JSON.stringify() for the common case of setting values to be discarded as undefined. This is generally preferred because the delete operator is notoriously bad for performance.

Details

The underlying APIs already drop keys with undefined values, the JsonFile class added an extra validation layer in front of serialization that turned this standard behavior into an error. Passing ignoreUndefinedValues: true in the options simply turns off this extra validation and defers to JSON.stringify() or jju.stringify().

How it was tested

Added unit test cases for keys with undefined as their value and the ignoreUndefinedValues: true option.

Edited: dropUndefinedValues -> ignoreUndefinedValues

@iclanton iclanton changed the title Add dropUndefinedValues option to JsonFile serialization APIs [node-core-library] Add dropUndefinedValues option to JsonFile serialization APIs May 17, 2021
@iclanton
Copy link
Member

This looks good to me, but I like @D4N14L's suggested property name better.

@dmichon-msft dmichon-msft changed the title [node-core-library] Add dropUndefinedValues option to JsonFile serialization APIs [node-core-library] Add ignoreUndefinedValues option to JsonFile serialization APIs May 18, 2021
@iclanton iclanton enabled auto-merge May 18, 2021 19:21
@iclanton iclanton merged commit 3e9aa33 into microsoft:master May 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants