Bug: Iterator as JSX children doesn't work right

[Jack-Works]

React version: 17

Steps To Reproduce

function App() {
    const x = [<h1>a</h1>, <h2>a</h2>].values()
    return <div>{x}</div>
}

Link to code example:

https://codesandbox.io/s/react-playground-forked-jv4p5?fontsize=14&hidenavigation=1&theme=dark

The current behavior

Nothing rendered.

React consumes the iterator twice so the items are gone. The first consume is to validate the JSX, the second consume is collecting them as children.

The expected behavior

Render JSX items in the list.

Why I'm considering this case is because I'm investigating how React will work with the ES Number.range proposal.

It seems like they can work in this way:

function App() {
return <>{Number.range(0, 10).map(x => <h1>{x}</h1>)}</>
}

Without appending .toArray()(iterator helper proposal) to the end. It seems like React does support iterators but used it in the wrong way.

[sebmarkbage]

React only accepts immutable input. A one-shot iterator wouldn't be immutable and so is not supported. Only iterables that can produce multiple iterators over the same immutable input are supported.

A user space description of why this is important might be something like this:

function Foo({ menuItems: ... }) {
  let [state, ...] = useState(...);
  return menuItems.map(...);
}

This component itself might rerender multiple times without the parent doing so. Therefore if you pass a one-shot iterable in to this component - rerendering wouldn't work.

It'd suggest you explore Number.range returning an iterable that can produce multiple iterators over the same range since the input range is immutable. (I think we'd probably want to object to a collection approach that is one shot and not compatible with an immutable approach. Like you shouldn't need to use toArray for something like this since it's inefficient.)

[gaearon]

Can we somehow detect a one-shot iterator? Maybe some check like x[Symbol.iterator]() === x should warn when true?

[sebmarkbage]

I really think it was a mistake for .values() to behave this way when it could've been an iterable. (.NET got it right as usual.)

[Jack-Works]

It'd suggest you explore Number.range returning an iterable that can produce multiple iterators over the same range since the input range is immutable.

Yes, we explored this a lot but it's hard to change to iterable instead of one-shot iterator (see discussion in tc39/proposal-iterator.range#17, tc39/proposal-iterator.range#41 and tc39/proposal-iterator.range#42)

[sebmarkbage]

At the end of the day, with a reusable iterable you can always get a one-shot iterator out of it but you can't get a reusable out of a one-iterator except by allocating the full memory at which point the whole point of iterables are defeated.

React is a good example of a use case where you can't use a one-shot. It's a very common paradigm in JS.

Since you can't use a one-shot and toArray defeats the purpose, your only option will be to use a third party user space library. If this pattern is useful enough, it'll eventually get added to the language as the reusable iterable form.

So it seems inevitable that either there's two APIs or just reusable iterable.

[Jack-Works]

I definitely support the iterable but others don't agree.
And whatever semantic the range proposal choose, if you want to make it useful in React, it's must be used with iterator helper. And the iterator helper proposal doesn't work with iterable, it's always returned a one shot iterator.

[sebmarkbage]

Unfortunately there's nothing we can do to support it but we can create a more descriptive error message.

[Jack-Works]

Can we somehow detect a one-shot iterator? Maybe some check like x[Symbol.iterator]() === x should warn when true?

Since react consume the iterator twice, react can warn if the second consume doesn't get the same array length as the first consume

[hax]

I totally agree with @sebmarkbage .

.NET got it right as usual.

Not only C#, but also Python, Ruby, Kotlin, Swift... all got it right.

---

I did a little research on the usage of lodash-range (which create an array, just like range in Python 2, and array of coz not a one-shot iterator but a reusable iterable), and found there is a common usage of lodash.range(start, end).map(i => (<div>{i}</div>)). For example some Material-UI official demos use this pattern.

So if range proposal eventually choose iterator semantic, it means React users very likely can't use such API in common cases and just bring frustration when they try to use new JS range API and found that not work as expect.

[hax]

And the iterator helper proposal doesn't work with iterable

We don't need to depend on iterator helper proposal, we could make range behave like immutable Array (like Tuple proposal do), so it could have map method which return an iterable. This is what ix library do, and in the research I found someone use ix.range(start, end).map(i => (<div>{i}</div>)) well, though most use lodash because lodash is far more popular than ix. I really think range proposal should consider ix path.

[ljharb]

@gaearon All iterators are one-shot, so you could warn on a plain object with a .next method - you could restrict it to built-in iterators if that's too broad, but it'd take a bit more code to do robustly. The check you suggested would also work for all built-in iterators.

I really don't understand the use case though - if you're passing an iterator to React, you're expecting React to consume every item, so why would you not reify it to an array first?

[hax]

if you're passing an iterator to React, you're expecting React to consume every item, so why would you not reify it to an array first?

@ljharb

Yes u should (convert it to array).

The problem is many people actually:

1. don't understand iterator are one-shot
2. don't know or forgot whether specific api (for example, values() or keys()) returns iterator
3. write component and consume the data which out of their control --- component users send the iterator because they have the problem (recursively :)

[sebmarkbage]

A lot of time it's more efficient to toArray at some intermediate points because it's worth materialize the memory over reexecuting it but not all. I don't think it's usually when it's passed into React itself but to an intermediate:

function App() {
  return <Bar numbers={Number.range(0, 100)} />;
}

function Bar({ numbers }) {
  let [state, ...] = useState(...);
  return <Foo items={numbers.map(n => {key: n, <div>{n}</div>})} />;
}

function Foo({ items }) {
  return <div>{items.map(item => <div key={item.key}>{item.children}</div>)}</div>;
}

In this case, it might be unnecessary to materialize the initial array and intermediate array. But it needs to be multi-shot since the inner component can rerender without rerendering a parent component.

But again, it's probably mostly best practice to materialize to an array when it passes a component boundary just because memoization is better than recomputation for updates.

[johtso]

The main situation where I'm bumping up against this limitation is when using Map and Set data structures for my state, and using something like iter-tools to iterate over Map.entries() to generate my elements. Having to materialise all of my Iterables into Arrays makes the JSX quite a bit more cluttered.

[stale]

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

[ljharb]

bump

[Jack-Works]

If we don't have progress on this, at least we can make this first.

but we can create a more descriptive error message.

[github-actions]

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

[ljharb]

bump

[github-actions]

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

[github-actions]

Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please create a new issue with up-to-date information. Thank you!