ionic-framework: bug: IonSelect w/ multiple={true} yields incorrectly formatted value in onIonChange

Prerequisites

I have read the Contributing Guidelines.
I agree to follow the Code of Conduct.
I have searched for existing issues that already report this problem, without success.

Ionic Framework Version

v4.x
v5.x
v6.x

Current Behavior

When a change is made in an IonSelect with multiple={true}, it triggers onIonChange 3 times with the following behavior:

Trigger 1) Values as array Trigger 2) Values as comma-separated string Trigger 3) Values as array

Obviously, this is very bad as it should only trigger once, and the 2nd trigger violates the Array<T> exected type.

Expected Behavior

onIonChange should only trigger once, and should include only an array of values, not a comma-separated string.

Steps to Reproduce

This happens with any IonSelect with multiple={true}. See example project below.

Code Reproduction URL

https://stackblitz.com/edit/react-ne3qq8

Ionic Info

Ionic:

Ionic CLI : 6.18.1 (/Users/evan/.config/yarn/global/node_modules/@ionic/cli) Ionic Framework : @ionic/react 6.0.2

Capacitor:

Capacitor CLI : 3.3.4 @capacitor/android : 3.3.4 @capacitor/core : 3.3.4 @capacitor/ios : 3.3.4

Utility:

cordova-res (update available: 0.15.4) : 0.15.3 native-run : 1.5.0

System:

NodeJS : v16.13.2 (/usr/local/bin/node) npm : 8.1.2 OS : macOS Catalina

Additional Information

No response

About this issue

Original URL
State: closed
Created 2 years ago
Reactions: 3
Comments: 23 (21 by maintainers)

Most upvoted comments

Thanks for reporting this issue! I’m seeing the same problems within your reproduction.

Similar issues have been marked as a duplicate of #20106, but I will confirm with the team on Tuesday (Monday is a holiday).

I’m currently doing internal exploration to rewrite how form controls behave and can add this test case to the list to see if it’s resolved with the change to how we notify value change events.

Regardless of the duplicate emission, the 1,2 string value is absolutely wrong and should never be emitted.

sean-perkins on Jan 17, 2022

@babycourageous very interesting, thanks for sharing! Maybe the temporary solution could be that simple, but handled internally for you.

For example, we could skip emitting ionChange if the ion-select has multiple enabled and the value that is to be emitted is a string.

I’ll do some discovery on this later in the week.

sean-perkins on May 2, 2022

@adamduren appreciate the ping, I will take a look tomorrow and compare my findings against your PR. Excluding the value from being reflected may have other impacts that I would like to explore/de-risk.

Also there’s another issue with form controls and React that I have in review (#24858) that may inspire a solution if the problem ends up being with how React transforms the value.

I’ll share where I’m at tomorrow afternoon. I’d love to get this bug resolved.

sean-perkins on Mar 9, 2022