Support Map interface for props

[swannodette]

High performance Map implementations are starting to appear in modern web browsers. By supporting the Map interface users can now instead supply props as an Immutable.js or ClojureScript Map instance. This change would not even require providing an equality hook for the first pass as users can memoize props themselves.

For users embracing immutable data this is a huge change, it means that styles can be defined in code, overridden and shared efficiently. React DOM elements with memoized immutable props can be skipped over, etc.

[levand]

I second this - it would be a huge boon to performance when using React from ClojureScript.

[bhauman]

Yes supporting Map will allow many possibilities, cursors, etc. This sounds good.

[ghost]

This is actually quite a good idea. I would love to see this in React, with or without Clojurescript. Offers considerably improved styles of working in the React environment.

[brigand]

I suppose the main thing blocking this is key/ref being passed like props, and a useable syntax/api to allow this. Adding it to JSX would be pretty straight forward.

<!--  Valid -->
<div {map} />
<div {map} key={...} />
<div {map} key={...} ref={...} />
<div key={...} ref={...} {map} />
<div key={...} {map} ref={...} />

<!-- Invalid -->
<div {map} foo={...} />
<div {map} {...foo} />

But createElement is variadic, which is a problem... some potential solutions (where meta is an object with key and ref):

// breaking change
React.createElement(tag, map, childrenAlwaysAsAnArray, metaOrUndefined)
React.createElement(tag, {props: map, key: ..., ref: ...}, childrenAlwaysAsAnArray)

// add another function, this makes it explicit that react should treat the
// second argument as an opaque data structure and metadata is separate
React.createElement2(tag, map, metaOrNull, ...children)

// generic, react agnostic, array
// see #2932 – unsure if it's feasible for performance 
[tag, map, childrenAlwaysAsAnArray, metaOrUndefined]

_{Related: Consider Using JsonML as Notation for ReactElement #2932}

[skrat]

Oh, this isn't there already? +1

[mybuddymichael]

Yesssss.

[dmhood]

👍

[syranide]

Has this actually been verified? Using objects for props is exactly what objects are intended for it seems to me, the same set of known keys used over and over. Maps should have way higher mem/perf overhead than objects for props I would assume.

PS. Also how would I access a specific prop in a universal way? Not possible AFAIK.

[dmhood]

@syranide
In our case we are using immutable.js to turn component state into an immutable map/record, and would like to be able to do the same with props in order to have the same api with each object.

As far as perf goes, having a way to make props immutable (along with state) would allow us to more effectively perform equality checks in shouldComponentUpdate, and I doubt the slightly higher mem/perf cost of immutable maps would ever be much of an issue.

As it stands now we can check for state equality immediately but still have to do a form of shallow equals comparison with the props object, which is slower and also not as robust.

[swannodette]

@syranide there are many very well known old tricks to supporting the hash map interface while incurring very small overheads - a common approach is to encode small maps (which are very typical in many programs including React) as arrays.

[syranide]

As far as perf goes, having a way to make props immutable (along with state) would allow us to more effectively perform equality checks in shouldComponentUpdate

@dmhood Objects can be immutable too and immutability in itself is not enough to get away with a simple equality test. I don't know how you would even go about universally reusing non-mutated "props" between renders.

there are many very well known old tricks to supporting the hash map interface while incurring very small overheads - a common approach is to encode small maps (which are very typical in many programs including React) as arrays.

@swannodette Certainly, but that's just making them faster, it doesn't make them "as fast". An object (with a hidden class) should be be cheaper to instantiate, use less allocated memory and be faster to access. An object seems like the obviously correct DS for props (and state too) if you ask me, maps haven't replaced objects, they replaced objects-as-maps.

Anyway, I assume CJS sees it differently and it might make sense for you, but AFAIK it doesn't make any sense for "regular" React and would even come at a big "cost".

[skrat]

@syranide Can you explain what is the "big cost"?

[syranide]

@skrat this.props.foo vs this.props.get('foo') (you can't have both), then there's also memory and perf which may be more or less measurable.

[swannodette]

@syranide allocation and access costs for supporting ES6 Map interface is almost entirely in user-land. A huge bottleneck this removes is the Object iteration cost that React eats everywhere during reconciliation. Which is going to be faster, iterating over an Object with hasOwnProperty checks or iterating over a array? I've never seen the former outperform the later. Couple this with the memoization story this provides I suspect Immutable.js Map props based programs will kick regular React ones in the ass.

[syranide]

@swannodette

A huge bottleneck this removes is the Object iteration cost that React eats everywhere during reconciliation.

Object iteration is not slow AFAIK, but hasOwnProperty is at current relatively very costly (but not in IE11), but if that's really a problem then as soon as we drop IE8 it's should be safe to proceed under the assumption that no environment worth supporting breaks the object prototype I would say. There's also #3227. Or you could also provide an abstraction that returns an object and a list of keys thus you avoid the hasOwnProperty check without having to resort to other overheads related to maps.

http://jsperf.com/objvsarrxyz (and this doesn't include the overhead of actually dynamically creating and accessing the underlying array structure)

APIs shouldn't be designed from the standpoint of current performance characteristics if you ask me.

Couple this with the memoization story this provides I suspect Immutable.js Map props based programs will kick regular React ones in the ass.

How would you (realistically) perform universal fast memoization for "props"? Perhaps I'm missing something but I can't see how you would accomplish that.

Also, memoization should work equally well for objects as it does for maps, it doesn't favor one or the other.

[dmhood]

@syranide

@skrat this.props.foo vs this.props.get('foo') (you can't have both), then there's also memory and perf which may be more or less measurable.

Why can't we have both? It seems like we could change the createElement function to accept a constructor or a collection (like an immutable map) and then it could be smart about building the right type of props object. // cc @brigand had some suggestions

I think for the majority of cases the perf difference is going to be negligible. What this really would help is those of us that are turning to Immutable.js or ClojureScript and currently have to program around the props object to fit whatever paradigm we choose.

@dmhood Objects can be immutable too and immutability in itself is not enough to get away with a simple equality test...

It's not that just making them immutable will give us equality test, but adding the map interface will allow us to more easily use libraries (again immutable.js/clojurescript/etc.) that bring along the features we need for it (https://github.com/facebook/react/blob/master/docs/docs/11-advanced-performance.md#immutable-js-to-the-rescue).

I don't know how you would even go about universally reusing non-mutated "props" between renders.

Not sure what you mean by this? Can you elaborate?

[brigand]

I don't know how you would even go about universally reusing non-mutated "props" between renders.

@syranide, the only readable way to do it requires compile time transforms for non-changing props.

R.cc({
  render(){ 
    return R.ce(Comp, {foo: 'bar'});
   }
});

// to

var __props_1 = {foo: 'bar'};
R.cc({
  render(){ 
    return R.ce(Comp, __props_1);
   }
});

You can already do this, but due to key/ref being on the props object, react allocates a new object and copies properties over anyway (I think... can anyone confirm this?).

it could be smart about building the right type of props object

@dmhood I didn't mean you could mix normal props and a collection object. Automatically converting from e.g. Map to Object is both infeasible for performance and impossible when keys aren't String/Symbol. You would also lose the wrapper/cursor when passing it down, which is not good at all.

The main requirement for this feature would be that react treats the props as opaque (it doesn't attempt to inspect/modify it, it just passes it around to where it needs to go).

---

I see people talk about equality comparison with immutable data structures, and just to be clear, you can't determine if the contents is not equal using ===.. it just won't give false positives (but false negatives are common). To actually use === to determine inequality of object contents in JS would make the constructor need a list of all other instances (memory leak), and to compare the contents of the to-be-created object with all other objects of the type, i.e. it's not feasible.

In the context of shouldComponentUpdate, this doesn't cause it to return false incorrectly, but we need to be mindful of reducing the false negatives.

---

For shouldComponentUpdate, if you use propTypes we could also create tooling for generating a shouldComponentUpdate based on propTypes and getInitialState that does plain comparisons rather than a loop on runtime object properties. I'm sure someone will do this eventually (I'll post a link here if I find time).

---

If you have only one prop which points to an immutable object, and you have state with just one key pointing to an immutable object, you basically have an immutable state and props, and the issue here is just about saving the props/state object allocation and a little typing (props.data.get('foo') vs props.get('foo')).

[syranide]

Why can't we have both? It seems like we could change the createElement function to accept a constructor or a collection (like an immutable map) and then it could be smart about building the right type of props object. // cc @brigand had some suggestions

@dmhood #3228

It's not that just making them immutable will give us equality test

@dmhood It won't for "props" (at least not more than object already does).

but adding the map interface will allow us to more easily use libraries (again immutable.js/clojurescript/etc.) that bring along the features we need for it (https://github.com/facebook/react/blob/master/docs/docs/11-advanced-performance.md#immutable-js-to-the-rescue).

@dmhood But my point is that I don't understand why you want that, "props" should be considered a class of sorts (a fixed structure), not a dynamic map of keys. Just like you wouldn't expect an "options"-object to be a Map. I don't see what benefit you expect to get from using a library to construct "props". In 99% of the cases only the values are dynamic, not the keys so the library just adds overhead.

the only readable way to do it requires compile time transforms for non-changing props.

@brigand Yes certainly, but that only works for truly static "props" and it works equally well for objects as it does for immutable Maps.

[skrat]

I think @brigand nailed it, props should be opaque to React. It's user data. The fact that React uses it for key/ref is unfortunate because it mixes it's internal requirements with user rendering input. I can see the convenience case here, but still, this should be resolved.

[dmhood]

@syranide
Really we would be using a record data structure to ensure that keys never change--I just figured a map interface would be more generic/used by others. Although if props were made opaque this is rather moot.

[syranide]

@dmhood Object.freeze(...) works for that (and will be "forced" I believe).

[cigitia]

This is an important conversation. As far as I can see, though, there are actually a couple of issues here:

• Is it useful to use map data structures (e.g., Immutable.js maps) directly as custom components' props?
• Is it useful to use map data structures (e.g., Immutable.js maps) as regular DOM elements' props?
• If either of the above is true, then where should things like key, ref, and child/children live? Should the props map contain them, or should they live somewhere else? How would they be accessed?
• Is it useful to use map data structures (e.g., Immutable.js maps) as regular DOM elements' style props [mentioned in original post]?

Personally, I think the answers to questions 1, 2, and 4 are yes (though question 4 matters much more to me than any other). The thing is that props aren't always simply options with constant keys; they commonly contain nested model data. In addition, those model data often might have variable keys.

In particular, CSS styles are especially useful to consider as manipulable data—I often use large, nested ClojureScript data structures to express elements' CSS styles, later dynamically merging them together and adding and removing keys from them at runtime.

This is possible with persistent immutable data structures but is not possible with deeply frozen regular objects without full copying, as far as I know. As a stopgap, it's possible to fully convert those data structures to regular JavaScript objects with every render, in order to be readable by React—that's what I do right now, in fact—but, of course, that's very wasteful. In any case, these aren't the sort of data that would benefit from being regular objects with backing classes with constant keys.

As for question 3 (i.e., would key, ref, child/children, etc. remain in a props object if it were a map), I don't know what I'd want; it doesn't affect question 4. Question 3 might be the biggest obstacle to implementing questions 1 and 2, and it might be the reason why this.props.foo vs this.props.get('foo') would be a performance problem. React-internal code relies on reading elements’ keys, refs, and children. It would have to determine how to access that information: props.key or props.get("key"), and it also needs to add the opaque children object. Then again, it might be practical to just use, e.g., props.get ? props.get("key") : props.key without too much of a penalty.

Other than that, I don't really yet understand how this.props.foo vs this.props.get('foo') is that huge of a performance problem, since that sort of thing is otherwise already up to the user of React, and it'd already be a problem for custom components if they're using any custom model data (i.e., the user is already doing this.props.data.get("foo").get("blah") if they're using anything like Immutable.js). React prides itself anyway on how it's not prescriptive about the user's models, after all.

A bigger and much more fundamental problem would be that it might slow down or complicate reconciliation, especially if Map support is added to DOM elements. Hopefully that wouldn't be the case, but I don't yet know enough about the details of reconciliation's implementation to really know.

I do know, though, that allowing the user to choose between JavaScript objects and whatever map data structures they want to—whether it's for a DOM element's style, all of a DOM element's props, or a custom component's props—probably would bring at least some benefits to the way many people use React. (I happen to care most about the first case myself; I'm considering creating an issue focusing only on it.)

[syranide]

@cigitia My unofficial answer to 1 and 2 is no. I just can't see the benefit of using a library for constructing the props object, if you have an example where it makes sense please share it. But from my perspective you're just making it more complicated, adding overhead and breaking compatibility between components for no benefit.

PS. key and ref are already separate from props.

[cigitia]

@syranide

My unofficial answer to 1 and 2 is no.

Right, thanks for the answer. But what about question 4, using generic data for styles? It's extremely useful to be able to express styles as data, to let the end user manipulate in the application's UI and to serialize them (e.g., for end-user UI themes), and to dynamically merge and manipulate them. (React Native already supports the last sort of thing with merging StyleSheets, but that's not yet supported in regular React, and it'd be even better for serializability and manipulability if generic style data could be used.)

Should that be considered as separate from this issue?

PS. key and ref are already separate from props.

Ah, whoops, right—I had been thinking of how keys and refs are passed as arguments to createElement/etc.; sorry about that.

[syranide]

@cigitia As for 4, style is inside props and is technically unrelated to this, this discussion is about the universal restriction on props having to be an object. The interpretation of style (and other properties) is the responsibility of the component (in this case ReactDOMComponent) and if there's no technical reason (say performance) then I'd imagine it will be supported through generic iterators in the future.

[cigitia]

@syranide Right, thanks. I'll make a separate issue for that, then.

[skrat]

@cigitia 1. Yes, 2. Yes 4. Yes. As for point No. 3, I would prefer the options that are used by React.js internally to live in their own argument, and not be mixed with opaque user data.

@syranide You seems to the only one in this thread not seeing the benefits of using anything else than POJSOs for props objects. No offense but there are many that see it, the reasons were explained here.

I don't see what benefit you expect to get from using a library to construct "props". In 99% of the cases only the values are dynamic, not the keys so the library just adds overhead.

You assume that the users are constructing the data for React.js, ie. in your view, there's little besides the view layer that is handled by React.js. That's a false assumption, in 99% of the cases, React.js is just a view on some data structures already existing in the app. So having to convert them to POJSOs adds extra overhead. I really don't see how can someone argue that user data shouldn't be opaque to some library.

[syranide]

@skrat

@syranide You seems to the only one in this thread not seeing the benefits of using anything else than POJSOs for props objects. No offense but there are many that see it, the reasons were explained here.

Could be because it's an issue about people wanting that feature no? :)

Anyway, my perspective is purely logical/technical, if someone can show me an actual example where it makes sense to do it then that's fine, but I can't think of one and so far there's just talk (I get that it might be natural for OM, but that's besides the point IMHO).

There's a clear distinction between what objects and maps are good for, objects are a perfect fit for props; should only have a fixed set of known keys, values can have any type, they're small in size, they're fast to create and they're fast to access. They can also be immutable. So why do you need immutable-js or w/e for props?

[mouthy75ks]

To be honest, i really have no idea what I'm doing. I want to be able to download programs and put them to use in different websites. But I'm not real computer savvy. I'm just trying really hard to understand all this stuff.

[cigitia]

For what it's worth, #3303 and #2059 are also both related to this issue. In particular, in #3303 sebmarkbage appears to float the idea of promoting Immutable.js to receive built-in support for being directly used as states, which might be considered to be analogous to this discussion's idea for props.

[sophiebits]

I think we can support this when createElement is bypassed, which it is (or can easily be) in environments like Om. defaultProps and propTypes won't work but I think everything else can…

[cigitia]

#3542 (“is it useful to use immutable map data structures (e.g., Immutable.js maps) as regular DOM elements' style props”) was closed, being considered similar enough to this issue here that either would be done probably only if both are.

#2196, from last September, is also related to this issue (most specifically to the matter of regular DOM elements’ style props). @sebmarkbage remarked apropos to immutable persistent data structures:

[A desired] feature is the ability to combine multiple of these constants in a light-weight way without actually copying. As has already been pointed out, it's even more efficient to just do this at the module level scope. The only exception is for dynamic values such as: { ...styles.base, width: this.props.width }

These might need a persistent object. The goal should be to get these into engines and to do that we need to start designing with that in mind.

We can polyfill with an immutable record object from immutable-js or something like that. A general data structure that can later be replaced with a native record object or we can compile to support it…

I don't think we should introduce a new proprietary data type that will be difficult to upgrade a more general solution for persistent records. Not until there's an imminent need. There won't be an imminent need until the other aspects of this style model has proven itself…

If we can introduce something that is more difficult to undo (such an ImmutableObject abstraction that is palatable and upgradable) then we can move forward with that instead. Or if we can prove the imminent value of this (that can't wait for other data types).

[RinatMullayanov]

It would be great.

[zackify]

Looking forward to this!

[settinghead]

+1

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution.

[stale]

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!