Inferno: Support for rendering ImmutableJS Maps, Lists, Records, etc.

[Vpr99]

I'm in the process of implementing ImmutableJS in one of our services and I noticed that when iterating over the items in a List, Inferno can't render the Immutable List directly, since it's not a JS array.

Here's the Stack Overflow discussion that led me to this conclusion:
http://stackoverflow.com/questions/27864706/when-to-use-tojs-with-immutable-js-and-flux

Here's the stack trace:

Uncaught (in promise) TypeError: Cannot read property 'key' of undefined
    at applyKeyIfMissing (inferno.node.js:75)
    at normalizeVNodes (inferno.node.js:143)
    at normalizeChildren (inferno.node.js:150)
    at normalize (inferno.node.js:217)
    at createVNode (inferno.node.js:268)
    at eval (app.jsx:88)
    at eval (immutable.js:3018)
    at List.__iterate (immutable.js:2208)
    at IndexedIterable.mappedSequence.__iterateUncached (immutable.js:3017)
    at seqIterate (immutable.js:606)

Code that caused the issue:

{sections.map(section => (
  <section>
    <div className="bb b-porcelain mt5 pb2">
      <h1 className="f3 fw6 mv0">{section.title}</h1>
    </div>
  </section>
))}

The workaround I'm currently using is to just run toJS() on the resulting Immutable List:

{sections.map(section => (
  <section>
    <div className="bb b-porcelain mt5 pb2">
      <h1 className="f3 fw6 mv0">{section.title}</h1>
    </div>
  </section>
)).toJS()}

While this works, it would be nice to eliminate this operation because it's pretty expensive. This obviously isn't a huge deal, but I'd be curious to hear your thoughts.

[trueadm]

@Vpr99 This has come up in past so it's a good question. The problem is that we found iterating over Immutable JS collections to be extremely slow compared to using plain JS objects. Behind the scenes, Inferno would also have to do a similar toJS() operation in order to properly work with the data, or to fully work with ImmutableJS's objects, there'd need to be a huge amount of code added to deal with all the different objects that can be passed in. :/ Sorry.

[longlho]

@Vpr99 toArray() is your friend

[jdalton]

I ran into this a few minutes ago while trying to switch Lodash docs over to Inferno.
I thought the win with ImmutableJS was object comparison speeds. Is that not necessarily the case?

[Vpr99]

@jdalton this is too funny. We were just doing an exploration into whether Immutable was worthwhile. We're gonna pass on Immutable for now because the cost of toJS() functions was pretty high. Alex Faunt brings up some good points.

Not being able to spread across props passed to a component is pretty annoying IMO:

//  Say goodbye to this :(
<Component
  name="foo"
  {...otherProps}
/>

That being said, we liked some of the Object functions that Immutable offers. I'm refactoring to use a certain library as we speak 😄

[longlho]

@jdalton yes object ref comparison is certainly sweet but pretty much everything else about ImmutableJS is slower :(

[jdalton]

@Vpr99

this is too funny. We were just doing an exploration into whether Immutable was worthwhile.

Cool! I use Lodash's site as my sandbox to explore things too 😎

[xirzec]

I just got hit with this while using Immutable.List, and having to do .toArray() myself is a bit annoying, since React supports it out of the box.

The real problem for me isn't that Inferno should do custom work for ImmutableJS, but that it should support ES6-style iterables. FWIW, preact has a PR to support iterables: https://github.com/developit/preact/pull/476

[trueadm]

@xirzec we'd be happy to take a PR that deals with this in a performant way. In their current state, itterables are slow on runtime performance though.

[xirzec]

@trueadm cool, I'll take a look into this in the not too distant future. 👍

I want to make sure that I understand the performance concern correctly. It makes total sense that if the iterable itself is slow to enumerate, there's nothing Inferno can really do about it. Is there another concern that by supporting iterables that inferno would be slower for other values?

Naively, my thought is that a test for checking if the value contains a Symbol.iterator property wouldn't add too much performance overhead to non-iterator cases. Is there something I'm missing?

[trueadm]

@xirzec I profiled this exact use case a while back and performance on looping over arrays dropped 80% on hot paths due to constantly checking for a bad property on an inline cache. It may have improved since though. Generally speaking, itterables are a nightmare for any JavaScript engine to make performant due to all the edge cases that can occur!

[Havunen]

Support for this has been added into inferno-compat. It will be available in next release. In future we could consider moving this into core behind inferno.options that way it could be done more efficiently than now. Closing.

[jdalton]

Whoa awesome! 🤘