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(core): add field listeners #801

Closed
wants to merge 1 commit into from

Conversation

ha1fstack
Copy link
Contributor

@ha1fstack ha1fstack commented Jun 30, 2024

from discussion #709

Background

Sometimes we may want to attach our own side effects to the field when some event happends. For example, resetting another field if some field value has changed.

Attaching these 'listeners' inside the validator does not work as intended because validator does not guarantee if it was triggered by the exact event. (eg. all validators run when form submits)

It's of course possible to implement but it's not very clean and the concerns become scattered.

Proposal

This PR adds listeners api to the field for easy handling of forementioned use cases.

onChange and onHandleChange are differentiated because onHandleChange captures the 'manual' changes to the field. (eg. user input) while onChange also captures programmatic changes. (eg. setValue)

Thoughts

I'm not certain about the whole idea or implementation, open to suggestions.

@ha1fstack ha1fstack marked this pull request as ready for review June 30, 2024 22:02
@ha1fstack ha1fstack marked this pull request as draft June 30, 2024 22:02
@zaosoula
Copy link

zaosoula commented Jul 2, 2024

This feature will be useful +1. We are currently hacking around validators by it not practical

@ha1fstack ha1fstack changed the title feat: add field listeners feat(core): add field listeners Jul 3, 2024
@ha1fstack
Copy link
Contributor Author

ha1fstack commented Jul 3, 2024

This feature will be useful +1. We are currently hacking around validators by it not practical

@zaosoula Bit clumsy code, but in the meantime you can do this:

import { DeepKeys, DeepValue, FieldApi, functionalUpdate, Validator } from '@tanstack/form-core';

export function attachOnChangeToField<
  TParentData,
  TName extends DeepKeys<TParentData>,
  TFieldValidator extends Validator<DeepValue<TParentData, TName>, unknown> | undefined = undefined,
  TFormValidator extends Validator<TParentData, unknown> | undefined = undefined,
  TData extends DeepValue<TParentData, TName> = DeepValue<TParentData, TName>
>(
  field: FieldApi<TParentData, TName, TFieldValidator, TFormValidator, TData>,
  onChange: (props: {
    value: DeepValue<TParentData, TName>;
    fieldApi: FieldApi<TParentData, TName, TFieldValidator, TFormValidator, TData>;
  }) => void
) {
  const handleChange: typeof field.handleChange = updater => {
    onChange({
      value: functionalUpdate(updater, field.store.state.value),
      fieldApi: field,
    });
    field.handleChange(updater);
  };

  return {
    ...field,
    handleChange,
  } as typeof field;
}

export function AttachOnchangeToField<
  TParentData,
  TName extends DeepKeys<TParentData>,
  TFieldValidator extends Validator<DeepValue<TParentData, TName>, unknown> | undefined = undefined,
  TFormValidator extends Validator<TParentData, unknown> | undefined = undefined,
  TData extends DeepValue<TParentData, TName> = DeepValue<TParentData, TName>
>(props: {
  field: FieldApi<TParentData, TName, TFieldValidator, TFormValidator, TData>;
  onChange: (props: {
    value: DeepValue<TParentData, TName>;
    fieldApi: FieldApi<TParentData, TName, TFieldValidator, TFormValidator, TData>;
  }) => void;
  children: (field: FieldApi<TParentData, TName, TFieldValidator, TFormValidator, TData>) => JSX.Element;
}) {
  const { field, onChange } = props;
  const newField = attachOnChangeToField(field, onChange);

  return props.children(newField);
}
<form.Field name="name">
  {field => (
    <AttachOnchangeToField
      field={field}
      onChange={({ value, fieldApi }) => { ... }
    >
      {field => ...

@crutchcorn
Copy link
Member

crutchcorn commented Jul 3, 2024

@zaosoula this would be helpful to understand a practical use case :) Can you share yours?

@zaosoula
Copy link

zaosoula commented Jul 4, 2024

@crutchcorn

Screenshot 2024-07-04 at 09 48 09

Here we are using onChange to dynamically set a checkbox to true if the selected value match a list

Screenshot 2024-07-04 at 09 49 11

Here we are using onChangeAsync to open a modal to create a new entry for a sub-relation

Each time we have to return null or an empty string as we do not want to show errors; using the validators property is also confusing for new developper reading our code as the functions above are clearly not validators

@crutchcorn
Copy link
Member

This alone makes a lot of sense to me. Let's move forward with the idea. Reviewing the code now

Comment on lines +283 to +296
onChange?: FieldListenerFn<
TParentData,
TName,
TFieldValidator,
TFormValidator,
TData
>
onHandleChange?: FieldListenerFn<
TParentData,
TName,
TFieldValidator,
TFormValidator,
TData
>
Copy link
Member

Choose a reason for hiding this comment

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

I'm not convinced we need a different function for programmatic vs user input. If the user needs this distinction, they can do so outside of our listeners

It also opens a naming can of worms and adds another one when we need an onBlurChange kinda deal

TFieldValidator,
TFormValidator,
TData
>
Copy link
Member

Choose a reason for hiding this comment

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

Do we want an onSubmit and onMount here as well?

Copy link

Choose a reason for hiding this comment

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

Yes it would give more versatility to the feature, and maybe onSubmit should allow for transformation before sending out the data ?

@zaosoula
Copy link

Any news on this feature ?

@harry-whorlow
Copy link
Contributor

@ha1fstack @crutchcorn If this task is stale, I would like to pick it up as this functionality is something we would really like.

For us, the use case as an example, is a payment form where if you select credit_card as a payment method you have a form field called creditCardBrands, which needs to be cleared when you change the payment method back to anything other than credit_card.

Personally, this is something that is not an infrequent use case, and having an api that's simpler than a hacky workaround utilising the validator linked-fields, is something that I feel is needed.

Let me know if I can take it up 🤟

@Balastrong
Copy link
Member

Completed in #1032

@Balastrong Balastrong closed this Nov 26, 2024
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.

5 participants