moroshko / react-autosuggest

WAI-ARIA compliant React autosuggest component
http://react-autosuggest.js.org
MIT License
5.97k stars 587 forks source link

Allow Immutable.Map (or Iterable) to be passed #252

Open cristian-sima opened 8 years ago

cristian-sima commented 8 years ago

Let's take this code:

const mySuggestions = Immutable.Map();

...

return <Autosuggest
      suggestions={mySuggestions}
      ...
    />);

This code does not work. I got:

app.js:9386 Warning: Failed prop type: Invalid prop `suggestions` of type `object` supplied to `Autosuggest`, expected `array`.
    in Autosuggest (created by Connect(Autosuggest))
    in Connect(Autosuggest) (created by AutosuggestContainer)
    in AutosuggestContainer (created by ItemNameField)
    in div (created by ItemNameField)
    in ItemNameField (created by ConnectedField)
    in ConnectedField (created by Connect(ConnectedField))
    in Connect(ConnectedField) (created by Field)
    in Field (created by Row)
    in td (created by Row)
    in tr (created by Row)
    in Row (created by renderItems)
    in tbody (created by renderItems)
    in table (created by renderItems)
    in div (created by renderItems)
    in renderItems (created by ConnectedFieldArray)
    in ConnectedFieldArray (created by Connect(ConnectedFieldArray))
    in Connect(ConnectedFieldArray) (created by FieldArray)
    in FieldArray (created by ItemsBox)
    in ItemsBox (created by Connect(ItemsBox))
    in Connect(ItemsBox) (created by Form)
    in div (created by Form)
    in form (created by Form)
    in Form (created by Form(Form))
    in Form(Form) (created by Connect(Form(Form)))
    in Connect(Form(Form)) (created by ReduxForm)
    in ReduxForm (created by AddInvoice)
    in div (created by AddInvoice)
    in div (created by AddInvoice)
    in AddInvoice (created by Connect(AddInvoice))
    in Connect(AddInvoice) (created by withRouter(Connect(AddInvoice)))
    in withRouter(Connect(AddInvoice)) (created by RouterContext)
    in div (created by Sidebar)
    in div (created by Sidebar)
    in div (created by Sidebar)
    in Sidebar (created by Sidebar)
    in div (created by Sidebar)
    in Sidebar (created by Connect(Sidebar))
    in Connect(Sidebar) (created by CompanyPage)
    in CompanyPage (created by Connect(CompanyPage))
    in Connect(CompanyPage) (created by RouterContext)
    in RouterContext (created by Router)
    in Router (created by Root)
    in Provider (created by Root)
    in Root
    in AppContainer

However, if I do

const mySuggestions = mySuggestions.toJS()

it then works, however there is a big problem.

I will lose the benefits of Immutable if I change the code to Immutable.List().toArray (or toJS)) as presented here

Also here.

Other comment about this:

Good question. I Wouldn't do it in the store though, since then you lose the abillity to do a simple object comparison (prevState !== this.state) if you'd want to optimize rendering with shouldComponentUpdate

So, in the end, the request is to support Immutable types

cristian-sima commented 8 years ago

@moroshko Any thought on this?

cristian-sima commented 8 years ago

Following the conversation from here https://github.com/pburtchaell/react-notification/issues/92 it seems there is no simple, free dependent way of creating this.

However, I am thinking of another way of approaching this problem.

We can create a new branch called support-immutablejs which contains this dependency react-immutable-proptypes (and implicit the immutable dependecy). Taking into account that those interested in using support-immutablejs will already use immutable, they will not find it as a problem.

For all others, there will be the master branch which is free dependent. For any updates on master, we can update the support-immutablejs branch from master.

What do you think?

merk commented 8 years ago

Using .toArray() on an immutable iterable will still keep the entries as immutable, which is what I've done.

The getSuggestionValue and renderSuggestion functions can handle immutable entries fine.