Closed mnpenner closed 8 years ago
just came across the same issue
class X extends React.PureComponent {
render() {
const { object } = this.props;
return (
<li>
{ object.name }
{ this.renderIcons(object) }
</li>
);
},
*renderIcons(object) {
if (object.x) yield (<Icon text={ object.x } />);
/* more conditons */
}
}
my current workaround is to wrap it with Array.from
{ Array.from(this.renderIcons(object)) }
The Array.from
solution seems to be the correct way to handle this. I don't think React should automatically iterate to completion any generator function it gets, because there's no guarantee that the generator will terminate. I think it's better to keep your intent explicit by creating an array from the generator first, since there's no such thing as an infinite array. Introducing values that are potentially non-finite to React's rendering algorithm would not be a good decision, in my opinion.
@ksmithbaylor Something like could be useful if/when React has integrated layout. A component could render an infinite scroll list (like facebook news feed) as a generator (or something similar), and React could render as much of it as is required to fill the user's screen. Otherwise, the component wouldn't know how big of an array to generate, and so it would need to always over-estimate, thus wasting CPU and memory.
i found #2296 a while ago, but i havn't had the time yet to check, if this is just an issue with the transpiling to ES5 or an actual bug.
@jimfb That's an interesting use case I hadn't thought of!
Right now we require that any iterables yield the same children each time you iterate through them. Otherwise we can't do diffing, etc. properly. I don't think it's likely this will change soon and it's likely that React would internally do basically an Array.from call so writing it in explicitly like suggested above is a reasonable choice.
As shown it seems like generally a bad idea as it breaks Reacts implicit indices, so if the result ever changes you must be sure to key everything explicitly. I also fail to see the point, the only thing React can do is keep calling it until its done, so the generator doesn't really provide any benefit as far as I can tell, it becomes equivalent to:
function() {
var result = [];
result.push(...);
result.push(...);
return result;
}
But slower and less idiomatic, so what's the point of using generators as you show? Technically it probably makes sense as it's just another form of iteration.
@syranide
so what's the point of using generators as you show?
It's just shorter is all. Your example would shrink to:
function*() {
yield ...;
yield ...;
}
Is it really slower? I know there's some overhead with regenerator, but in a couple years when browser support is good, would it still be slower than an array?
it breaks Reacts implicit indices
Are implicit indices even a good thing? I mean, they're nice for prototyping because I can just ignore the warnings and slowness, but in practice everything needs a key anyway, doesn't it? In fact, using a generator would discourage me from just using the array index (because there isn't one) which usually ends up being bad choice because elements shift up and down the array.
@spicyj
Right now we require that any iterables yield the same children each time you iterate through them.
They would yield the same children unless the state/props change. This isn't fundamentally different than arrays is it?
I think @ksmithbaylor has a decent point about generators that never terminate. However, React could handle this better than Array.from
does. Array.from
will just crash your browser, React could stop after 10K iterations or so (or X milliseconds) and throw an error.
@jimfb's idea is interesting, but I imagine any kind of infinite list or table would be handled by the specific component anyway, no?
Anyway, I respect the core team's decision on this. It's just syntax sugar.
I meant, React already iterates through any iterable more than once which seems incompatible with generators.
It's just shorter is all.
IMHO, sure, shorter syntax is definitely preferable to some extent, but it's important not to ignore the purpose behind the functionality either. Generators are a special class of functions (i.e. resumable functions), and not just simple "iterables", they're similar but not the same.
Is it really slower? I know there's some overhead with regenerator, but in a couple years when browser support is good, would it still be slower than an array?
It will almost certainly always be slower. Generator functions require their own stack which must be allocated and stored on the heap (rather than the stack) and involves restoring CPU registers every time its called. Its imaginable that it could be optimized away in some circumstances (i.e. when you don't use them async), but considering the complexity and dynamic nature of JS it seems very unlikely.
Are implicit indices even a good thing? I mean, they're nice for prototyping because I can just ignore the warnings and slowness, but in practice everything needs a key anyway, doesn't it? In fact, using a generator would discourage me from just using the array index (because there isn't one) which usually ends up being bad choice because elements shift up and down the array.
Implicit indices work perfectly for everything but dynamic lists. Generator functions may promote a behavior where people generate "static" content the same way (like someone did in this very issue). Also they're not primarily about performance, it's about maintaining correct behavior for stateful/animating components.
We're going to start warning about this. https://github.com/facebook/react/pull/13312
Do you want to request a feature or report a bug?
Feature
What is the current behavior?
Generators are silently discarded
If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via https://jsfiddle.net or similar (template: https://jsfiddle.net/reactjs/69z2wepo/).
This is allowed:
This should be legal too:
Would be even better if the trailing
()
wasn't required, and it just executed the function.What is the expected behavior?
Both examples should generate:
This would make it much easier to write
if
conditions in the middle of a JSX block. Here's a snippet of how this can be used from my current project:Notice how I can write
if
conditions andswitch
statements without having to instantiate an empty array, push elements into it as needed, and then return an array at the end -- generators take care of all of that for you.Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?
n/a