lawnstarter / react-native-picker-select

🔽 A Picker component for React Native which emulates the native <select> interfaces for iOS and Android
https://npmjs.com/package/react-native-picker-select
MIT License
1.75k stars 499 forks source link

onValueChange Event triggers every time render update #112

Open indrsidhu opened 5 years ago

indrsidhu commented 5 years ago

Describe the bug Picker has function prop "onValueChange" as per expectation it should work like click event, when ever someone select value, but i found this event execute all the time when render() updates, so it creating problem, because it null my state value.

To Reproduce onValueChange = { (value) => console.log("Event Triggered"+value)} Monitor console, it will execute all the time whenever render() update by any state or prop change.

Expected behavior It should execute only when we change dropdown value, after pick , not all the time

Smartphone (please complete the following information):

        <PickerSelect
        value={formData['country']}
        placeholder={{label: I18n.t('select'), value: ''}}
        style={pickerStyle}
        onValueChange={(itemValue,itemIndex) => {
            console.log("Country index="+itemIndex);
            console.log("Country current="+formData['country']);
            console.log("Country Changed="+itemValue);
            this.handleChangeValue(itemValue,"country","picker")
        }}
        items={COUNTRY_RENDER}
        />

Once select country i set in state using "handleChangeValue" formData['country'] Whenever i change language it update my render() I tried to debug

            console.log("Country index="+itemIndex);
            console.log("Country current="+formData['country']);
            console.log("Country Changed="+itemValue);
            this.handleChangeValue(itemValue,"country","picker")

formData['country'] always return my selected country, but console.log("Country Changed="+itemValue);

does not return selected index once, i change language.

If language value is same for both english and spanish language, then it should return same index

lfkwtz commented 5 years ago

Please update your bug report with the missing information

indrsidhu commented 5 years ago

I updated my post

lfkwtz commented 5 years ago

Thanks. I’ll take a look. I assume you meant version 5.1.0?

indrsidhu commented 5 years ago

yes i have latest version

vikasdosi commented 5 years ago

Describe the bug Picker has function prop "onValueChange" as per expectation it should work like click event, when ever someone select value, but i found this event execute all the time when render() updates, so it creating problem, because it null my state value.

To Reproduce onValueChange = { (value) => console.log("Event Triggered"+value)} Monitor console, it will execute all the time whenever render() update by any state or prop change.

Expected behavior It should execute only when we change dropdown value, after pick , not all the time

Smartphone (please complete the following information):

  • Device: OnePlus2
  • OS: Android
  • react-native-picker-select version: 5.1.10
  • react-native version: 0.55.4
  • react version: 16.3.1 Reproduction and/or code sample
        <PickerSelect
        value={formData['country']}
        placeholder={{label: I18n.t('select'), value: ''}}
        style={pickerStyle}
        onValueChange={(itemValue,itemIndex) => {
          console.log("Country index="+itemIndex);
          console.log("Country current="+formData['country']);
          console.log("Country Changed="+itemValue);
          this.handleChangeValue(itemValue,"country","picker")
      }}
        items={COUNTRY_RENDER}
        />

Once select country i set in state using "handleChangeValue" formData['country'] Whenever i change language it update my render() I tried to debug

          console.log("Country index="+itemIndex);
          console.log("Country current="+formData['country']);
          console.log("Country Changed="+itemValue);
          this.handleChangeValue(itemValue,"country","picker")

formData['country'] always return my selected country, but console.log("Country Changed="+itemValue);

does not return selected index once, i change language.

If language value is same for both english and spanish language, then it should return same index

you can solve by using if condition in on value change

like this

  onValueChange={(value) => {
 if(itemValue!=this.state.PickerValueHolder){
this.setState({
                          PickerValueHolder: value,
                        });
                   //your function 
}
}
}
AndrewAi commented 5 years ago

Hi, When I change the value, the onValuechange fires, but its delayed and the value returned is always one behind. i.e the default value is 'Select a Category' the other values are Music, Poetry, Comedy. when I pick Music and the onValueChange fires the first time it returns "null", when I pick Poetry it returns "Music". so everytime its one behind, any ideas whas going on am I doing something wrong

                            <RNPickerSelect
                                placeholder={{
                                    label: 'Select a Category...',
                                    value: null,
                                }}
                                items={this.state.items}
                                onValueChange={(value) => this.setEventCategory(value)}

                                style={{...pickerSelectStyles}}
                                value={this.state.eventCategory}
                                ref={(el) => {
                                    this.inputRefs.picker = el;
                                }}
                            />

Thank you for any help and thank you for RNPickerSelect

lfkwtz commented 5 years ago

@indrsidhu please create an example on snack.expo.io or share a github project with your issue -- I have tried to reproduce and I'm not having that issue (https://snack.expo.io/S1O1Q5914)

@AndrewAi please create an example on snack.expo.io or share a github project with your issue --- nothing looks wrong in what you've shared (although I don't know what setEventCategory does.

vikasdosi commented 5 years ago

Hi, When I change the value, the onValuechange fires, but its delayed and the value returned is always one behind. i.e the default value is 'Select a Category' the other values are Music, Poetry, Comedy. when I pick Music and the onValueChange fires the first time it returns "null", when I pick Poetry it returns "Music". so everytime its one behind, any ideas whas going on am I doing something wrong

                            <RNPickerSelect
                                placeholder={{
                                    label: 'Select a Category...',
                                    value: null,
                                }}
                                items={this.state.items}
                                onValueChange={(value) => this.setEventCategory(value)}

                                style={{...pickerSelectStyles}}
                                value={this.state.eventCategory}
                                ref={(el) => {
                                    this.inputRefs.picker = el;
                                }}
                            />

Thank you for any help and thank you for RNPickerSelect

Yes i think you are setting state and then calling some Api Will suggest you should use like that this.setstate({“your state same”:value},()=>{console.log(this.state.yourstatename)})

AndrewAi commented 5 years ago

Hi thank you for your comments. this is my setEventCategory function setEventCategory(eventCategory){ this.setState({eventCategory: eventCategory}); console.log('setEventCategory: eventCategory: ', this.state.eventCategory) }
very simple. should I be using some onSubmit or onDonePress instead of onValueChange ? Thanks very much

vikasdosi commented 5 years ago

Hi thank you for your comments. this is my setEventCategory function setEventCategory(eventCategory){ this.setState({eventCategory: eventCategory}); console.log('setEventCategory: eventCategory: ', this.state.eventCategory) } very simple. should I be using some onSubmit or onDonePress instead of onValueChange ? Thanks very much

Try this way this will work setEventCategory(eventCategory){ this.setState({eventCategory: eventCategory},()=>{console.log('setEventCategory: eventCategory: ', this.state.eventCategory) })}

AndrewAi commented 5 years ago

Ok I will thank you, but I wont be able to try it till tomorrow. thanks

glenne commented 5 years ago

I'm seeing the same issue but am unable to reproduce via snack.expo.io. I observed the following sequence:

  1. Picker initialized with an initial value, say 'one'
  2. Callback occurs via onValueChange and setState invoked with new value, say 'two'
  3. getDerivedStateFromProps called in RNPickerSelect. The nextProps has the prior select value of 'one', state has the value of 'two'.
  4. Logic decides that selected state has changed and onValueChanged called again with the nextProps value of 'one' which is the 'old' value. onValueChanged queues up another setState switching the value back to 'one',
  5. The parent finally invokes render for the first time with the state of 'two'
  6. The infinite loop begins switching between the two states.

I worked around this by removing the call nextProps.onValueChange(selectedItem.value, idx); from getDerivedStateFromProps.

jpandl19 commented 5 years ago

I experienced the exact same issue as @glenne. Removing nextProps.onValueChange(selectedItem.value, idx) from getDerivedStateFromProps also fixed the problem for me.

lfkwtz commented 5 years ago

@jpandl19 @glenne i'm going to release a version with a prop to disable that. should resolve the issue for now and then we can look into it further in the future.

glenne commented 5 years ago

I fixed mine by turning it into a controlled component like most of the other react-native components so rendering is only driven by props with not state memory. This actually simplifies a number of interactions.

onValueChange(value, index) {
    const { onValueChange } = this.props;
    onValueChange(value, index);
  }

static getDerivedStateFromProps(nextProps, prevState) {
     const newItems = RNPickerSelect.handlePlaceholder({
      placeholder: nextProps.placeholder
    }).concat(nextProps.items);
    const { selectedItem, idx } = RNPickerSelect.getSelectedItem({
      items: newItems,
      key: nextProps.itemKey,
      value: nextProps.value
    });
    return {
      items: newItems,
      selectedItem: selectedItem
    };
  }
lfkwtz commented 5 years ago

@glenne @EyMaddis @jpandl19 would you mind checking if this branch solves the issue you were having?

i'd ideally like to make as few breaking changes as possible, so this change will just prevent the internal selectedValue state change from occurring after an onValueChange call IF you have the value prop defined. that way, it will be closer to the controlled component pattern.

lfkwtz commented 5 years ago

@santanapaulo does this branch solve your issue?

santanapaulo commented 5 years ago

@santanapaulo does this branch solve your issue?

No @lfkwtz , It didn't work. I installed the version at that branch you said, but no happy ending :/

codazzo commented 5 years ago

I was experiencing this in the iOS simulator. Disabling Hot Reloading from the Inspector seems to have fixed this in my case.

YoshiYo commented 5 years ago

I don't know why, but I fixed the extra render when I removed the props. itemKey.

lfkwtz commented 5 years ago

Happy to look into this further if someone can provide a working demo that clearly identifies the issue

altyaper commented 5 years ago

It happened for me whenever I setState inside a promise on onValueChange:

Example:

import PickerSelect from 'react-native-picker-select';
import { View } from 'react-native';

class MyComponent extends Component {

   constructor(props) {
      super(props);
      this.state = {
         state: null,
         states: [{ label: 'Arizona', value: 1}, { label: 'California', value: 2}]
      }
   }

   handleStateChange(state) {
       // This is a promise
       StateApi.getStates().then(statesResponse => {
            console.log("Logger");
            this.setState(state);
       }); 
   }

  render() {
    <View>
       <PickerSelect
              items={states}
              onValueChange={this.handleStateChange}
              value={this.state.state}
              ref={(el) => {
                  this.inputRefs.state = el;
              }}
          />
    </View>
  }
}

Whenever I change the value of the picker one time, the function handleStateChange is triggered twice.

image

shanekoss commented 5 years ago

@EyMaddis fork fixed this issue for me.

https://github.com/simpleTechs/react-native-picker-select/commit/e70f4b34c7cebeb8f322524a8f262f08e9c0cc3a

Could this get merged?

My problem was anytime the value was changed elsewhere, the callback was still called.

Fixed for now.

Achilles718611 commented 5 years ago

image

How about change order of two lines?

Or we can fire onValueChange after state is updated like followings. this.setState({ selectedItem: this.state.items[index], }, () => onValueChange(value, index));

glenne commented 5 years ago

@glenne @EyMaddis @jpandl19 would you mind checking if this branch solves the issue you were having?

i'd ideally like to make as few breaking changes as possible, so this change will just prevent the internal selectedValue state change from occurring after an onValueChange call IF you have the value prop defined. that way, it will be closer to the controlled component pattern.

Sorry for the late reply. This branch worked for me.

jpandl19 commented 5 years ago

@lfkwtz The branch also seems to fix the problem for me 👍

glenne commented 4 years ago

Due to some personal travel I won't be able to test for a few weeks but check it out.

On Wed, Nov 20, 2019 at 5:31 PM Michael Lefkowitz notifications@github.com wrote:

@jpandl19 https://github.com/jpandl19 @glenne https://github.com/glenne https://github.com/lawnstarter/react-native-picker-select/tree/v7

i've incorporated those changes into an upcoming release - mind testing that branch out to see if the changes are still resolved? it's been a while since I made that original one

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/lawnstarter/react-native-picker-select/issues/112?email_source=notifications&email_token=AAITQ6EDB5OKV5AX2SWW7FLQUXQGPA5CNFSM4GE7T5C2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEEX463A#issuecomment-556781420, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAITQ6D74WEKZ5RRLKIMAR3QUXQGPANCNFSM4GE7T5CQ .

lfkwtz commented 4 years ago

@glenne thanks - i pulled that comment off as I don't think the branch was ready. will post here again soon if anyone wants to check out the bugfix.

my plan is to release this change so we can finally close out this bug - afterwards, I'd like to setup a split export in a future version with a controlled component which can live in beta for now and potentially replace the original down the line

lfkwtz commented 4 years ago

@glenne @jpandl19 (and anyone else)

please test this beta version to see if this issue has (finally) been resolved https://www.npmjs.com/package/react-native-picker-select/v/7.0.0-beta.0

as mentioned above, i plan to release a v7 with this fix -- and then start work on a controlled component

jawadm commented 4 years ago

I am using the beta version 7.0.0-beta.0. It did fix an issue with the dropdown changing the values a couple times before going back to the correct value.

In the onValueChange I still have to check if the value does not equal the new one using an if statement, otherwise it results in an infinite loop. That is still a bug should be addressed. In other pickers there is no need for the if statement.

onValueChange={(value, index) => { if (value != this.state.value) this.valueChanged(value, index-1); }}

Also, when there is a placeholder, it gets included in the array of values, so the index needs to be adjusted by -1. That should probably not be the case either.

lfkwtz commented 4 years ago

@jawadm thanks for the feedback -- @glenne / @jpandl19 - either of you have a chance to check it out?

himrocks33 commented 4 years ago

@lfkwtz is there a release date for 7.0.0? I'm having multiple issues with this component since upgrading to expo 37.0.6 and react 16.9.0. Mainly setting value no longer works as onValueChanged is called twice. Once with the correct value and then a second time with the previous value.

lfkwtz commented 4 years ago

@himrocks33 no - i'm going to try to start rolling out some major versions with some smaller breaking changes to clean up some other issues -- but this bug specifically won't be fixed until I or someone else finds time to tackle it. hopefully soon but i'm not going to make any promises on my availability.

himrocks33 commented 4 years ago

@lfkwtz Thank you. I installed beta.7.0.0 and it fixed my issues.

himrocks33 commented 4 years ago

@lfkwtz I've noticed that you update the version number to 7.0.0 but did not include the beta version code that fixed this issue? When I ran npm install this morning this in turn broke my code again as react-native-picker-select@^7.0.0-beta.0 now resolves to 7.0.0 but the code is broken in 7.0.0. Can the beta branch code be merged into 7? Thank you.

lfkwtz commented 4 years ago

my response to your earlier question explains that - you probably will want to force pointing to the beta version

himrocks33 commented 4 years ago

@lfkwtz yes that is what I ended up doing and it is working again. Just for further information. I've noticed this only seems to be an issue while using with redux. If I do the same implementation with the react state manager then this component works as expected with any version.

jameschetwood commented 4 years ago

Ive just switched to 7.0.0-beta.0 for my Expo + Redux project and I'm getting the same bug.

TheAdamGalloway commented 4 years ago

Hey guys, I'm having the same issue (Snack: https://snack.expo.io/@adamgalloway/79883d)

Any recommendations on how I can fix my code?

thanks!

sarcadass commented 4 years ago

If you want a very cheap, shameful and dirty solution (but working nonetheless). You can validate that the value has been changed by the user if the value has been changed x milliseconds (in that case 100 ms) after the component has been mounted (because the user won't have enough time to click the element and select a value in such a short time). I tested it and it's working on Android, iOS and web. You should however wash your hands after using that hack 😬 :


const Component = ({ ... }) => {
  const defaultValue = 'defaultValue'
  let mountedOn

  useEffect(() => {
    mountedOn = Date.now()
  })

  return (
    <RNPickerSelect
      ...
      value={defaultValue}
      onValueChange={(value) => {
        if (value !== defaultValue && Date.now() - mountedOn > 100) {
          dispatchFunction(value)
        }
      }}
    />
  )
}
TheAdamGalloway commented 4 years ago

Thank @sarcadass - this worked for me!

SmirnovM91 commented 4 years ago

Hey everyone, same issue

SmirnovM91 commented 4 years ago

I've fixed it for me by adding key parametr to RNPickerSelect like this image

P.S. I use Mobx state management

vreuling-bcs commented 4 years ago

I'm having the same issue. (only on Android) I really hope to see a solution soon!

But since the issue has been unresolved for over 1.5 years now, I might need to consider other options. Sad though, because other than this issue, this is a really great component!

@lfkwtz Is there any way I can help resolve the issue?

My reproduction snack: https://snack.expo.io/@bcsbv/rnpickerselect-issue-reproduction

lfkwtz commented 4 years ago

https://github.com/lawnstarter/react-native-picker-select/pull/368

feedback re: this PR would be appreciated

jlampel commented 4 years ago

This was driving me nuts and the above hack by @sarcadass worked for me in the meantime!

lfkwtz commented 4 years ago

@jlampel mind testing #368?

lfkwtz commented 4 years ago

@vreuling-bcs can you check #368?

jlampel commented 4 years ago

Ok, apologies for the delay but I tested #368 and unfortunately it did not solve this particular issue - the picker continued flickering back and forth between the old and new value, even with the above workaround in place.

lfkwtz commented 3 years ago

@jlampel can you provide a repro?