Iterators are not supported inside Component.render

[askoretskiy]

What is the current behavior?

When you provide a list of React components -- its works. When you provide an iterator of React components -- it does not work.

This works:

const App = props => (
    <ul>
        {[...elements()]}
    </ul>
);

const elements = function* () {
    yield (<li>1</li>);
    yield (<li>2</li>);
    yield (<li>3</li>);
};

Doesn't work:

const App = props => (
    <ul>
        {elements()}
    </ul>
);

const elements = function* () {
    yield (<li>1</li>);
    yield (<li>2</li>);
    yield (<li>3</li>);
};

Unfortunately, I couldn't make it run on JSFiddle, maybe it has no support of iterators.

What is the expected behavior?

It should render exactly the same version as list-based one:

• 1
• 2
• 3

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

react-dom 16.0.0 and 15.x, browser-independent (I tried in Firefox 56).

[gaearon]

Did it work in 15?

[askoretskiy]

No

[mcjazzyfunky]

Maybe these fiddles may only work in modern browsers:

Please compare the results of

React 15.6.2 (working fine):
https://jsfiddle.net/8hm69uyo/

with the results of React 16 (not working properly):
https://jsfiddle.net/0vgf7tvs/

[gaearon]

Can you look into why this is not working? The relevant code is in ReactChildFiber.js.

[mcjazzyfunky]

Okay I checked that in detail:
Result: In my opinion this is not a bug!

First: It's not a "React15 vs React16" thing but more like a "react.development" vs "react.production" thing.
Unfortunatelly, in those two JSFiddle examples above I've used the "production" modules for the React15 example and the "dev" modules for the React16 example.

If you use the React production modules, the example also works with React16:
https://jsfiddle.net/0vgf7tvs/2/

Reason:
React's "development" module does not use "createElement" directly (as the "production" module does) but uses a function called "createElementWithValidation" instead.

For details see:

react/packages/react/src/React.js

Line 47 in 365c2db

createElement: __DEV__ ? createElementWithValidation : createElement,

This "createElementWithValidation" function uses a local function called "validateChildKeys" which iterates each item of an array/iterable/iterator child to check whether the "key" attributes are set properly.

For details see:

react/packages/react/src/ReactElementValidator.js

Line 199 in 365c2db

var iterator = iteratorFn.call(node);

As an iterator can only be fully iterated once (in contrast to an "iterator aggregate" aka. "iterable") the items of an external iterator will not be rendered in the browser (because the second time when ReactDOM tries to iterate the iterator it will not get any results).

For those who are interested in more details check:
https://en.wikipedia.org/wiki/Iterator_pattern
and maybe google for "internal iterator vs external iterator"

In my personal opinion, it's ALWAYS a very bad idea to pass iterators around directly (though I know this is unfortunatelly and unnecessarily done quite often). Iterators always have an internal state that will be modified on each iteration step. This more or less hidden side effect will sooner or later be a source of trouble (like in this issue here).
If someone uses iterators directly then these iterators should only be used locally, in all other cases "iterator aggregates" should be used instead of "iterators" as "iterator aggregates" do NOT have something like an internal iteration state but will create a new internal iterator each time necessary.

Means:

Do NOT use:

const elements = function * () {
    yield 1;
    yield 2;
    yield 3;
}

Instead ALWAYS use something like

const elements = {
    [Symbol.iterator]: function *() {
        yield 1;
        yield 2;
        yield 3;
    }
}

I personally prefer to use some "Seq" class that is specialized for lazy sequencing (with "filter", "map", "reduce" etc.), which allows things like:

const elements = Seq.from(function *() {
    yield 1;
    yield 2;
    yield 3;
});

[gaearon]

As an iterator can only be fully iterated once (in contrast to an "iterator aggregate" aka. "iterable") the items of an external iterator will not be rendered in the browser (because the second time when ReactDOM tries to iterate the iterator it will not get any results).

Oh. I get it now. This is pretty subtle 😞

[askoretskiy]

Thanks, guys. I was using Map.values method that returns iterator instead of list.

Should results of this discussion be part of an official React documentation? Or I should fill another issue for this? It would be helpful to see the difference between DEV and PRODUCTION modes, see if iterators are supported and how to work-around for DEV.

[gaearon]

Is it possible for us to warn if you pass the wrong thing, somehow? I agree it's confusing. On the other hand, it's good that it ends up being broken in development mode because at least in this case you notice. It would've been worse if it worked in development but broke in production.

We could document it but I don't see a good place. If you have ideas please raise it on the docs repository.

[askoretskiy]

@mcjazzyfunky Could you please provide the source code of this Seq class? I try to develop it myself (with "filter", "map", "reduce" etc.) but it seems not that easy )

[askoretskiy]

Example of this Seq implementation:

class Seq {
    constructor(fn) {
        this[Symbol.iterator] = fn;
    }

    filter(fn) {
        let that = this;
        return new Seq(function* () {
            let i = 0;
            for (let value of that) {
                if (fn(value, i)) {
                    yield value;
                }
                i++;
            }
        });
    }

    map(fn) {
        let that = this;
        return new Seq(function* () {
            let i = 0;
            for (let value of that) {
                yield fn(value, i);
                i++;
            }
        });
    }
}

let val;

val = new Map([[1, 1], [2, 2], [3, 3], [4, 4], [5, 5]]);

console.log('all');
for (let v of new Seq(val.values.bind(val))) {
    console.log(v);
}

console.log('map ^2');
for (let v of new Seq(val.values.bind(val)).map(num => num ** 2)) {
    console.log(v);
}

console.log('filter odd');
for (let v of new Seq(val.values.bind(val)).filter(num => (num % 2))) {
    console.log(v);
}

Result:

all
1
2
3
4
5
map ^2
1
4
9
16
25
filter odd
1
3
5

[mcjazzyfunky]

@askoretskiy: I really like your Seq implementation.
If you also like to see some other implementations (as you have asked):

• Have a look at the Seq class in Facebook's "Immutable" library:
  http://facebook.github.io/immutable-js/docs/#/Seq
  Unfortunatelly (AFAIK) it does not have something like "Seq.from(function * () {....})" yet.
  But it's not a big deal to add that - maybe a you could open a feature request there or just implement a function like

const seq = createSeq(function * () {....});
// [Edit: removed stupid proposal]

• I have also implemented a Seq class by myself once.
  But very important: This is in alpha state and has never been used in production.
  It is part of a small library that will be completely rewritten in Typescript as soon as I will have time to do so.
  Mean: Feel free to copy code or ideas from that Seq implementation below, but do not use it directly nor that small library it is part of.
  Also please be aware that I wanted that Seq class also be useful in pure ES5 programs (without using a transpiler like Babel there). That's why that "[Symbol.iterator]()"-method may seem a little strange and overcomplicated.
  If you'll always use ES2015 and higher than this is surely an overkill and the way your have done it in your example above is surely the better solution.

https://github.com/js-works/js-essential/blob/785eaf6b6fd6fbdd7ef863396306924733fc0a7e/src/main/Seq.js
https://github.com/js-works/js-essential/blob/785eaf6b6fd6fbdd7ef863396306924733fc0a7e/src/test/SeqTest.js

Also please be aware that my implementation is not optimized for Seqs based on arrays (AFAIK, Facebooks Seq classes are optimized for that):

// create an array with 1000 undedfined elements
const array = new Array(1000); 

// the following works, but does not work in constant time,
// as there's no optimization implemented (yet) for the array case.
const size = Seq.from(array).count();

Also please do not copy that ".bind(this)" stuff, your "that = this" solution is surely much better than using bind.

Last but not least:
Maybe you are interested in the following benchmarks which show the performance difference between different ways of iteration:
https://jsfiddle.net/7v7zo268/2/

[askoretskiy]

@mcjazzyfunky Thanks, I may extend my implementation with some new tricks ))

Yes, one could write really lean code if he opts in to use ES2015.

[simonbuchan]

Pasting this here, which I incorrectly pasted as a solution to #11864 (which should have a similar fix on the the render result):

React can fix this without too much juggling, essentially have createElementWithValidation() pre-iterate arguments, something like:

// EDIT: Don't treat component types that are for some reason iterable like arrays!
// EDIT: What about iterable items of props.children?
const fixedArgs = Array.prototype.slice.call(arguments, 1).map(arg => {
  const iteratorFn = arg[ITERATOR_SYMBOL]; // actually check all the polyfills
  if (!iteratorFn) return arg;
  return [...iteratorFn()]; // iterators (as opposed to iterables) can only give their results once.
});
const element = createElement.apply(null, fixedArgs);
...
validateChildKeys(fixedArgs[i], type);
...

[gaearon]

We're going to start warning about this.
#13312