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

feat(forms): improved state management and reacting to more changed props #2882

Merged
merged 4 commits into from
Nov 13, 2023

Conversation

henit
Copy link
Contributor

@henit henit commented Nov 10, 2023

There has been som limitations in the handling of changed external props, as well as the prioritization between different error sources. The different issues are closely linked together, so here are a summary of the changes:

  • Errors from Provider and local errors in field components could overwrite each other incorrectly because they where put in the same state. They now have separate states in useDataValue and both are taken into account when returning an error to display in the component (as part of the return value of the hook)
  • Depdendencies on external props (like errorMessages, schema and validator) in validator callbacks could lead to endless loops because they revalidated when something changed, creating a new error object and if the props was recreated (like if a field component did not memoize errorMessages), that would trigger re-validation giving a new error object and so on (endless loop). To avoid this problem, all state in both useDataValue and Provider are now put in useRef instead of useState, and a function check if validation leads to the same error that is already in the error state, and if so, it skips replacing it and re-rendering the component.
  • Some props was not monitored for external changes, or not re-validated so that a new value kept the error state from the previous value.
  • Custom hooks for mount effects (only run on mount) and update effects (not running on mount), with tests.
  • A custom hook ensure that race conditions does not cause problems. An example was when a unit test ran, the validation triggered by the first mount (validating the initial value) and then re-validating after a change callback, but some times the mount validation process finished after the change validation process, setting the error state based on the initial value, while a changed value was in the state. This is avoided using the new useProcessManager hook in useDataValue.
  • Rewritten validation code for better readability
  • Various small improvements and bugfixes

Copy link

vercel bot commented Nov 10, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
eufemia ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 13, 2023 1:25pm

Copy link

codesandbox-ci bot commented Nov 10, 2023

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 77c63d9:

Sandbox Source
eufemia-starter Configuration

@tujoworker tujoworker changed the title Improved state management and reacting to more changed props feat(forms): improved state management and reacting to more changed props Nov 10, 2023
@henit henit force-pushed the fix/forms-context-and-local-validation-sync branch from e59f2ac to 8c526bf Compare November 10, 2023 14:29
Copy link
Member

@tujoworker tujoworker left a comment

Choose a reason for hiding this comment

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

Well done 👏

const mask = useMemo(
() =>
omitMask
? [/\d/, /\d/, /\d/, /\d/, /\d/, /\d/, /\d/, /\d/, /\d/]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
? [/\d/, /\d/, /\d/, /\d/, /\d/, /\d/, /\d/, /\d/, /\d/]
? Array(9).fill(/\d/)

😄

@@ -33,7 +38,7 @@ function SelectCountry(props: Props) {
title={country.i18n[lang] ?? country.i18n.en}
/>
)),
[sharedContext.locale]
[lang]
Copy link
Member

Choose a reason for hiding this comment

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

👍

Copy link
Member

Choose a reason for hiding this comment

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

This change is not needed technically. But I'm fine with it.

@henit henit force-pushed the fix/forms-context-and-local-validation-sync branch from e7201ac to 77c63d9 Compare November 13, 2023 13:09
@henit henit merged commit 0ca9533 into main Nov 13, 2023
7 checks passed
@henit henit deleted the fix/forms-context-and-local-validation-sync branch November 13, 2023 15:16
tujoworker pushed a commit that referenced this pull request Nov 16, 2023
## [10.14.0](v10.13.0...v10.14.0) (2023-11-16)

### 📝 Documentation

* **Forms:** lists component specific props 1st ([#2892](#2892)) ([c05740c](c05740c))
* **PhoneNumber:** remove unsupported props ([#2894](#2894)) ([4800a8e](4800a8e))
* **PostalCodeAndCity:** remove unsupported props ([#2890](#2890)) ([fe1ee9e](fe1ee9e))

### ✨ Features

* **Forms:** expiry field ([#2660](#2660)) ([af5aa61](af5aa61))
* **forms:** improved state management and reacting to more changed props ([#2882](#2882)) ([0ca9533](0ca9533))
* **Input:** add clear button event "on_clear" ([#2898](#2898)) ([eb6b722](eb6b722))
* **Radio:** Sbanken styling ([#2888](#2888)) ([d7ffcf8](d7ffcf8))

### 🐛 Bug Fixes

* add "use client" to non hook components like the Button ([#2895](#2895)) ([2d54a13](2d54a13))
* **Autocomplete:** enhance logic for when to blur ([#2886](#2886)) ([ce5c3fa](ce5c3fa))
* **Autocomplete:** make clear button work with enter key ([#2901](#2901)) ([30007c4](30007c4)), closes [#2185](#2185)
* **FieldBlock:** enhance fieldset/legend detection ([#2902](#2902)) ([4c62052](4c62052)), closes [#2893](#2893)
* fix Flex and Grid export to work with Vite.js ([#2905](#2905)) ([ef83713](ef83713))
* fix vertical label_direction support for Radio group and ToggleButton group ([#2899](#2899)) ([d650c66](d650c66))
* **forms:** Field block error handling ([#2900](#2900)) ([9582c64](9582c64))
* **forms:** Improved message value replacements ([#2903](#2903)) ([a61140b](a61140b))
* **PhoneNumber:** keep selected countryCode value on blur ([#2869](#2869)) ([7e0f9c5](7e0f9c5))
@tujoworker
Copy link
Member

🎉 This PR is included in version 10.14.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

2 participants