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

React Fire: Modernizing React DOM #13525

Open
gaearon opened this Issue Aug 31, 2018 · 213 comments

Comments

Projects
None yet
@gaearon
Copy link
Member

gaearon commented Aug 31, 2018

This year, the React team has mostly been focused on fundamental improvements to React.

As this work is getting closer to completion, we're starting to think of what the next major releases of React DOM should look like. There are quite a few known problems, and some of them are hard or impossible to fix without bigger internal changes.

We want to undo past mistakes that caused countless follow-up fixes and created much technical debt. We also want to remove some of the abstraction in the event system which has been virtually untouched since the first days of React, and is a source of much complexity and bundle size.

We're calling this effort "React Fire".

🔥 React Fire

React Fire is an effort to modernize React DOM. Our goal is to make React better aligned with how the DOM works, revisit some controversial past decisions that led to problems, and make React smaller and faster.

We want to ship this set of changes in a future React major release because some of them will unfortunately be breaking. Nevertheless, we think they're worth it. And we have more than 50 thousands components at Facebook to keep us honest about our migration strategy. We can't afford to rewrite product code except a few targeted fixes or automated codemods.

Strategy

There are a few different things that make up our current plan. We might add or remove something but here's the thinking so far:

  • Stop reflecting input values in the value attribute (#11896). This was originally added in React 15.2.0 via #6406. It was very commonly requested because people's conceptual model of the DOM is that the value they see in the DOM inspector should match the value JSX attribute. But that's not how the DOM works. When you type into a field, the browser doesn't update the value attribute. React shouldn't do it either. It turned out that this change, while probably helpful for some code relying on CSS selectors, caused a cascade of bugs — some of them still unfixed to this day. Some of the fallout from this change includes: #7179, #8395, #7328, #7233, #11881, #7253, #9584, #9806, #9714, #11534, #11746, #12925. At this point it's clearly not worth it to keep fighting the browser, and we should revert it. The positive part of this journey is that thanks to tireless work from our DOM contributors (@nhunzaker, @aweary, @jquense, and @philipp-spiess) we now have detailed DOM test fixtures that will help us avoid regressions.

  • Attach events at the React root rather than the document (#2043). Attaching event handlers to the document becomes an issue when embedding React apps into larger systems. The Atom editor was one of the first cases that bumped into this. Any big website also eventually develops very complex edge cases related to stopPropagation interacting with non-React code or across React roots (#8693, #8117, #12518). We will also want to attach events eagerly to every root so that we can do less runtime checks during updates.

  • Migrate from onChange to onInput and don’t polyfill it for uncontrolled components (#9657). See the linked issue for a detailed plan. It has been confusing that React uses a different event name for what's known as input event in the DOM. While we generally avoid making big changes like this without significant benefit, in this case we also want to change the behavior to remove some complexity that's only necessary for edge cases like mutating controlled inputs. So it makes sense to do these two changes together, and use that as an opportunity to make onInput and onChange work exactly how the DOM events do for uncontrolled components.

  • Drastically simplify the event system (#4751). The current event system has barely changed since its initial implementation in 2013. It is reused across React DOM and React Native, so it is unnecessarily abstract. Many of the polyfills it provides are unnecessary for modern browsers, and some of them create more issues than they solve. It also accounts for a significant portion of the React DOM bundle size. We don't have a very specific plan here, but we will probably fork the event system completely, and then see how minimal we can make it if we stick closer to what the DOM gives us. It's plausible that we'll get rid of synthetic events altogether. We should stop bubbling events like media events which don’t bubble in the DOM and don’t have a good reason to bubble. We want to retain some React-specific capabilities like bubbling through portals, but we will attempt to do this via simpler means (e.g. re-dispatching the event). Passive events will likely be a part of this.

  • classNameclass (#4331, see also #13525 (comment) below). This has been proposed countless times. We're already allowing passing class down to the DOM node in React 16. The confusion this is creating is not worth the syntax limitations it's trying to protect against. We wouldn't do this change by itself, but combined with everything else above it makes sense. Note we can’t just allow both without warnings because this makes it very difficult for a component ecosystem to handle. Each component would need to learn to handle both correctly, and there is a risk of them conflicting. Since many components process className (for example by appending to it), it’s too error-prone.

Tradeoffs

  • We can't make some of these changes if we aim to keep exposing the current private React event system APIs for projects like React Native Web. However, React Native Web will need a different strategy regardless because React Fabric will likely move more of the responder system to the native side.

  • We may need to drop compatibility with some older browsers, and/or require more standalone polyfills for them. We still care about supporting IE11 but it's possible that we will not attempt to smooth over some of the existing browser differences — which is the stance taken by many modern UI libraries.

Rollout Plan

At this stage, the project is very exploratory. We don't know for sure if all of the above things will pan out. Because the changes are significant, we will need to dogfood them at Facebook, and try them out in a gradual fashion. This means we'll introduce a feature flag, fork some of the code, and keep it enabled at Facebook for a small group of people. The open source 16.x releases will keep the old behavior, but on master you will be able to run it with the feature flag on.

I plan to work on the project myself for the most part, but I would very much appreciate more discussion and contributions from @nhunzaker, @aweary, @jquense, and @philipp-spiess who have been stellar collaborators and have largely steered React DOM while we were working on Fiber. If there's some area you're particularly interested in, please let me know and we'll work it out.

There are likely things that I missed in this plan. I'm very open to feedback, and I hope this writeup is helpful.

@matteing

This comment has been minimized.

Copy link

matteing commented Aug 31, 2018

I love this. Reducing bundle size and the "class" prop are changes that will be very welcome.

Great work!

@developit

This comment has been minimized.

Copy link
Contributor

developit commented Aug 31, 2018

🙂

@erikras

This comment has been minimized.

Copy link

erikras commented Aug 31, 2018

Attention form library authors! 🤣

@LaurenceM10

This comment has been minimized.

Copy link

LaurenceM10 commented Aug 31, 2018

Great!

@ryanflorence

This comment has been minimized.

Copy link
Contributor

ryanflorence commented Aug 31, 2018

className → class is fantastic

What about all the others? Seems weird to still be doing clipPath, htmlFor, tabIndex, etc.

@diegohaz

This comment has been minimized.

Copy link

diegohaz commented Aug 31, 2018

Adopting class is a major breakthrough in making the library more friendly for beginners. Congratulations.

@tannerlinsley

This comment has been minimized.

Copy link

tannerlinsley commented Aug 31, 2018

This is awesome. I'm so curious how the move to class is actually going work with props.

Seems like ({ class }) => <div class={class} /> would initially present a reserved keyword problem?

@solomonhawk

This comment has been minimized.

Copy link

solomonhawk commented Aug 31, 2018

This is fantastic news, thanks @gaearon!

@felixfbecker

This comment has been minimized.

Copy link

felixfbecker commented Aug 31, 2018

I love every of these points, except the className change. It seems downright contradictory to the goal the other points are pursuing (aligning with the DOM API). React binds to DOM properties, not HTML attributes (this is this even articulated in the first point). The DOM Element property is named className, not class. So why would it be named class in React?

@alexparish

This comment has been minimized.

Copy link

alexparish commented Aug 31, 2018

Fantastic! Do you have a goal for bundle size reduction?

@mknepprath

This comment has been minimized.

Copy link

mknepprath commented Aug 31, 2018

👏

@gaearon

This comment has been minimized.

Copy link
Member Author

gaearon commented Aug 31, 2018

What about all the others? Seems weird to still be doing clipPath, htmlFor, tabIndex, etc.

I’m open to discussion but I’d argue these changes aren’t worth it (except for maybe).

@nhunzaker

This comment has been minimized.

Copy link
Collaborator

nhunzaker commented Aug 31, 2018

I think a re-write of the event system is the most interesting aspect of this. There is significant opportunity to reduce the bundle size and ease community contributions.

Let's do it!

@kentcdodds

This comment has been minimized.

Copy link
Contributor

kentcdodds commented Aug 31, 2018

I wonder if it would be worthwhile to also work on releasing JSX 2.0 at the same time? People are going to need to re-learn a handful of things anyway. Maybe it's better to have to re-learn a bunch at one time rather than a few things twice over a period of time? Just a thought that occurred as I read this.

@gaearon

This comment has been minimized.

Copy link
Member Author

gaearon commented Aug 31, 2018

I love every of these points, except the className change. It seems downright contradictory to the goal the other points are pursuing (aligning with the DOM API). React binds to DOM properties, not HTML attributes (this is this even articulated in the first point).

And yet if we pass an unknown key/value pair it will be treated as an attribute since React 16. So we’re already inconsistent. Also, my comment was about users being wrong in expecting React to set value attribute. Whether React API uses attribute name or property name in its API is compeletely orthogonal.

I’ve defended your side of this argument for years but I think now that this is friction that’s just not worth it. You don’t gain anything from it. Just letting people use class has no negative effects except it doesn’t work with destructuring, and the migration cost. Everybody complains about it when they learn React. I think doing what people expect in this case is more important than being pedantic. And since we’re changing other DOM things anyway, let’s batch this together.

@erikras

This comment has been minimized.

Copy link

erikras commented Aug 31, 2018

As long as React Fire is blazing fast.... 👍

@kentcdodds

This comment has been minimized.

Copy link
Contributor

kentcdodds commented Aug 31, 2018

These changes are all fantastic. I'm way excited about this and its implications for react-testing-library. In particular, events being bound to the react root (or maybe even drop event delegation altogether as it may not be necessary in modern environments anymore?), potentially removing/rewriting synthetic events, and onChange -> onInput will seriously improve the implementation of react-testing-library and the experience in using the tool.

I'd love to provide feedback on this as it's being implemented.

@gaearon

This comment has been minimized.

Copy link
Member Author

gaearon commented Aug 31, 2018

maybe even drop event delegation altogether as it may not be necessary in modern environments anymore

We considered this but think this might be an oversimplification. Event delegation lets us avoid setting up a bunch of listeners for every node on the initial render. And swapping them on updates. Those aspects shouldn’t be ignored. There is likely more benchmarking to be done there though.

@drcmda

This comment has been minimized.

Copy link

drcmda commented Aug 31, 2018

@tannerlinsley ({ class: className }) => <div class={className} /> unfortunately this will kill jsx 2.0 object short hand notation (<div class />), but so be it ...

It would be super, super nice if class could finally take objects and arrays btw next to strings.

@gaearon

This comment has been minimized.

Copy link
Member Author

gaearon commented Aug 31, 2018

Do you have a goal for bundle size reduction?

Dropping a third of React DOM would be nice. We’ll see. It’s hard to say early but we’ll do our best.

@AlexGalays

This comment has been minimized.

Copy link

AlexGalays commented Aug 31, 2018

Wow, this is an enumeration of almost all the design decisions I mention when people ask me about React cons.

Love the direction this is going.

@jacobp100

This comment has been minimized.

Copy link

jacobp100 commented Aug 31, 2018

What would the upgrade path be for libraries that use className and want to support multiple versions of React?

@ryanflorence

This comment has been minimized.

Copy link
Contributor

ryanflorence commented Aug 31, 2018

@gaearon Maybe it's not a big deal, today its nice to say "props are DOM properties not HTML attributes", now it'll be "except className, that one is the HTML name". I'd also like to be able to copy/paste SVG w/o 10 minutes of messing around with attributes to match up with React's

@fforw

This comment has been minimized.

Copy link
Contributor

fforw commented Aug 31, 2018

What about htmlFor?

@phpnode

This comment has been minimized.

Copy link

phpnode commented Aug 31, 2018

It feels like the className -> class transition will have to be done very carefully, probably over an extended period of time. It's going to cause a lot of issues for the ecosystem, as virtually every component will need to be changed - even very trivial ones. This is mostly fine if you "own" the code and there's a codemod, but when you're dependent on 3rd party libraries you're largely at the mercy of maintainers.

The rest of the changes seem relatively low risk from an ecosystem point of view, but getting rid of className will really cause a lot of pain. It seems like it should be possible to split that into a separate issue and release it on a different schedule to the rest of the 🔥 changes.

@Pajn

This comment has been minimized.

Copy link

Pajn commented Aug 31, 2018

I agree with @felixfbecker
Everything sounds awesome and would improve quality for both developers and users, but the className change.

Beeing able to deconstruct all properties but one would certainly cause more confusion and be harder to explain to new users than that they need to use className because class is a keyword (which they can clearly see when the misstake is made, as it's colored differently). Working around class in deconstructing requires introducing new syntax or a completely different way to read out that specific property that would only work untill you need to use a rest property.
It creates many problems just to save four characters.

@caub

This comment has been minimized.

Copy link

caub commented Aug 31, 2018

@felixfbecker it could it be class/for in JSX and className/htmlFor in the JS version?

<label class="foo" for="bar">..</label>

would be

React.createElement('label', {className: 'foo', htmlFor: 'bar'}, '..')
@alexeybondarenko

This comment has been minimized.

Copy link

alexeybondarenko commented Aug 31, 2018

Great plan! Nice to here that! 👏👏👏

@jamsch

This comment has been minimized.

Copy link

jamsch commented Nov 14, 2018

@cullophid

Would it be possible for react fire to not rebind event handlers?

You can bind the method to the class once using arrow function auto bindings or binding the function in your constructor. If there's unique parameters you want to chain on to the event, you can look in to memoizing the event handlers or attaching DOM attributes to the nodes themselves and access them through event.currentTarget.

class Compo extends Compoent {
 onClick = () => {
   ...
 }
 render() {
   <button onClick={this.onClick}>click me</button>
 }
}
@WeShared

This comment has been minimized.

Copy link

WeShared commented Nov 16, 2018

😀

@AlexGalays

This comment has been minimized.

Copy link

AlexGalays commented Nov 17, 2018

Another benefit of simplifying the event system (can we go back to raw DOM events without any pooling like ALL frameworks do? :) ) is that it's easier to statically type as well.

@MatthewHerbst

This comment has been minimized.

Copy link

MatthewHerbst commented Nov 30, 2018

Seems like a great plan! It would be really nice to have IE 11 support (even if that means shipping our own polyfills which is totally fine). As someone who works with government clients, when they will stop using IE 11 is a complete unknown :(

@yordis

This comment has been minimized.

Copy link

yordis commented Nov 30, 2018

@MatthewHerbst same boat, our clients do use IE 11 and we have enough traffic of it 😢

@dominic-p

This comment has been minimized.

Copy link

dominic-p commented Dec 1, 2018

Just wanted to chime in to say this is all super exciting. The one thing I read that worried me was:

it's possible that we will not attempt to smooth over some of the existing browser differences

Right now I love knowing that React's event system will "Just Work" on any browser that React supports. I'm afraid that this change will mean that it will be become the app's responsibility to account for cross browser issues. This would likely introduce a lot of difficult to trace bugs. ...at least it would for my apps. :)

Anyway, thanks as always for the great library.

@JacobMGEvans

This comment has been minimized.

Copy link

JacobMGEvans commented Dec 27, 2018

@kentcdodds had mentioned JSX 2.0 integration as a possible idea I was reading through the Strategy and did not see anything it seemed related to that. I was wondering if that was just up in the air or something that will be held off for the future?

@hbroer

This comment has been minimized.

Copy link

hbroer commented Jan 17, 2019

Let the IE die! Don't support absolutely outdated browsers. There is no reason, even for restrictive companies, not to switch to an up to date browser. You will see that IE11 in your stats until you and the web stops supporting it.

@j-f1

This comment has been minimized.

Copy link

j-f1 commented Jan 17, 2019

@hbroer Legacy software?

@ljharb

This comment has been minimized.

Copy link
Contributor

ljharb commented Jan 17, 2019

@hbroer nobody stays on an older browser because their sites still work, they do it because they either have no choice, or don’t know how to update. Breaking a site for these users is exceedingly hostile and leaves them not with an updated browser, but with nothing.

@akotlar akotlar referenced this issue Jan 17, 2019

Closed

WIP: Greenfield web app #5162

2 of 10 tasks complete
@hbroer

This comment has been minimized.

Copy link

hbroer commented Jan 22, 2019

  • "don’t know how to update" - not our problem, but we can put in messages "install a new browser" with a list of browsers and maybe with help text and phone number of the Microsoft Hotline ^^
  • "have no choice" - Right, but the organisation has to update their environment. If their tools don't work, they will.

i hate to code for the past and can't use the benefit of new tech, because 90% of coders still support the crappy browsers of the bottom 15% consumers living in last century. ^^ Don't wanna have this "oh what is with this internet explorer 6 nerds, we have to support that 15%" again for the next 10 years.

btw M$ seams to bring Edge for Windows 7 and 8 with their switch to webkit.

@ljharb

This comment has been minimized.

Copy link
Contributor

ljharb commented Jan 22, 2019

@hbroer everything is our problem if it impacts users. Have some empathy for other humans.

(Separately, M$ is a pretty juvenile and very dated late-90s reference that you may wish to remove; I’m happy to remove this aside if you do)

@hbroer

This comment has been minimized.

Copy link

hbroer commented Jan 23, 2019

I see, here are many M$ fanboiz. :D I really care on humans, but not on organisations which block technology progression, or the coders who mean to have to support that old crap over years. I have no problem with an additional package which makes a library "old crap compatible". But a library in 2019 should coded with >=2019 tech in mind, and not <=2013 tech. It is bad enough to have to support that (oh let us do this a little bit another way) safari and (old) edge crap.

micdrop

@ljharb

This comment has been minimized.

Copy link
Contributor

ljharb commented Jan 23, 2019

@hbroer in fact ancient android browser, and ancient safari, are more of a problem than any microsoft browser. it's not about being a "fanboi" but being professional, and using the term "M$" is not.

And no, it's not "bad" in any way to support old tech, it's the moral and ethical thing to do. What's bad is websites that "work best in X", whether that's Netscape Navigator, IE 6, or latest Chrome.

@dmitriid

This comment has been minimized.

Copy link

dmitriid commented Jan 23, 2019

I see, here are many M$ fanboiz.

@hbroer I see here a person with blanket ad hominem attacks.

But a library in 2019 should coded with >=2019 tech in mind, and not <=2013 tech.

No, it shouldn't.

If you took one seconds and looked into browserlist defaults, you'd be surprised. Instead, you resort to ad hominem attacks and school cafeteria-level insults.

Once you get enough users, IE11 becomes a sizeable chunk of people who access your website. At a previous job we had close to a million people a month accessing our website from IE11.

@hbroer

This comment has been minimized.

Copy link

hbroer commented Jan 23, 2019

browsers

If u want support old crap add poly fills. No reason not to code for the future and the present. Supporting old tech makes web apps more fat and slower than needed.

P.s. i see a person who cant read sarcasm and don't care about smilies ^^

@dmitriid

This comment has been minimized.

Copy link

dmitriid commented Jan 23, 2019

@hbroer You're showing that IE11 is the present, so we are expected to be coding for that.

And, once again. See the default value at https://browserl.ist.

P.s. i see a person who cant read sarcasm and don't care about smilies ^^

Yeah, no. This is a common tactic employed by trolls and playground bullies. "What?! I didn't insult you! It was just a joke!".

@ljharb

This comment has been minimized.

Copy link
Contributor

ljharb commented Jan 23, 2019

0.20% of people on the planet, even filtered by "those who use the internet", is 6.4 million human beings. Percentages are utterly irrelevant. Code for humans; the past and the future, and your bundle size, doesn't matter.

@rovansteen

This comment has been minimized.

Copy link

rovansteen commented Jan 23, 2019

If you want to code for humans bundle size, just like browser compatibility, certainly matters. Users not only expects things to work, it should be fast to load as well.

@hbroer

This comment has been minimized.

Copy link

hbroer commented Jan 23, 2019

Now in 2019 i will use for new projects css grid (which has only experimental support on IE11), and will transpile to ES6 and i don't want deliver polyfills for that outdated Browser. IE11 Users simply get a message: Update your browser or use an alternative.

There are people out there who don't want use java script. You care about them? You don't see that guys in the stats. And i am not in statistics too, like many other people who block that tools.

I take a eye on every browser which is not outdated. I support edge, which has less users than that IE11 crap, because of Windows 7 (which supports ends january 2020), and modern people use modern browsers. ^^

Nobody stops you from using polyfills and something like a compatibility package. But the core should be up to date and not stay way behind only because of one piece of an old M$ tech browser.

What i am missing in many javascript frameworks is LTS. Thats what we can talk about. If you build a modern page with features from the present, then its nice to use a up to date tech framework. And if u are building a webapp for b2b stuff which needs the maximum of stability and compatibility then u can use a LTS version. Or use knockout. Then u can support the few 100t people who are still using an not updated windows XP with IE6 ^^

@manigandham

This comment has been minimized.

Copy link

manigandham commented Jan 23, 2019

Compatibility is clearly listed under "trade-offs" in the original post so having perfect support for old browsers is already a secondary focus.

They will require more polyfills and integration if you need to support them but that would be your choice to do so. You can also create multiple bundles for different browser targets and deliver the smallest possible JS for each visitor so it won't affect your whole audience.

The progress on React Fire itself seems unaffected so there's really nothing to be gained from this debate. Let's move on please and get back to the main topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment