Skip to content
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

Accept iterables and collections as children. #2296

Closed
leebyron opened this issue Oct 6, 2014 · 9 comments
Closed

Accept iterables and collections as children. #2296

leebyron opened this issue Oct 6, 2014 · 9 comments
Assignees

Comments

@leebyron
Copy link
Contributor

leebyron commented Oct 6, 2014

React currently accepts Arrays as children:

var array = [<span />, <span />, <span />];
<div>{spans}</div>

It would be excellent if we can generalize this concept to accept both collections and iterables.

Collections (such as Set, Map, or Immutable-js's variants) can be identified if they implement values() and the result of calling values() is an Iterator (typeof maybeIterator.next === 'function').

Iterables (Array, arguments object, other array-ish things, third-party data structures) can be identified if they have Symbol.iterator or @@iterator returning an Iterator.

In both cases, you're left with an Iterator, at which point extracting the children becomes straight-forward.

This unlocks a few really useful things:

Use Immutable-js with React efficiently:

var data = Immutable.Vector('a', 'b', 'c');
<div>{data.map(str => <span>{str.toUpperCase}</span>)}</div>

Use generators:

var data = function*() {
  yield <span />;
  yield <span />;
  yield <span />;
}
<div>{data}</div>
@leebyron leebyron self-assigned this Oct 6, 2014
@leebyron
Copy link
Contributor Author

leebyron commented Oct 6, 2014

cc @sebmarkbage

@sebmarkbage
Copy link
Collaborator

I'm not sure about the Collections duck typing. Set also have @@iterator so that should be enough.

Maps are special in that we'd probably want to use the key as the React key value. So we could either detect maps specifically, or just allow any iterable with a sequence of two value tuples. The difficult part is, how do you determine if the tuple is a nested array value or Map tuple?

@leebyron
Copy link
Contributor Author

leebyron commented Oct 6, 2014

Maybe Iterable is enough then.

Map iterators return tuples while Set and Array iterators return individual values. Perhaps we can hold this convention and treat tuple[0] as React key.

@leebyron
Copy link
Contributor Author

leebyron commented Oct 6, 2014

The drawback with that is nested iterables/arrays. Explicitly looking for a method spec'd for Collections (entries if we also want to see keys) would differentiate the cases

@syranide
Copy link
Contributor

syranide commented Oct 7, 2014

Perhaps I misunderstood the discussion, but it seems to me that you must explicitly test for Map vs Set, etc. A Set containing tuples should not be interpreted as key/value, as it's not unthinkable that you would want store lists of children in a Set.

PS. IE11 collections does not have iterator, @@iterator nor values, etc.

@leebyron
Copy link
Contributor Author

#2376 implements this.

IE11 collections are a mess. I think the right strategy is to intelligently check for a native ES6 Map/Set implementation and polyfill over them if they are non-compliant. This is the approach we're taking at Facebook.

@syranide
Copy link
Contributor

@leebyron Sounds sane, "we" could even polyfill Array to be a cheap iterable very easily as well. As for IE11, I believe I showed you my earlier work on back-porting Map and Set (and an approximate WeakMap and WeakSet as well) to all legacy browsers which actually perform rather well (both perf and memory). Those could easily be made smart enough to use IE11 Map/Set as the underlying data structure and just provide the additional data required for fast ordered iteration.

@graue
Copy link
Contributor

graue commented Nov 14, 2014

@leebyron Should this be closed now that #2376 is merged?

@leebyron
Copy link
Contributor Author

Yep, thanks for the reminder :)

josephsavona added a commit that referenced this issue May 15, 2024
I realized this while working on Forest. When computing the dependencies of a 
reactive scope we can omit setState functions in the general case (exception 
described below). Currently that's implemented in PruneNonReactiveDependencies. 
However, this causes us to miss some optimizations — a value isn't reactive if 
its only dependency is a setState, and that may allow further downstreams values 
to become non-reactive. We lose out on that by only filtering out setStates in 
PruneNonReactiveDependencies — this logic really belongs in InferReactivePlaces. 

So this PR moves the check for setState types to that pass. The updated fixtures 
show that this already uncovers some wins. The _new_ fixtures covers the 
exception. It's possible for a value to be typed as being a setState function, 
but to still be reactive: if its a local that is conditionally assigned 
different setState function values. Currently this test happens to work because 
our phi type inference is incomplete (see #2296). I'm adding the test now though 
to prevent regressions when we fix phi type inference.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants