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

[useForm]: eliminate tech debt #2668

Open
AlekseyManetov opened this issue Dec 2, 2024 · 4 comments
Open

[useForm]: eliminate tech debt #2668

AlekseyManetov opened this issue Dec 2, 2024 · 4 comments
Labels
Tech debt Internal tech debt
Milestone

Comments

@AlekseyManetov
Copy link
Collaborator

AlekseyManetov commented Dec 2, 2024

To Do

  1. Remove state from the ref approach
    Previously, we used refs to avoid closures inside hooks and to memoize the form API, ensuring that API callbacks were not recreated on every state change. This was necessary to prevent breaking user components wrapped in React.memo that rely on form API methods. We can consider migrating to native state with setState callbacks to always have access to the latest value without needing to include form values in hook dependencies. However, we first need to address the issue of closures in the Lenses implementation.

  2. Enable storing form state outside the useForm hook
    Add value and onValueChange props to allow external control of the form state. When these props are provided, the form should operate based on them, similar to the useFormState hook.

  3. Lens improvements:

    • Resolve closure limitations:
      The current Lens implementation creates lenses only on mount, providing getters tied to the initial state:

      const lens = useMemo(
          () =>
              new LensBuilder<T, T>({
                  get: () => formState.current.form,
                  set: (_, small: T) => {
                      handleFormUpdate(() => small);
                      return small;
                  },
                  getValidationState: getMergedValidationState,
                  getMetadata: () => getMetadata(formState.current.form),
              }),
          [],
      );

      If we move away from the ref-based approach, this will prevent accessing updated values outside closures.

    • Memoize lenses:
      Ensure lenses are memoized to prevent breaking components that receive lenses as props and are wrapped in React.memo.

See #2683

@Kuznietsov
Copy link
Collaborator

Kuznietsov commented Dec 2, 2024

I have a small request for improvements. While using lens, I've faced the need to access data within big while validating or updating small. WDYT, is it worth the effort?

@cpoftea
Copy link
Contributor

cpoftea commented Dec 2, 2024

I have a small request for improvements. While using lens, I've faced the need to access data within big while validating or updating small. WDYT, is it worth the effort?

@Kuznietsov Do you have an example?

@AlekseyManetov
Copy link
Collaborator Author

I have a small request for improvements. While using lens, I've faced the need to access data within big while validating or updating small. WDYT, is it worth the effort?

You can provide .onChange callback for the big lens and than make a small for this patched lens with your handler, like in this example:
https://uui.epam.com/documents?id=lenses&mode=doc&category=advanced#provide_your_own_setter
In this case your onChange callback we be invoked while small lens value was set and you will have access to the big lens data alongside.

For validation we also pass parent data to the customValidator callback in a second param, like in this example:
https://uui.epam.com/documents?id=form&mode=doc&category=components#complex_validation

This is what you are looking for or you have another cases?

@Kuznietsov
Copy link
Collaborator

Kuznietsov commented Dec 4, 2024

I have a small request for improvements. While using lens, I've faced the need to access data within big while validating or updating small. WDYT, is it worth the effort?

You can provide .onChange callback for the big lens and than make a small for this patched lens with your handler, like in this example: https://uui.epam.com/documents?id=lenses&mode=doc&category=advanced#provide_your_own_setter In this case your onChange callback we be invoked while small lens value was set and you will have access to the big lens data alongside.

For validation we also pass parent data to the customValidator callback in a second param, like in this example: https://uui.epam.com/documents?id=form&mode=doc&category=components#complex_validation

This is what you are looking for or you have another cases?

Thanks for the quick response.

Example:

Records in a table may be linked, and specific rules declare these relations. When the value is changed, there is a need to update the record's fields so as not to break the rules of relations or shift dates if the relation type is changed.

I've achieved this goal with big.onValueChange and in some case, with small.onValueChange((prev, current) => onValueChange(prev, current, big)), but I'd like to see some more idiomatic approach for the cases, when access to the other records is required, but there is no need for their modification.
It would prevent a loop over all records when only two specific ones are needed. This approach is very useful, when auto-filling fields, for example.

Maybe, something like:

small.onChange((prev, current) => {
      const relatedRecord = this.big().prop(current.relatedId).get();
      ....
      return updatedSmall;
});

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Tech debt Internal tech debt
Projects
Status: Backlog
Development

No branches or pull requests

3 participants