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

Add option to use a custom comparator for dirty state checking #1296

Merged
merged 2 commits into from
Mar 3, 2020

Conversation

michaelmelanson
Copy link
Contributor

@michaelmelanson michaelmelanson commented Feb 24, 2020

Description

This change adds an option to the useField hook in react-form to use a custom comparator for checking whether a field is dirty. This allows you to, for example, apply a deep equals function for deep equality checking if you field requires it:

  const myField = useField<Type>({
    value: initialValue,
    validates: () => false,
    dirtyStateComparator: _.isEqual
  });

This is required to resolve a bug we have an in upcoming Web feature (Slack me for more details) where we are storing a list of objects in a field, and currently we remain in the dirty state if the user manually resets the form to its default state. With this feature, we correctly detect that the form is again in its default state, and report that it's now clean.

Note: I recommend turning on "Hide whitespace changes" when reviewing this feature to make the diff readable.

Type of change

  • react-form Minor: New feature (non-breaking change which adds functionality)

Checklist

  • I have added a changelog entry, prefixed by the type of change noted above

@michaelmelanson michaelmelanson force-pushed the michaelmelanson/deep-dirty-checking branch from 4d2af14 to 68ff637 Compare February 24, 2020 21:05
@michaelmelanson michaelmelanson marked this pull request as ready for review February 24, 2020 22:16
@michaelmelanson michaelmelanson force-pushed the michaelmelanson/deep-dirty-checking branch 3 times, most recently from cd996c9 to da0c883 Compare February 25, 2020 16:19
@michaelmelanson michaelmelanson changed the title Support using deep equality to check dirty state Add option to use a custom comparator for dirty state checking Feb 25, 2020
@michaelmelanson michaelmelanson force-pushed the michaelmelanson/deep-dirty-checking branch 2 times, most recently from 027044a to 6a0e114 Compare February 25, 2020 18:02
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.

One implementation question, otherwise looks good

Copy link
Contributor

@patsissons patsissons left a comment

Choose a reason for hiding this comment

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

just a few suggestions for minor tweaks from me, changes overall make sense.

packages/react-form/src/hooks/field/field.ts Outdated Show resolved Hide resolved
packages/react-form/src/hooks/field/field.ts Outdated Show resolved Hide resolved
packages/react-form/src/hooks/field/index.ts Outdated Show resolved Hide resolved
};
}
}
export function reduceField<Value>(
Copy link
Contributor

Choose a reason for hiding this comment

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

this and the shallowFieldReducer feel like they may belong somewhere else?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It feels like that to me too, but I just tried moving it out of that file and couldn't find a place that felt right because the call to makeFieldReducer would have to call back here anyway. 😕

packages/react-form/src/hooks/field/reducer.ts Outdated Show resolved Hide resolved
@michaelmelanson michaelmelanson force-pushed the michaelmelanson/deep-dirty-checking branch 2 times, most recently from 0914ebb to 55a3e9c Compare March 3, 2020 20:54
@michaelmelanson michaelmelanson force-pushed the michaelmelanson/deep-dirty-checking branch from 55a3e9c to e26f69f Compare March 3, 2020 20:59
Copy link
Contributor

@patsissons patsissons left a comment

Choose a reason for hiding this comment

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

LGTM

@michaelmelanson michaelmelanson merged commit d63d282 into master Mar 3, 2020
@alexandcote alexandcote deleted the michaelmelanson/deep-dirty-checking branch March 4, 2020 16:08
@alexandcote alexandcote temporarily deployed to production March 4, 2020 16:53 Inactive
@helloneele helloneele deployed to test-use-memoized-number-format March 6, 2020 19:25 Active
@TayKangSheng TayKangSheng deployed to expand_locale_support_for_shopify_address March 9, 2020 08:16 Active
@michenly michenly temporarily deployed to gem March 24, 2020 21:43 Inactive
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.

7 participants