Join GitHub today
GitHub is home to over 28 million developers working together to host and review code, manage projects, and build software together.Sign up
RFC: Plan for Attributes in React 16 #10399
Note: the final plan has changed. Refer to https://facebook.github.io/react/blog/2017/09/08/dom-attributes-in-react-16.html for details on what ends up in React 16.
This is meant to address #140.
I wrote this doc but it’s mostly based on discussion with @nhunzaker. I decided to write it in an attempt to formalize the behavior we want, so that if there are bugs, we can refer back to this.
// No warning <div className /> // => <div class /> <img srcSet /> // => <img srcset /> <svg enableBackground /> // => <svg enable-background /> // Warns <div class /> // => <div /> <img srcset /> // => <img /> <svg enable-background /> // => <svg />
There are two downsides to this.
Problem: Custom Attributes
You can’t pass custom, non-standard, library-specific, or not-yet-standardized attributes:
// Warns <input nwdirectory /> // => <input /> <div ng-app /> // => <div /> <div inert /> // => <div />
This is a very popular feature request.
Problem: Maintaining a Whitelist
We currently have to maintain a whitelist of all allowed attributes, and use it even in the production build.
If we change the current behavior, there’s a few existing principles we want to preserve:
I think there is a compromise that lets us solve the problems above without deviating from these principles.
Proposed Behavior: Overview
We drop a large part of the whitelist, but we make the behavior less strict.
<div srcset /> // works but warns <div classname /> // works but warns <svg CalcMode /> // works but warns
Instead of being omitted, they will only emit a warning now.
<div class /> // doesn't work, warns <div accept-charset /> // doesn't work, warns <svg stroke-dasharray /> // doesn't work, warns
This lets us drop 7% of ReactDOM bundle size and keep most of the whitelist for development only.
Proposed Behavior: In Depth
In React 15, it might look like this:
Proposed Changes to the Whitelist
We remove any attributes where
(This is where we get 7% size savings.)
Proposed Changes to the Algorithm
Step 1: Check if there exists a
If it there is a match, use the corresponding
<div className /> // => <div class /> <div acceptCharset /> // => <div accept-charset /> <svg strokeDashArray /> // => <svg stroke-dasharray />
This matches behavior in 15.
Step 2: Check if there exists a
We’re trying to determine if the user was using a DOM version of attribute that is sufficiently different from the one suggested by React (that is, by more than casing).
In this case, don’t render anything, warn and exit.
<div class /> // => <div /> + warning <div accept-charset /> // => <div /> + warning <svg stroke-dasharray /> // => <svg /> + warning
So far this matches behavior in 15.
Note that this does not catch the cases that were sufficiently similar that we excluded them from the whitelist. For example:
<div srcset /> // not in the whitelist, continue the algorithm <div classname /> // not in the whitelist, continue the algorithm <svg CalcMode /> // not in the whitelist, continue the algorithm
This is because we don’t keep them in the whitelist anymore.
Step 3: If the value type is valid, write
This is where we deviate from 15.
<div srcset /> // => <div srcset /> (works) + warning <div classname /> // => <div classname /> (not very useful) + warning <svg CalcMode /> // => <svg CalcMode /> (works) + warning
We only render strings and numbers.
If the value is of a different type, we skip it and warn.
Success now depends on whether DOM accepts such an attribute.
However, we will still warn if there is a
In other words, we warn if there is a canonical React API that differs in casing, such as for all above cases.
This step also captures the new requirement. Any completely unknown attributes will happily pass through:
<input nwdirectory /> // => <input nwdirectory /> <div ng-app /> // => <div ng-app /> <div inert /> // => <div inert />
One thing we haven’t quite worked out yet: should we pass numbers and booleans through.
We have (mostly) settled on this tradeoff:
@sebmarkbage wants to look into a few more corner cases but this is likely the last tweak.
would not render it to the DOM and would display a warning about it not being a string or number.
The concern is that browser APIs are not consistent about what
This doesn’t affect known boolean attributes. We will still keep them in the whitelist. So you can keep passing
Even for objects/arrays, we should only use properties if one exist. But we should warn and not set if none exist.
The problem with detecting properties is that
Are you talking about custom elements or regular elements (focus of this issue)?
Let’s keep this issue focused on regular elements. One doesn’t block the other, and I don’t want bikeshedding over custom elements to black landing this. :-)
I don’t think we should ever let
For custom elements I’d expect we pass them (as objects) if property exists, and don’t pass otherwise. If you want it to be an attribute IMO you should explicitly call
For regular elements we don’t pass them through and always warn. Since you could be accidentally spreading something from a parent component, and
I think it's OK to use the
My understanding is this kind of dilemma comes up a lot with specs because there are all sorts of libraries out there which might use a particular property or function and then become super popular. So spec authors will do an audit of the web and see if their proposed name is already in use. If some other popular library "owns" that name, the spec authors will use a different one.
@robdodson The difference here is that attribute is the common way. So if the component uses attributes there is no property that shadow the base class. We'll pick it up from the base class and start using it instead of the attribute.
This doesn't usually happen with other specs because the common case is that things shadow.
E.g. If you do
To preserve this invariant we must only use properties and never fallback to attributes.
What do you mean by the common way? Not sure I follow.
That's true. In general I encourage folks to always create a corresponding property. I'm writing up some best practices docs that cover this.
I think I see what you're saying. The Custom Element might only work off of a
In the wild I haven't seen that many Custom Elements that only use attributes. FWIW, if a developer uses Polymer to create their element then it'll create corresponding properties for any attributes.
I think that's probably fine. Personally I prefer properties for everything :) If the Custom Element author is really concerned about this there is a pattern they can use to capture properties set on unupgraded instances.
@gaearon Nice write-up! Though it feels like a middle-of-the-road approach where we inherit both the naming from convention of DOM properties AND DOM attributes, with it not being obvious to the user when they're supposed to use one or the other. Like you say (if I understood you correctly), this still means the whitelist will live on in some limited way. I think there's already a lot of confusion regarding e.g.
Everything needs to be considered (incl legacy) and I can't say I know a better way forward than what you've put forth (and I've gone back and forth in the past trying to wrap my head around it
So TBH I'm not entirely sure where I'm going with this, but it would be interesting to know what your high-level long-term goal is. Should ReactDOM just be a "dumb" renderer? Should it be more? How much more? (controlled inputs being implemented in ReactDOM vs userspace means we're not simply a dumb renderer, but should ReactDOM extend the same courtesy to video too?). Should we be able to largely copy-paste SVG? HTML?
Don't get be wrong, all things considered, what you're proposing causes the least friction and is probably the right way forward given the legacy, but it seems like it's replacing a whitelist with special behavior. Which doesn't seem all that much better other than from the perspective of bundle size. So is there a long-term goal towards a more stable/consistent behavior or is the idea to keep the current special-case for legacy reasons? Or do you perhaps even see this as the right long-term approach?
PS. As for
We were already inconsistent anyway, both with custom elements, and with
If we take that into consideration, it seems like the distinction between properties and attributes is not the one that is useful to explain the public API. Whether ReactDOM uses an attribute or a property for a particular “prop” is its implementation detail. Conceptually, the mental model I propose is that:
The last point is important. We’re not calling them
We’re only offering
That’s my mental model. I agree it’s not ideal but it seems like best compromise we could find.
For now, it’s to solve the above two problems (too large whitelist and no custom attributes) with minimal friction. It’s not a high-level goal, but then we don’t really have ones for ReactDOM at the moment. It works well for most cases, and we’re mostly working on the core these days. I’d argue this change brings ReactDOM closer to how the community expects it to work (and how some other libraries have done it).
Will we write applications in five years with ReactDOM? I don’t know. Maybe a higher, more intentionally designed layer like
The only downside is that it requires special-logic in React as opposed to just plain
I feel that what you're saying is a nice feature for beginners, but far less so for experienced people. When you're more experienced you want to know what to expect in detail up-front, currently it's basically trial-and-error.
That was my first though, my only concern is the compatibility between custom renderers (and obviously the overhead of bundling different ones). Even as-is I suspect it wouldn't really work considering how event handling is implemented. And it also hurts the community if it fragments (everyone using their own slightly different version of the hypothetical "ReactDOMLite").
Yeah I've gotten that feeling, and I fully understand it!
Anyway, I don't mean to hold up the PR, it's an interesting discussion. Thanks for your answer!
I think the discussion about whether we move closer to mirror attributes or not is orthogonal (haha, I used that word) to this proposal. It is already the case that we sometimes use the one thing, and in other cases the other. We can keep changing our stance on this in the future (and likely will) but it seems like this shouldn’t block the ability to set arbitrary attributes (for which properties don’t exist in the first place). Similarly, I agree with your point about experienced users, but it doesn’t seem to me that this proposal either makes it worse or better.