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

Proposal: Don't bail out of reconciliation for pure functions #5219

Closed
gaearon opened this issue Oct 20, 2015 · 3 comments
Closed

Proposal: Don't bail out of reconciliation for pure functions #5219

gaearon opened this issue Oct 20, 2015 · 3 comments

Comments

@gaearon
Copy link
Collaborator

gaearon commented Oct 20, 2015

This might be a terrible idea so I'm just throwing it against the wall here.

React has a core reconciliation principle: if the type has changed, don't bother reconciling.
From my understanding, it has two goals:

  1. It's a good heuristic because it's unlikely that two different components yield similar DOM.
  2. One stateful component instance can't “morph” into another component type.

However, it makes pure function components pretty much the same pain to hot reload as class components currently are. We can't just “swap” <ButtonBeforeEdit> with <ButtonAfterEdit> because React will bail out of reconciliation and destroy any state and DOM under it as a result. This undermines the hot reloading experience in a crucial way, so we have to keep resorting to workarounds like insanely complex proxies.

The interesting thing to note about pure function components is that (2) no longer applies to them. They're stateless and have no instances right? So even if React did not bail out of reconciling different types of stateless component, there would be no weird “morphing” of instances.

It would probably be less efficient in development, but we can live with that. On the other hand, making hot reloading easy with “simple tools” is beneficial because maintaining complex solutions is hard, both for me and for people who depend on my infra and have to wait for my upgrades.

Maybe I'm suggesting something crazy. I'm not sure.
It certainly has important consequences outside of hot reloading or performance changes.

For example, this transition:

const Button = ({ children }) => <div className='button'>{children}</div>;
const Toggle = ({ children }) => <div className='toggle'>{children}</div>;

<Button><StatefulThing /></Button>
<Toggle><StatefulThing /></Toggle>

currently resets the StatefulThing internal state, but with the proposed change, it would not, because merely changing Button to Toggle (two stateless components) is now not, per this proposal, enough to bail out from the reconciliation.

What do you think?

@jimfb
Copy link
Contributor

jimfb commented Oct 20, 2015

The interesting thing to note about pure function components is that (2) no longer applies to them.

I think this is inaccurate. Stateless components themselves don't have this.state, but stateless components may have stateful children which is mathematically the same as having a this.state themselves.

@gaearon
Copy link
Collaborator Author

gaearon commented Oct 20, 2015

Stateless components themselves don't have this.state, but stateless components may have stateful children which is mathematically the same as having a this.state themselves.

Yes, but then reconciliation can continue normally there. If the children are incompatible, it bails out. What I'm proposing is exactly the same behavior as if functions weren't supported first class by React and were called directly:

const Button = ({ children }) => <div className='button'>{children}</div>;
const Toggle = ({ children }) => <div className='toggle'>{children}</div>;

Button({ children: <StatefulThing /> })
Toggle({ children: <StatefulThing /> })

// this transition won't destroy StatefulThing's state just because React isn't aware
// that types of Button and Toggle are different. I propose the same behavior
// when using function components normally.

@gaearon
Copy link
Collaborator Author

gaearon commented Mar 8, 2016

Meh, pretty sure this is a dead end and not worth pursuing.

@gaearon gaearon closed this as completed Mar 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants