Skip to content
This repository has been archived by the owner on Oct 1, 2024. It is now read-only.

✨ Support use of onInitialValueChanged prop with fields having nested properties #464

Merged
merged 1 commit into from
Jan 11, 2019

Conversation

allyshiasewdat
Copy link
Contributor

@allyshiasewdat allyshiasewdat commented Jan 9, 2019

#446 introduced the onInitialValueChanged prop to <FormState />, allowing control over the form's reset behaviour when the value of the initialValues prop changed.

The problem

When onInitialValueChanged is set to reset-where-changed, only fields whose initial values had been changed are reset. However, !== was being used to check for changes rather than a deep equality check. This meant that for fields that had nested properties, if the field's initial value pointed to a new object but with unchanged values from the previous, <FormState /> would interpret that the field's initial value had changed, and would reset it.

The suggested fix

This PR introduces a deep equality check to support having complex fields for which we cannot rely on shallow equality. It also makes the reconciliation more consistent with the deep equality check performed by default when no value is specified for onInitialValueChanged.

Some concerns

  • Is there a legitimate use case for wanting to clear a field when an initial value is reassigned, even if the actual values don't change?
  • Would support for the intended use case here be better achieved through using a custom function for onInitialValueChanged?

@allyshiasewdat allyshiasewdat changed the title ✨ Support use of onInitialValueChanged prop with fields having nested properties ✨ Support use of onInitialValueChanged prop with fields having nested properties Jan 9, 2019
@allyshiasewdat allyshiasewdat self-assigned this Jan 9, 2019
@allyshiasewdat allyshiasewdat force-pushed the formstate-deep-equals-reconciliation branch from 24dbc51 to 4b68dc8 Compare January 9, 2019 22:23
Copy link
Contributor

@marutypes marutypes left a comment

Choose a reason for hiding this comment

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

👍

@allyshiasewdat allyshiasewdat force-pushed the formstate-deep-equals-reconciliation branch from 4b68dc8 to 81d4282 Compare January 10, 2019 16:25
Copy link
Contributor

@michenly michenly left a comment

Choose a reason for hiding this comment

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

LGTM!

@allyshiasewdat allyshiasewdat force-pushed the formstate-deep-equals-reconciliation branch from 81b718d to 998265e Compare January 11, 2019 17:09
@allyshiasewdat allyshiasewdat merged commit 17362ff into master Jan 11, 2019
@marutypes marutypes temporarily deployed to production January 11, 2019 19:36 Inactive
@michenly michenly deleted the formstate-deep-equals-reconciliation branch January 14, 2019 18:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants