-
Notifications
You must be signed in to change notification settings - Fork 47.4k
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
Allow generators (that yield components) in place of arrays #7536
Comments
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(this.renderIcons(object)) } |
The |
@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:
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. |
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?
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.
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 @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. |
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.
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.
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. |
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
The text was updated successfully, but these errors were encountered: