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

bug: changing value of component in ionChange results in an infinite loop #20106

Closed
ebk46 opened this issue Dec 18, 2019 · 28 comments
Closed

bug: changing value of component in ionChange results in an infinite loop #20106

ebk46 opened this issue Dec 18, 2019 · 28 comments
Labels
package: core @ionic/core package type: bug a confirmed bug report

Comments

@ebk46
Copy link

ebk46 commented Dec 18, 2019

Bug Report

Ionic version:
[x] 4.7.11

Current behavior:
When the value of an Ionic input element is changed from OUTSIDE of the element, the element's onIonChange is triggered, thereby duplicating the change event.

Expected behavior:
As with every other React input, the onIonChange should only be triggered when the value was change from the element itself.

Steps to reproduce:
See console in included stackblitz.

  1. Both the IonInput and regular input are controlled via the same value.
  2. When changing it from the IonInput, only the onIonChange is triggered.
  3. When changing it from the regular input, the input's onChange is called AS WELL AS the onIonChange from the IonInput.

Related code:
https://stackblitz.com/edit/ionic-v4-11-7-controlled-inputs

Other information:
This same bug is happening with other Ionic inputs as well. I haven't tested all of them, but it is happening with at least IonToggle and IonDateTime.

This is bad because unless each input has its own separate controlled state, doing something like clearing a state object could trigger 10+ onIonChange events, leading to race conditions and other annoyances.

Ionic info:

Ionic:

   Ionic CLI       : 5.4.13 (/Users/evan/.config/yarn/global/node_modules/ionic)
   Ionic Framework : @ionic/react 4.11.7

Capacitor:

   Capacitor CLI   : 1.4.0
   @capacitor/core : 1.4.0

Utility:

   cordova-res : not installed
   native-run  : not installed

System:

   NodeJS : v13.1.0 (/usr/local/Cellar/node/13.1.0/bin/node)
   npm    : 6.12.1
   OS     : macOS Catalina
@ionitron-bot ionitron-bot bot added the triage label Dec 18, 2019
@mhartington mhartington changed the title bug: Controlled Inputs trigger onIonChange even if the change was from other components bug: IonInput fires onIonChange when value is change outside of IonInput Dec 18, 2019
@masimplo
Copy link
Contributor

ionChange is triggered with every change of the input value, either through user input or programmatically. If you want your handler to be triggered only when the user changes the input, you should use ionInput event instead.

@elylucas elylucas added the package: react @ionic/react package label Dec 19, 2019
@ionitron-bot ionitron-bot bot removed the triage label Dec 19, 2019
@ebk46
Copy link
Author

ebk46 commented Dec 20, 2019

@masimplo Thanks for that clarification; I now see what happened. The event types for onIonInput may be broken. I tried using onIonInput and was getting Property 'value' does not exist on type 'EventTarget', but if I declare event: any it does work.

@ebk46
Copy link
Author

ebk46 commented Dec 20, 2019

And as a followup, it seems that other inputs don't have onIonInput or anything like it that only triggers for inputs via that element. For example, IonDateTime and IonToggle only have onIonChange. What are we supposed to do in those cases?

@antoninbeaufort
Copy link

antoninbeaufort commented Feb 6, 2020

Hi, I have the same problem, this is my organization :

  • App (contains filters state and update methods)
    • Menu (contains filters form)
    • FirstPage (display Menu and results)
    • SecondPage (display another filters form and results)

I have 2 forms that use a single state and when I change a value of a field from one form, it trigger the update method on both forms. (this is a problem because I fetch data on each update)

How can I trigger only one time the update method ? onIonInput is not available on IonSegment too...

@antoninbeaufort
Copy link

antoninbeaufort commented Feb 7, 2020

My first example is not really explicit because I can use differents filters on differents pages but I have another example where I haven't found a solution :

  • Form (keep all states of input in one object and contains update method)
    • Step 1
    • Step 2

At the end of the form I want to reset it, but if I do it it just reset some fields because they all interacts with the same object and re-trigger the update method for each input... The only workaround I have is to use one state for each input and when reset the form, call the setter of each state 😞

EDIT:
For those who would have the same problem as me: to avoid state modification conflicts when reacting it is possible with useState to modify from the previous state.

Example:

setSomething({
  ...something,
  anyProperty: value
})

becomes:

setSomething(previousState => ({
  ...previousState,
  anyProperty: value
}))

@KingMatrix1989
Copy link

@ebk46, Hi Evan. It seems there is not any good document or sample code for Ionic-React. Can you guide me, please?

@KingMatrix1989
Copy link

ionChange is triggered with every change of the input value, either through user input or programmatically. If you want your handler to be triggered only when the user changes the input, you should use ionInput event instead.

Same problem exist about onIonInput event

@svrakata
Copy link

Any ideas how to deal with that? I have IonSelect that is wired with redux state and when logic other than user interacting with that component is updating the values the ionOnChange event is triggered. How to know where the change is coming from?

@jayenashar
Copy link

onIonInput is working for me (4.11.10), but somehow when i change the value (type=date or type=time) outside of the onIonInput handler, it doesn't rerender with the new value.

@oskarkook
Copy link

This is quite an annoying "feature". Additionally, onIonChange calls the function from the previous render, which makes the issue even more annoying.

My current workaround is to keep a useRef reference that is updated on every render. Then when onIonChange is called, you can ignore the call if the new value is the same as the value in useRef.

@oskarkook
Copy link

Note that this feature ruins use-cases where one input field depends on another input field. You can end up in a weird update loop and the end state becomes completely unpredictable.

@kaloczikvn
Copy link

kaloczikvn commented Oct 1, 2020

There is no onIonInput for IonSelect so I can't reset the value from outside.

@foerderlabor
Copy link

foerderlabor commented Oct 13, 2020

It actually works when using useEffect as below:

useEffect(() => {
    validate();
  }, [email, password]);

  const changeEmail = (e: any) => {
    setEmail(e.detail.value);
  };

  const changePassword = (e: any) => {
    setPassword(e.detail.value);
  };

  const validate = () => {
    if (email === "" || password === "") {
      setInputValid(false);
    } else {
      setInputValid(true);
    }
  };

The problem is, that when checking for state-changes right after setFoo(newValue), the state didn't actually change yet. useEffect() can be triggered, when a state changes ... so, all you have to do is tell useEffect() in an array, which changes it should listen to.

Hope this helps.
Cheers!

@liamdebeasi liamdebeasi added package: core @ionic/core package type: bug a confirmed bug report labels Oct 20, 2020
@liamdebeasi liamdebeasi changed the title bug: IonInput fires onIonChange when value is change outside of IonInput bug: changing value of component in ionChange results in an infinite loop Oct 20, 2020
@liamdebeasi
Copy link
Contributor

This affects most components that have ionChange, resulting in some pretty annoying infinite loops when used this way.

I had filed #22365, but then realized this issue already existed. Here is a copy of my bug report with an up to date reproduction:

Bug Report

Ionic version:

[ ] 4.x
[x] 5.x

Current behavior:
When changing the value of an Ionic component via an ionChange callback, an infinite loop occurs because changing that value triggers another ionChange callback. This affects any component that has an ionChange event but this issue comes up more often in ion-toggle, ion-checkbox, and ion-input.

Expected behavior:

I would expect programatically changing the value in the ionChange callback would not trigger another ionChange, similar to how the native checkbox element works.

Steps to reproduce:

  1. Open this CodePen: https://codepen.io/liamdebeasi/pen/MWejVLE
  2. Open dev tools and switch to the console.
  3. Click the "Ionic Checkbox". Notice that this causes a new ionChange handler to be called every 1000ms.
  4. Click the "Native Checkbox". Notice that this causes the change event to only be called once.

Other information:

Changing this behavior is likely a breaking change.

@liamdebeasi liamdebeasi removed the package: react @ionic/react package label Oct 20, 2020
@MarekGogol
Copy link

MarekGogol commented Oct 21, 2020

What about leave ionChange as it is, for backward compactibility. And provide new event 'change' or something similar with fixed behaviour?

Next step: Events like @change in vuejs can be linked to this new fixed event, but when somebody uses @ionChange, they will be facing same issue till next release of Ionic v6, as mentioned bellow.

Next step: In v6 version this names can be replaced, for smoother transition...

Because it looks, this won't be fixed in short future.

@muhrifqii
Copy link

I still cannot find a workaround for this at the moment..

@Najros
Copy link

Najros commented Feb 5, 2021

A bug-around for anybody trapping into this Issue:
I found a simple workaround (actually I use it with IonDatetime component):
setting the attribute "key" on the component to current value, prevents the element from being updated after IonChange-event. Instead a new instance will be created and therefore the IonChange-event is not called in a loop. ;)
Good luck!

        <IonDatetime
          key={week.toISOString()}
          displayFormat="MMMM YYYY"
          value={format(week, "yyyy-MM")}
          onIonChange={onChange}
        />

@Cooya
Copy link

Cooya commented Apr 24, 2021

My workaround consists to check if the select has the focus :

let hasFocus = false;
<IonSelect value={value} onFocus={() => hasFocus = !hasFocus} onIonChange={e => hasFocus && onChange(e.detail.value)}>
...
</IonSelect>

@ihorbond
Copy link

Is there any update on this?

@dev-sammy
Copy link

dev-sammy commented Sep 8, 2021

I am an Angular guy, I don't know if it will help in react components but you can check this I have found a workaround.

ion-toggle unnecessarily triggers change event when default value is true

@babycourageous
Copy link
Contributor

babycourageous commented Dec 19, 2021

UPDATE 12/20: The "key" workaround is not fully working after all. If you make a selection and hit OK all is well. If however you open the select and hit OK without making a change (so it's the same selection) the infinite loop still happens.


Just want to mention that I also ran into this infinite loop using an IonSelect (Ionic React v5) alongside React Hook Form as a Controlled component.

I experience it when select options are simple strings and multiple is true. I also notice it as well as ​for complex object options in both single and multiple selects.
I do not experience it for simple string single select.

For anyone else using RHF alongside Ionic, I went with a variation on @Najros "bug-around". Thanks for that!

<Controllername="saleGroups"control={control}render={({ field: { value, onChange } }) => (<IonSelectkey={value.toString()} // required to prevent infinite loop for "multiple"multiplevalue={value}onIonChange={(e) => onChange(e.detail.value)}>{selectOptions.map((item) => (<IonSelectOptionkey={item.id}value={item.id}>{item.name}</IonSelectOption>))}</IonSelect>)}
/>

I did manage to get it working with the more complex object value select. It's a bit more complicated as it involves the compareWith prop and JSON.stringify but am happy to share if anyone is facing a similar hurdle.

I'll update with any changes after upgrading to Ionic v6

@otemler
Copy link

otemler commented Jan 7, 2022

Facing the same loop issue in combination with React Hook Form (Controller) using Ionic v6.
The mentioned key workaround only works as long as you don't hit "OK" again without changing the selected value.

Edit: Workaround with mentioned hasFous & key prop in React Hook Form

 <Controller
      render={({ field: { onChange, value } }) => (
        <IonSelect
          multiple={multipleSelect}
          value={value}
          onFocus={() => (hasFocus = !hasFocus)}
          key={value}
          onIonChange={(e) => hasFocus && onChange(e.detail.value)}
        >
          {options?.map((item: ISPItem) => (
            <IonSelectOption key={item.Id} value={item.Id}>
              {item.Title}
            </IonSelectOption>
          ))}
        </IonSelect>
      )}
      name={name}
      defaultValue={defaultValue}
      control={control}
      rules={{ required: required }}
    />

That's working for me even when I update with no value changed.

@XueMeijing
Copy link

well,i tried the workaround above such as hasFous & key , but this doesn't work. When i add random key on IonToggle,it works and won't trigger more or infinite loop of onIonChange.
<IonToggle
slot='end'
key={Math.random()}
checked={item.enabled}
disabled={item.status === 'EXPIRED'}
onIonChange={(e) => onChangeDelegationStatus(e, item)}
/>

@babycourageous
Copy link
Contributor

Hi. I'm still running into these infinite loops with React Hook Form and so far none of the above workarounds do the trick. What I have so far found to work is to only call onChange if the value has changed. This new workaround seems to work because opening the Select and not changing anything then hitting OK is what throws me into a loop (with or without using any of the above suggestions).

<Controller
  name="multiSelectExample"
  control={control}
  render={({ field: { value, onChange } }) => {
    return (
      <IonSelect
        key={value.toString()}
        multiple
        value={value}
        onIonChange={(e: CustomEvent<SelectChangeEventDetail<Array<string>>>) => {
          // EARLY RETURN IF VALUE WASN'T CHANGED
          if (
            JSON.stringify(e.detail.value) === JSON.stringify(value)
          ) {
            return
          }
          onChange(e.detail.value)
        }}>
          {selectOptions.map((option) => (
            <IonSelectOption
              key={option.value}
              value={option.value}
            >
              {option.label}
            </IonSelectOption>
          ))}
       </IonSelect>
     )
   }}
 />

While the above is a React Hook Form sample, the issue doesn't seem to be limited to the React Hook Form integration. #24691 points back to the original RHF issue which has a non-RHF repro - https://codesandbox.io/s/thirsty-pasteur-wqpuz?file=/src/App.tsx:137-143

Hope all this extra info and another workaround is helpful!

@sean-perkins
Copy link
Contributor

Hello everyone, I've created an RFC to discuss changes to the API design that should correct this issue: #25532

Please share your thoughts and if there is anything we may be missing.

Thanks!

@captainR0bbo
Copy link

Hi. I'm still running into these infinite loops with React Hook Form and so far none of the above workarounds do the trick. What I have so far found to work is to only call onChange if the value has changed. This new workaround seems to work because opening the Select and not changing anything then hitting OK is what throws me into a loop (with or without using any of the above suggestions).

<Controller
  name="multiSelectExample"
  control={control}
  render={({ field: { value, onChange } }) => {
    return (
      <IonSelect
        key={value.toString()}
        multiple
        value={value}
        onIonChange={(e: CustomEvent<SelectChangeEventDetail<Array<string>>>) => {
          // EARLY RETURN IF VALUE WASN'T CHANGED
          if (
            JSON.stringify(e.detail.value) === JSON.stringify(value)
          ) {
            return
          }
          onChange(e.detail.value)
        }}>
          {selectOptions.map((option) => (
            <IonSelectOption
              key={option.value}
              value={option.value}
            >
              {option.label}
            </IonSelectOption>
          ))}
       </IonSelect>
     )
   }}
 />

While the above is a React Hook Form sample, the issue doesn't seem to be limited to the React Hook Form integration. #24691 points back to the original RHF issue which has a non-RHF repro - https://codesandbox.io/s/thirsty-pasteur-wqpuz?file=/src/App.tsx:137-143

Hope all this extra info and another workaround is helpful!

I also commented on #24584 - try an earlier version of react-hook-form version if you need a workaround for now. 7.12.2 worked for me.

@sean-perkins
Copy link
Contributor

This issue has been resolved in this pull request: #25858 and will be available in the next major release (v7) of Ionic.

@ionitron-bot
Copy link

ionitron-bot bot commented Oct 12, 2022

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Oct 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
package: core @ionic/core package type: bug a confirmed bug report
Projects
None yet
Development

No branches or pull requests