rkit / react-select2-wrapper

Wrapper for Select2
MIT License
163 stars 97 forks source link

using `onChange` causes the page to hang #63

Open dimak opened 7 years ago

dimak commented 7 years ago

When I try to use the onChange binding, the page freezes up until I make my first change on the select. Then the page behaves relatively normally. I think there is some kind of circular checking that happens, when using this handler. onSelect seems to do the trick, not causing hanging.

On a separate note, when trying to test the functionality with enzyme, simulate('change') seems to not work, most likely because I'm using onSelect instead of onChange.

Stack trace:

Uncaught (in promise) RangeError: Maximum call stack size exceeded
    at RegExp.[Symbol.replace] (native)
    at String.replace (native)
    at Function.htmlPrefilter
    at jQuery.fn.init.<anonymous>
    at access
    at jQuery.fn.init.html
    at DecoratedClass.Placeholder.createPlaceholder
    at DecoratedClass.createPlaceholder
    at DecoratedClass.Placeholder.update
    at DecoratedClass.update

Please let me know if you need more details, thanks.

Addendum: My value is '', but my data does not contain this value. I think this is what is causing the infinite loop.

rkit commented 7 years ago
  1. Can you show example code for reproducing that?
  2. What kind of version do you have?
dimak commented 7 years ago

approximate code:

...
constructor(props) {
  super(props);
  this.state = {
    fields: {
      name: '',
      address: '',
      country: ''  // value is set to empty string
    }
  }

  this.countries = [  // does not contain an `id` with empty string
    { id: 'USA', text: 'United States' },
    { id: 'CAN', text: 'Canada' },
    { id: 'CHN', text: 'China' },
    ...
  ]
}

...

handleOnChange(e) {
  const target = e.target;
  const fields = { ...this.state.fields, [name]: value };
  this.setState({ fields });
}

...

render() {
  return (
    <div>
      <FormGroup>
        <ControlLabel>Name</ControlLabel>
        <FormControl
          type='text'
          placeholder='Name'
          name='name'
          value={this.state.fields[name]}
          onChange={this.handleOnChange}
        />
      </FormGroup>
      <FormGroup>
        <ControlLabel>Address</ControlLabel>
        <FormControl
          type='text'
          placeholder='Address'
          name='address'
          value={this.state.fields[name]}
          onChange={this.handleOnChange}
        />
      </FormGroup>
      <FormGroup>
        <ControlLabel>Country</ControlLabel>
        <Select2
          value={this.state.fields.iso_country}
          name='country'
          onChange={this.handleOnChange}
          data={this.countries}
          options={{
            placeholder: 'Country'
          }}
        />
      </FormGroup>
    </div>
  )
}

version: "react-select2-wrapper": "^1.0.4-beta2"

rkit commented 7 years ago

Thanks for this code. This will be fixed soon.

rkit commented 7 years ago

Please, try a new beta release 1.0.4-beta3

dimak commented 7 years ago

Sorry for taking so long to reply. The initial hanging issue is resolved, however, now it hangs when a selection is made from the select2.

FYI: The only changes I've made to the code, is switching from using onSelect to using onChange.

Thanks again.

chrisrichard commented 7 years ago

I'm seeing what I think is the same issue, although the page doesn't completely hang, I'm just unable to scroll after making a selection that gets handled by onChange. Using onSelect seems to work fine so thanks @dimak for the workaround.

rkit commented 7 years ago

@dimak @chrisrichard, Guys, I can't reproduce this. Can you show me the example code?

gazpachu commented 7 years ago

I can confirm that this issue is also happening for me in beta-2 and beta-3

v-Zer0 commented 7 years ago

@rkit @gazpachu Yes I still see this issue in beta-3

Both onSelect and onChange cause the page to hang with an eventual overflow error.

The setState call in the handleChange method causes the Select2 element to rerender. When this happens React rerenders the page infinitely because maybe the change event is probably being fired a second time?

Setting the local state on change is a pretty common practice in react... Is there a known way around this for your react-select2-wrapper?

  handleChange(e) {

    this.setState({
      errors: {...this.state.errors, ...{ [name]: undefined } },
      submitDisabled: false
    });

    const { value, name} = e.target;
    const element = this.findElement(name);

    if (element.validation) {
      this.validateOnTimeout(value, name, element);
    }
  }

<Select2
  name={ ele.name }
  multiple={ ele.config.multiple }
  data={ ele.options }
  options={ opts }
  onSelect={ this.handleChange }
/>;

@chrisrichard any pointers here?

v-Zer0 commented 7 years ago

dimak @rkit @gazpachu @chrisrichard

To simplify my example from yesterday:

  selectChange(e) {
    console.log("Select changed");
    this.setState({...this.state});
  }

  render() {
    console.log("CALLING RENDER");
    return (
      <Select2 data={ [{text: "foo", id:1}] } onChange={ this.selectChange } />
    )
  }

When an option in the dropdown is selected, the results in console:

screen shot 2017-04-14 at 9 58 39 am

So it looks like there is an issue with the onChange event being fired too often

rkit commented 7 years ago

@cclifford3 thanks. I'm working on a fix.

v-Zer0 commented 7 years ago

@rkit thank you! It looks as if I avoid set state calls inside the select onChange function I can do local book keeping of the selected value.

There is another issue though with multi select onChange function. It would seem as if the event is fired before the value is added to the multi select. Because when you check the value in the onChange function you will only get the value that was selected previously, not the newly selected value.

screen shot 2017-04-16 at 12 03 05 pm
v-Zer0 commented 7 years ago

@rkit @gazpachu @chrisrichard

More on the multi select issue: I have built a work around using a method that will get all the selected options from the underlying select element and I just use that now in the onChange

screen shot 2017-04-17 at 10 52 25 am

In the screenshot notice that the first time i select a value ('user-supplied') I get that value coming back in the onChange method. Then notice I select a second value ('user-supplied2'). The onChange then fires but only the first value is present in e.target.value but then notice that when I pass it through the following method (shown below) getSelectedValues you can see both values logged.

  static getSelectedValues(select){
    let result = [];
    let options = select && select.options;

    forEach(options, (opt) =>{
      if (opt.selected) {
        result.push(opt.value || opt.text);
      }   
    });
    return result;
  }

@rkit let me know when you have a fix for this functionality so I can rip out my patch code.

v-Zer0 commented 7 years ago

@rkit getting a little closer, I think this solution will work for me for now. I think the root of most of these onChange being fired too many times and other issues is the componentWillRecieveProps method gets the nextProps in with a nested array instead of a single array.

  componentWillReceiveProps(nextProps) {
    /* Inheritted props value and defaultValue are == [['values', 'here']]
       Not sure why this happens but I have updated the fuzzyValuesEqual method
       to return equal thus not firing an onChange when this is the case.
    */
    this.initialRender = false;
    this.updSelect2(nextProps);
  }
  // newValue is passed as an array with a single nested array.
  fuzzyValuesEqual(currentValue, newValue) {
    let nested = false;
    if (typeof newValue === 'object' && newValue.length === 1){
      if (isEqual(currentValue, newValue[0])){
        nested = true;
      }
    }
    return nested || (currentValue === null && newValue === '') ||
      shallowEqualFuzzy(currentValue, newValue);
  }

screen shot 2017-04-17 at 11 53 57 am

rkit commented 7 years ago

@cclifford3 Please, try a new beta release 1.0.4-beta4. This is a fix for onChange event. Regarding about getting multiple values, try to get values via ref

this.refName.el.val()
kapasipk commented 7 years ago

Same issue is observed for 1.0.4-beta5. Hangs on triggering onChange.

Console errors :

Failed prop type: Invalid prop `value` supplied to `Select2`.
ReactDebugTool.js:29 Uncaught RangeError: Maximum call stack size exceeded(…)
v-Zer0 commented 7 years ago

@kapasipk you should try this - https://github.com/JedWatson/react-select

Works much better for me than trying to debug this select2 wrapper.

kapasipk commented 7 years ago

@cclifford3 Thanks, worked like a charm!

apurvgandhwani commented 7 years ago

Any fix for Select2. I am using react-select2-wrapper and I am getting the same hanging of page.

iashraful commented 7 years ago

This problem just f**ked my whole night. :-1: :-1: :-1:

austingayler commented 7 years ago

+1 for using react-select

BakytzhanAkzhol commented 6 years ago

Hello, Maybe i have fix tihs bug. It starts to work incorrectly if the value("value") is incorrect and not in the list("data"). In the beginning it worked well. But after changing the database, I forgot to fix the first value that is given to this component. v:1.0.4-beta6

BakytzhanAkzhol commented 6 years ago
<Select2
         value={item.value}
         defaultValue={1}
         data={carsList}
         id={index}
         onOpen={() => this.setState({isSelectOpen: true})}
         onClose={() => this.setState({isSelectOpen: false})}
         onChange={e => {
               if (this.state.isSelectOpen) {
                       this.handlerChangeInput(e);
                       this.setState({isSelectOpen: false})
                }
            }}
   />

This solved my problem.

adrazek commented 6 years ago

Sorry @rkit but I changed for react-select in about 1 hour and all works fine.

montaserfzy commented 6 years ago

Very Bad **** The same problem in updated version :1.0.4-beta6. Don't tried to download this plugin

montaserfzy commented 6 years ago

Moving fast to react-select

CWSites commented 5 years ago

For anyone that comes here, I'm using onChange without an issue on latest build. and react-select is trash. I switched to it for a different reason, spent a week getting it to work and found that IT was running an infinite loop in v2 (latest).

montaserfzy commented 5 years ago

to solve it onChange={_ => { if (_) { //your code } }}

suresh-jbt commented 4 years ago

@dimak @chrisrichard @CWSites @gazpachu @cclifford3 Try this way it will works :100:

const {selectedSkills} = this.state;
<Select2
  multiple
  value={selectedSkills}
  onOpen={() => this.setState({isSelectOpen: true})}
  onClose={() => this.setState({isSelectOpen: false})}
  data={skills}
  ref={(e) => { this.skillsRef = e; }}
  onChange={this._onSelectSkill}
  onUnselect={({target: {value}}) => {
    selectedSkills.splice(selectedSkills.indexOf(value), 1);
    this.setState({selectedSkills});
  }}
_onSelectSkill = ({target: {value}}) => {
    if (this.state.isSelectOpen) {
      this.setState({selectedSkills: this.skillsRef.el.val(), isSelectOpen: false})
    }
  }
shahmanish877 commented 3 years ago

Anyone has solution for this? I can't trigger change as it freezes.