Skip to content
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

Fix: Prevent undefined properties at the root level #3430

Merged
merged 2 commits into from
Feb 5, 2023

Conversation

heath-freenome
Copy link
Member

@heath-freenome heath-freenome commented Feb 4, 2023

Reasons for making this change

Fixes: #3424 by preventing the inclusion of undefined properties at the root level

  • Updated @rjsf/utils, making computeDefaults() helper in getDefaultFormState() to skip adding undefined values when allowEmptyObject is set.
    • Updated the getDefaultFormState() definition in the ValidatorType to add this new option
    • Updated the tests accordingly
  • Updated @rjsf/core, switching the excludeObjectChildren with allowEmptyObject in Form
  • Updated the documentation for getDefaultFormState() to add this new option
  • Updated the CHANGELOG.md accordingly

Checklist

  • I'm updating documentation
  • I'm adding or updating code
    • I've added and/or updated tests. I've run npm run test:update to update snapshots, if needed.
    • I've updated docs if needed
    • I've updated the changelog with a description of the PR
  • I'm adding a new feature
    • I've updated the playground with an example use of the feature

Comment on lines 216 to 220
passAsFalseForIncludeUndefinedValues.includes(
includeUndefinedValues
)
? false
: includeUndefinedValues
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of inlining this can we set it before L210?

          const nestedIncludeUndefinedValues = passAsFalseForIncludeUndefinedValues.includes(
              includeUndefinedValues
            )
              ? false
              : includeUndefinedValues
          // Compute the defaults for this node, with the parent defaults we might
          // have from a previous run: defaults[key].
          const computedDefault = computeDefaults<T, S, F>(
            validator,
            get(schema, [PROPERTIES_KEY, key]),
            get(defaults, [key]),
            rootSchema,
            get(formData, [key]),
            nestedIncludeUndefinedValues
          );

Also, couldn't

passAsFalseForIncludeUndefinedValues.includes(includeUndefinedValues) ? false : includeUndefinedValues

be simplified to this?

includeUndefinedValues === true

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, yes it could. Great simplification

Fixes: rjsf-team#3424 by preventing the inclusion of undefined properties at the root level
- Updated `@rjsf/utils`, making `computeDefaults()` helper in `getDefaultFormState()` to skip adding undefined values when `allowEmptyObject` is set.
  - Updated the `getDefaultFormState()` definition in the `ValidatorType` to add this new option
  - Updated the tests accordingly
- Updated `@rjsf/core`, switching the `excludeObjectChildren` with `allowEmptyObject` in `Form`
- Updated the documentation for `getDefaultFormState()` to add this new option
- Updated the `CHANGELOG.md` accordingly
  - Also added information the changelog for PR rjsf-team#3202
@heath-freenome heath-freenome merged commit bf07637 into rjsf-team:main Feb 5, 2023
@heath-freenome heath-freenome deleted the fix-3424 branch February 5, 2023 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

formData includes undefined properties
2 participants