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

If cache is not supplied, props._cache is common across all Async instances. #1342

Closed
RossRogers opened this issue Nov 2, 2016 · 4 comments

Comments

@RossRogers
Copy link

If you create two AsyncCreatable instances, they will share a cache, because defaultProps has a shared, default cache instance across all creations of Select.Async.

Example jsfiddle

Shouldn't the Async constructor compare against the defaultProps.cache as in the following or somehow deduplicate multiple instances of Async ?

constructor(props) {
  super(props);
  if (this.props.cache === defaultProps.cache) {
    this.props.cache = { ...this.props.cache};
  }
  [...]

or define defaultProps.cache to return a new instance on each access like:

var defaultProps = {
  [...]
};
Object.defineProperty(defaultProps, "cache", { get: function () { return {}; } });

Apologies if this is expected behavior, however it does seem to violate the principle of least astonishment, at least for me.

Pertinent example code:

class Example extends React.Component {
  constructor(props) {
    super(props);
    this.state = {
      ref1: null,
      ref2: null

    };
    // can't set state withing ref callback, so doing this prevents an infinite loop
    setTimeout(() => { 
      this.setState( {...this.state, ref1: this.ref1, ref2: this.ref2});
    }, 2000);
  }
  render() {
    return <div>
      <Select.AsyncCreatable
        placeholder="1st Instance"
        valueKey="id"
        loadOptions={ text => { then: clbk => clbk([ {label:'1st results', id:1} ]) } }
        ref={ r => this.ref1 = r }
      />
      <Select.AsyncCreatable
        placeholder="2st Instance"
        valueKey="id"
        loadOptions={ text => { then: clbk => clbk([ {label:'2nd results', id:1} ]) } }
        ref={ r => this.ref2 = r }
      />
      <tt>ref1.props._cache === ref2.props._cache</tt> is {
          this.state.ref1 && this.state.ref2 ?
          (this.state.ref1.props._cache === this.state.ref2.props._cache ? 'true' : 'false'):
          'loading'}
    </div>;
  }
};

ReactDOM.render(
  <Example/>,
  document.getElementById('container')
);
@bmv437
Copy link

bmv437 commented Nov 11, 2016

Yea, I just ran into this issue and had to disable caching across my Select.Async fields.

@TheSharpieOne
Copy link
Contributor

TheSharpieOne commented Nov 17, 2016

Dupe of #1236. This was fixed in #1241 and is awaiting release

@atsu85
Copy link

atsu85 commented Feb 7, 2018

@JedWatson, @RossRogers - as i understand, this issue should be closed, as the PR from @bvaughn was merged

@bladey
Copy link
Contributor

bladey commented May 27, 2020

Hello -

In an effort to sustain the react-select project going forward, we're closing old issues / pull requests.

We understand this might be inconvenient but in the best interest of supporting the broader community we have to direct our limited efforts to maintain the latest version.

If you feel this issue / pull request is still relevant and you'd like us to review it, please leave a comment and we'll do our best to get back to you.

@bladey bladey closed this as completed May 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants