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

Creatable.js mutates props.options, effectively changing it in parent Component as well #1477

Closed
dannemanne opened this issue Jan 12, 2017 · 9 comments
Labels
issue/bug-confirmed Issues about a bug that has been confirmed by a maintainer

Comments

@dannemanne
Copy link

The component Creatable.js has the function createNewOption () which contains the following line

options.unshift(option);

When this executes, it will directly mutate the options property, something that not only seems like an anti-pattern for React, but can result in a changed value in the Parent component as well.

Given the following example, when onChange is being called after a new option has been created, the options property of the Parent component has already been changed.

class CreatableWrapper extends React.Component {
    static propTypes = {
        options:        React.PropTypes.array.isRequired
    }
    constructor(props){
        this.onChange = this.onChange.bind(this);
    }

    onChange(newValue) {
        const { options } = this.props;

        // When a new option has been created in Creatable, 
        // the options property here has been changed.
        console.log({ options });
    }

    render() {
        const { options } = this.props;

        return <Creatable {...{ options, onChange: this.onChange }} />
    }
}

Is this something that is a temporary but known behaviour that will change in the future? Is it a behaviour that is here to stay, or is it simple a bug?

@robertmaloney
Copy link

robertmaloney commented Jan 27, 2017

Definitely seems weird, here's a fiddle derived from your example: https://jsfiddle.net/robertmaloney/77r13rpm/

I would consider this a bug.

@bpartridge
Copy link

If we want the Creatable to "keep track of" the fact that things that were added are valid options, it should either:

  • Make the user/parent responsible for ensuring that options includes all things that have ever been passed to onChange, if that is the desired behavior. Simply removing the offending unshift call and updating examples/Creatable.js to append to this.state.options would do the trick.

  • Or, own its own state, and pass down something to the underlying Select like options: [...options, this.state.createdOptions] rather than unshifting. However, this should be optional; for instance, if you're reusing a form multiple times in a row without unmounting it, you may not want your custom value from last time to be autosuggested this time (i.e. if that data could be custom to each record).

I'd highly recommend the first approach; the second seems too "magical" for such a reusable component.

@jtbandes
Copy link

jtbandes commented May 10, 2017

+:100:. @bpartridge's comment above exactly matches my own thoughts after running into this issue. The "convenience" of Creatable doing its own unshift is outweighed by how difficult it becomes to implement any custom behavior. Also, whenever a user-created option is selected and then the set of options or selected option change externally, the component gets confused. (Note: I think this issue might be marginally related to #1547.)

My use case: I want the Creatable-"created" option to be ephemeral: if the user ever selects another option, their created option should disappear unless they type it and explicitly create it again. I was hoping to implement this with a custom onChange and onNewOptionClick, but even when these are customized it tries to unshift onto my options array. 😕

(Another solution to my problem would be if react-select had the ability to allow any text to remain in the text field, regardless of whether it matches an option. This behavior would be more similar to a combo box such as react-autocomplete.)

@agirton agirton added the issue/bug-confirmed Issues about a bug that has been confirmed by a maintainer label May 23, 2017
@bpartridge
Copy link

Following up on this. Would the maintainers welcome a PR that implemented the first approach I suggested?

Make the user/parent responsible for ensuring that options includes all things that have ever been passed to onChange, if that is the desired behavior. Simply removing the offending unshift call and updating examples/Creatable.js to append to this.state.options would do the trick.

@josh--newman
Copy link

josh--newman commented Jul 21, 2017

I'm using React 15.6.1 and seeing a full blown TypeError on this now. Has anyone else seen this?

Creatable.js:120 Uncaught TypeError: Cannot add property 12, object is not extensible
    at Array.unshift (<anonymous>)
    at Object.createNewOption (Creatable.js:120)
    at Object.onOptionSelect (Creatable.js:228)
    at Object.handleMouseDown (Option.js:51)
    at Object.ReactErrorUtils.invokeGuardedCallback (ReactErrorUtils.js:69)
    at executeDispatch (EventPluginUtils.js:85)
    at Object.executeDispatchesInOrder (EventPluginUtils.js:105)
    at executeDispatchesAndRelease (EventPluginHub.js:43)
    at executeDispatchesAndReleaseTopLevel (EventPluginHub.js:54)
    at Array.forEach (<anonymous>)

@jtbandes
Copy link

The secret to happiness: never upgrade any software

@Katarinich
Copy link

Still no updates?

It is kinda pretty serious issue. For example, I have the functionality to add a new option, and after I click clear button I expect to new option to be deleted from the input as well as from the option list. But because of it, the new options remains in the option list and there is nothing I can do about it.

@tomaass
Copy link

tomaass commented Dec 3, 2018

It's also a big issue if your options are immutable array. ( made my day :-D )

@bladey
Copy link
Contributor

bladey commented May 28, 2020

Hello -

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

We understand this might be inconvenient but in the best interest of supporting the broader community we have to direct our efforts towards the current major version.

If you aren't using the latest version of react-select please consider upgrading to see if it resolves any issues you're having.

However, if you feel this issue 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 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
issue/bug-confirmed Issues about a bug that has been confirmed by a maintainer
Projects
None yet
Development

No branches or pull requests

10 participants