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 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

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.

I agree that the doubled ionChange events are a duplicate of #20106 (the underlying behavior is the same), but we can use this issue to track the bug with the 1,2 string value.

@sean-perkins

@sean-perkins I have observed this behavior as well. As far as I can tell the reason the array is getting stringified is because react is detecting ion-select as a customer component and when it detects this it converts the attributes to a string.

@sean-perkins

@sean-perkins following up with additional data. I updated the reproduction of #24687 to also demonstrate this bug. Video attached

ionic-select-object-stringify-bug.mov

In my case it seems like setting the value in ionOnChange via the setter of useState triggers React updates ion-select using node.setAttribute(__attributeName, '' + value) which is why it gets stringified. Stencil sees this change reflected which triggers another ionOnChange.

@sean-perkins

@sean-perkins do you have any idea where the root of this issue might lie? If I have the bandwidth I'd be happy to help but afraid I would need more context to do so?

The first question I have is should React see IonSelect as a Web Component? From what I understand natively IonSelect is a Web Component generated by Stencil but is exposed to React as a web component wrapped in a React element. However if that were the case I would not expect React to be calling setAttribute on the element and the call stack seems to suggest it knows it's a Web Component.

Without knowing a great deal about how React interacts with Ionic Stencil components my two guesses would be in either the creation of the wrapped element or in Stencil itself. Any thoughts would be appreciated.

@adamduren

@adamduren appreciate the willingness to help out here!

All Stencil web components are wrapped in React's createElement function, so they are consumed as React components. There's some logic in the middle layer between React and our web component, that likely contributes to this issue.

I would suspect the issue is somewhere in here:

This utility component & functions actually comes from the output targets repository: https://github.com/ionic-team/stencil-ds-output-targets/tree/master/packages/react-output-target/react-component-lib and any manual changes you make here for verification, would need to be eventually submitted through that repository.

Any time you run npm run build from packages/core, will overwrite your local changes in those directories, so I would advise building Core once with the logs you need, then make your local changes in packages/react/src/components/react-component-lib/ and validate from the React test app or a local project.

Let me know if this information is helpful or if you have any follow-up questions!

@sean-perkins

@sean-perkins thank you, this is helpful. Will let you know if I have any breakthroughs.

@sean-perkins

@sean-perkins any update on the suggested fix for this issue in #24763?

@sean-perkins

@sean-perkins to avoid developer confusion maybe we should add to the documentation the using multiple with non-string values is not currently supported in react.

@adamduren

Thanks for all this great info @adamduren , @sean-perkins

Anyone landing here looking for a workaround to prevent errors or crashes. I'm currently doing a typeof check in my code to early return from that initial string list.

ie

function handleThing(selectData) {
  if (!Array.isArray(selectData)) return

  // REST OF CODE WITH ASSURANCE WE ARE WORKING WITH AN ARRAY
}

This is especially helpful for code running in an effect - which was my use-case - and causing crashes on initial run.
Can't guarantee it's foolproof but so far is doing the job.

@babycourageous

@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

Thanks @sean-perkins for diggin' into it some more. Lemme know if you need any more info!

Investigated this further, here is a dev-build to experiment with: 6.1.5-dev.11651861415.15120db6.

There are two conditions that we need to check for internally:

Is the value not an array (due to the way that React manipulates the attribute value when assigning).
Does the emitted value match the previous emitted value (React is still assigning the value to the component directly, so we need to still skip/ignore the value change from our setState correcting the value; to avoid duplicate ionChange events).

Here is the updated reproduction app with the dev-build: https://stackblitz.com/edit/react-x2eaeh

Overall, I am happier with this approach. It does require an opinionated change to the web component layer, but is introducing safety around the value being emitted, which does benefit apps in scenarios where the value may be mistakenly assigned as a string instead of an array. I will discuss with the team more next week + write tests around this change to validate I am not missing any unexpected side effects.

@sean-perkins

Thanks @sean-perkins !

@sean-perkins

@sean-perkins, @babycourageous - this dev build did resolve my issue. There is a lot going on here that I don't fully understand. but I think this might be helpful for some:

react-hook-form 7.33.0 and Ionic 6 creates this infinite loop using a basic Ionic multi-select.
react-hook-form 7.12.2 and Ionic 6 did NOT have this infinite loop issue
react-hook-form 7.33.0 and @sean-perkins dev build did NOT have this loop issue

Here is an example if the issue using latest react-hook-form
https://stackblitz.com/edit/react-vhsayj?file=src%2FApp.js

Of course there a lot of versions in between but the latest react-hook-form update is what broke my implementation.

Thanks!

Hi everyone,

We have an update coming in Ionic 7.0 that changes how ionChange is fired from components. To summarize, ionChange will only fire when the value property is changed as a result of a user gesture. You can read more about this revision here: #25532

The reported issue is resolved by the ionChange revisions. The work has been completed and merged into our codebase, so we are going to close this ticket out. These changes should be available in Ionic 7.0. Please see our blog or Twitter to be notified when this release is published.

I have also provided a development build of Ionic 7.0 below if anyone would like to test the changes. Please note that this is a major release of Ionic and is subject to the Ionic 7.0 Breaking Changes. Let me know if you have any questions!

Dev Build:

6.3.3-dev.11666796611.134ca337

Install Example:

npm install @ionic/angular@6.3.3-dev.11666796611.134ca337

@liamdebeasi

Thanks for the update @liamdebeasi !

Looking forward to the future release, and I think we have enough options to find workarounds until then!

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.

Navigation Menu

Repository navigation

bug: IonSelect w/ multiple={true} yields incorrectly formatted value in onIonChange #24584

Prerequisites

Ionic Framework Version

Current Behavior

Expected Behavior

Steps to Reproduce

Code Reproduction URL

Ionic Info

Additional Information

4 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

bug: IonSelect w/ multiple={true} yields incorrectly formatted value in onIonChange #24584

Description

Prerequisites

Ionic Framework Version

Current Behavior

Expected Behavior

Steps to Reproduce

Code Reproduction URL

Ionic Info

Additional Information

Activity

sean-perkins commented on Jan 17, 2022

averyrousseau commented on Feb 1, 2022

adamduren commented on Feb 3, 2022

adamduren commented on Feb 3, 2022

adamduren commented on Feb 3, 2022

adamduren commented on Feb 9, 2022

sean-perkins commented on Feb 9, 2022

adamduren commented on Feb 9, 2022

adamduren commented on Feb 17, 2022

4 remaining items

adamduren commented on Mar 10, 2022

babycourageous commented on Apr 30, 2022

sean-perkins commented on May 2, 2022

babycourageous commented on May 2, 2022

sean-perkins commented on May 6, 2022

babycourageous commented on May 11, 2022

captainR0bbo commented on Jun 30, 2022

liamdebeasi commented on Oct 26, 2022

babycourageous commented on Oct 26, 2022

ionitron-bot commented on Nov 25, 2022

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions