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

React + Immutable.JS: Best practice for sending Immutable.Map as props to children components? #667

Closed
liorbrauer opened this issue Oct 21, 2015 · 19 comments

Comments

@liorbrauer
Copy link

liorbrauer commented Oct 21, 2015

Hi,

I'm doing the following inside a render() function on my Component:

posts.map((post) => 
  <PostComponent post={post} key={post.get('id')} />
)

Where posts is an Immutable.Map, and each item in it post is also a Map.

This seems to work but I'm getting the following warning:
Warning: Using Maps as children is not yet fully supported. It is an experimental feature that might be removed. Convert it to a sequence / iterable of keyed ReactElements instead.

What alternative approach is suggested? I don't want to lose Immutable.Map's functionality like .hasIn and .getIn.

Thanks!

@liorbrauer liorbrauer changed the title React + Immutable.JS: Best practice for Sending Maps as props to children components? React + Immutable.JS: Best practice for sending Immutable.Map as props to children components? Oct 21, 2015
@eyston
Copy link

eyston commented Oct 22, 2015

Immutable.Map#map returns an Immutable.Map so you are giving react a Map of children. You should be able to do:

posts.values().map(post =>
  <PostComponent post={post} key={post.get('id')} />
)

I don't know if you'd need to append an toArray or not. I also get values and valueSeq confused :)

@liorbrauer
Copy link
Author

@eyston Thanks for the help! 👍

Yeah, it's valueSeq and then toArray is not even needed for the iteration:

posts.valueSeq().map(post =>
  <PostComponent post={post} key={post.get('id')} />
)

I guess I'm still a bit confused at to what an Immutable.Seqis. Is there a good primer/tutorial on all the different Immutable data types (other than the regular documentation)?

@eyston
Copy link

eyston commented Oct 22, 2015

I don't know of one. I came from clojure so I just guess a lot when it comes to Immutable.js :).

I'm actually not sure why so many functions have a x and xSeq version. I generally use x and then get told map is not a function and see if an xSeq version exists.

Guessing from clojure ... Immutable.Seq is an immutable lazy list. It is the base most of the other data structures implement. So when you call something like valueSeq the contract is it is going to return a Seq but that might take the form of an eager List of all values, or it might be a lazy Seq where it only does the work to pull a single value out at a time. The thing is, you don't care. As long as it implements the Seq interface you can map away.

@liorbrauer
Copy link
Author

@eyston Thanks for the explanation 👍

Hopefully we'll get a more thorough documentation and some more examples from Immutable.js

@agilgur5
Copy link

+1 for the solutions described here!

@ccerrato147
Copy link

+1

@anhhh11
Copy link

anhhh11 commented Jan 6, 2016

+1 solutions

posts.valueSeq().map(post =>
  <PostComponent post={post} key={post.get('id')} />
)

@cettoana
Copy link

+1 thanks

@rikkuporta
Copy link

If you don't want to lose the key-value relationship you can also use entrySeq(), which is like a combination of keySeq and valueSeq:

posts.entrySeq().map(o => 
     <Post value={o[1]} key={o[0]} />
)

@vinnymac
Copy link
Contributor

vinnymac commented May 19, 2016

entrySeq().map and valueSeq().map did not throw a warning and produced a IndexedIterable.

When using mapEntries() I get the warning the OP originally had in the first comment. It produced a OrderedMap, which is the data structure I mapped with. If you attempt to use mapEntries() hopefully this provides a hint as to why you may be receiving that warning.

I settled on using this syntax in the end.

posts.entrySeq().map( ([key, value]) => 
  <Post key={key} value={value} />
)

fk pushed a commit to fk/bu-tumblr that referenced this issue Oct 12, 2016
* Bump rupture to 0.6.2.
* Bump gulp-sourcemaps to 2.0.1.
* app/alt.js: Add support for https://github.com/goatslacker/alt-devtool.
* app/routes.js: One-line import.
* Load "Space Mono" 400 from Google Fonts.
* app/api/index.js: Remove obsolete import.
* Remove the last remaining image – favicon should suffice to demonstrate how to handle static assets.
* Add .post-footer className.
* Remove Brooklyn United address in footer.
* Remove mastheadColor from AppStore.
* Fix indention in various files.
* app/components/Header/index.js: Tidy consts.
* Various: Move defaultProps, propTypes into class (thanks to es7.classProperties).
* app/components/Posts/index.js: Fix "Using Maps as children is not yet fully supported. It is an experimental feature that might be removed. Convert it to a sequence / iterable of keyed ReactElements instead." warning; @see immutable-js/immutable-js#667 (comment)
* Fix NoteBox avatar size for retina displays, touch styles along the way, add "reblogged from blogname" to "reblog" notes.
* setInnerHTML for PostQuote .source.
* Close navigation when clicking a react-router <Link> in it.
* Simplify navigation CSS.
@luiscarlosjayk
Copy link

luiscarlosjayk commented Apr 15, 2017

You could instead of passing Immutable Collections such as Maps and Lists to your React components, pass normal js objects and arrays, respectively. Thay way, it's easier to read and understand, and you remove dependency/boilerplate of Immutable from your React components.

Remember that Immutable Map each keys and values method return an iterator, which can be converted to an array using the spread operator inside brackets.

This way, using a normalized state for a Todos demo app e.g:

const stateShape = Record({
    todos: Map({
        1: {
            id: 1,
            name: 'Eat a lot'
        },
        2: {
            id: 2,
            name: 'Take span'
        },
    })
})

I'd suggest a mapStateToProps function like this:

[...] // some other code
const getProjectsArray = state => {
    const todos = state.get('todos')
    const todosArray = [...todos.values()]
   
   return todosArray
}

const mapStateToProps = (state) => {
    projects: getProjectsArray(state)
}
[...] // Some other code

Notes:

@matiasgarcia
Copy link

Wouldn't that, however, kill any performance optimization done on react components based on ImmutableJS?

@andrewcapodieci
Copy link

andrewcapodieci commented Aug 17, 2017

@Ciul that's going to cause unnecessary rerenders every time your state changes because you're generating a new object every time, unless you implement shouldComponentUpdate with a deep comparison, which can be costly depending on your object.

@ghost
Copy link

ghost commented Oct 9, 2017

posts.map((post) => <PostComponent post={post} key={post.get('id')} />).toArray();

@acusti
Copy link
Contributor

acusti commented Oct 14, 2017

@wSLecHayfIeNdock That behavior of toArray() that your example relies on, where it just takes the values of an immutable Map and drops the keys, has been removed in Immutable v4: #1340

The immutable v4 version of your snippet:

posts.map((post) => <PostComponent post={post} key={post.get('id')} />).valueSeq().toArray();

@luiscarlosjayk
Copy link

@andrewcapodieci That's why I suggest using Reselect. This will make a kind of cache and will return same array if not changed.

@bogdaniy
Copy link

@Ciul are your transforming your immutable objects toJS() in selectors or using .get() syntax in jsx?

@CMCDragonkai
Copy link

CMCDragonkai commented Jun 16, 2019

I have a strange request here. I'm also using Typescript, and converting to using entrySeq for React lists seemed like the right idea. Especially as https://reactjs.org/docs/lists-and-keys.html recommends getting the key as well.

However what is the type that should be set for the component that receives a Seq? It shouldn't be an Immutable Seq. It should be some sort of "Functor" type that supports map. Because the component can also take just an array as well.

Furthermore, this is basically React rendering of a Map. Does converting to an entrySeq mean that every time, the Map changes, we have to reproduce a list so that React can render it. It seems rather inefficient for React, and it seems that it would be better if there was some key-value structure that was also always iterable without needing to be converted into a different in-memory representation.

I want to use some generic functor type instead of immutable Seq so to avoid the dependency of react components on immutablejs.


To clarify what I'm asking:

Some examples:

m = new Map()

// this doesn't work because React doesn't know how to render the Map type
jsx= <ul>{m.map(v => <li>{v}</li>)}</ul>

// this works
jsx = <ul>{m.entrySeq().map([k, v] => <li key={k}>{v}</li>)}</ul>

// this works, but why bother converting to an array needlessly incurring another O(n) operation
jsx = <ul>{m.entrySeq().toArray().map((v, k) => <li key={k}>{v}</li>)}</ul>

Assuming that the JSX expression does an O(n) operation <li> elements in the virtual DOM. Using toArray() is just an extra O(n) operation that is not needed as React is capable of rendering the Seq type.

Questions:

  1. If I want to just use Seq, is there some sort of Functor type that I can use in typescript that works for both Arrays and Seq?
  2. Is there a way to avoid doing an O(n) operation to reproduce the <li> elements each time I add an entry into the Map? Even in the Virtual DOM, an incremental change to the Map should be an incremental change to the JSX virtual DOM and thus an incremental change to the real DOM.

@usman-subhani
Copy link

usman-subhani commented Sep 8, 2019

Try reduce()

posts.reduce((jsxArray, post, itemKey) => {
  jsxArray.push(
    <PostComponent post={post} key={post.get('id')} />,
  );
  return jsxArray;
}, []);

char0n pushed a commit to swagger-api/swagger-ui that referenced this issue Jun 22, 2020
char0n pushed a commit to swagger-api/swagger-ui that referenced this issue Jun 22, 2020
Avoid mapping Immutable.Map as React children.

Co-authored-by: Vladimir Gorej <vladimir.gorej@gmail.com>

Ref immutable-js/immutable-js#667
mattyb678 pushed a commit to mattyb678/swagger-ui that referenced this issue Jun 24, 2020
Avoid mapping Immutable.Map as React children.

Co-authored-by: Vladimir Gorej <vladimir.gorej@gmail.com>

Ref immutable-js/immutable-js#667
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

16 participants