The Reconciler #711

theKashey · 2017-12-06T13:40:00Z

This implements idea from #703
Fixes: #650 (HoC), #666 (HoC), and any other HoC or decorator issues.

Idea - instead of relying on registering the variables - relay on react-tree.

Algorithm is simple

use something like react-deep-force-update to hydrate the render tree
render the tree back, swapping the old components, and the new ones on the fly.

Results
The good one - ANY component could be Hot-Reloaded. You can use complex HOC or decorators and still have RHL working🏆
The bad one - all components should be wrapped by proxy. This actually does not affect performance at all, as long stand-in operates on pure js prototypical inheritance.

To activate reconciler - add reconciler prop to AppContainer.

PS: Not to be merged yet.

theKashey · 2017-12-06T13:42:10Z

packages/react-hot-loader/src/patch.dev.js

-let firstInstances
-let knownSignatures
-let didUpdateProxy
+import {


patch.dev was completely rewritten. Things like error reporting about failed registrations or failed replacement were removed, as long they are obsolete with reconciler.

gregberge · 2017-12-06T14:15:34Z

Very good work! The idea is great! I think you have resolved the major issue of this project 😄.

gregberge · 2017-12-06T14:17:33Z

packages/react-hot-loader/src/reconciler/ComponentMap.js

+export default class ComponentMap {
+  constructor(useWeakMap) {
+    if (useWeakMap) {
+      this.wm = new WeakMap()


We could rely on WeakMap since we are targetting only modern browsers.

gregberge · 2017-12-06T14:20:49Z

packages/react-hot-loader/src/AppContainer.dev.js

@@ -2,16 +2,20 @@ import React, { Component } from 'react'
 import PropTypes from 'prop-types'
 import deepForceUpdate from 'react-deep-force-update'
 import { getGeneration } from './updateCounter'
+import hydrate from './reconciler/traverse'


hydrate / traverse

gregberge · 2017-12-06T14:22:36Z

packages/react-hot-loader/src/reconciler/deepForceTraverse.js

+  }
+}
+
+const deepForceTraverse = (instance, stack) => {


The goal of this function is to traverse a tree (stack) and swap all components is it right? I think the naming is not good.

gregberge · 2017-12-06T14:23:47Z

packages/react-hot-loader/src/reconciler/traverse.js

+  }
+}
+
+export default function hydrate(instance) {


The goal is to traverse React tree and build an AST of it in order to traverse it and swap all swappable components, yes?

theKashey · 2017-12-06T14:24:20Z

Ideally this should be moved into react-mount, as a user defined class comparison function. And, as result we could even remove proxy, as long there will be no need to fake types.
A single line change in react internals can make things must simpler.
How to sell this idea to the React team?

gregberge · 2017-12-06T14:28:12Z

@gaearon can help us!

gregberge · 2017-12-06T14:29:57Z

@theKashey we can't remove proxy because we have to keep state and variables. So even if we have a custom comparison function proxies are still needed.

gaearon · 2017-12-06T16:31:09Z

Please avoid relying on React internal APIs if you can. We don't want to introduce more dependencies than already exist in deep-force-update.

I intend to work on officially supporting hot reloading in React in the beginning of next year. So it's best to coordinate our efforts.

theKashey · 2017-12-06T21:58:14Z

@gaearon - does this mean, that we should not proceed with this PR, as long it uses some React internals, or we could proceed, as long it uses the same internals as deep-force-update?

gaearon · 2017-12-06T22:00:54Z

If it uses the same exact internals it’s okay.

theKashey · 2017-12-07T08:13:15Z

All react internals were move into a single (and quite small) file. Also refactored logic a bit (just perform dead code elimination).

theKashey · 2017-12-14T11:22:54Z

@neoziro - just added a couple of tests. Coverage is not great, but enough for a start.
It is much more important to test this solution against some real edge cases, I am not quite aware of.

Waive if you are keen to merge this to the lerna branch.

gregberge · 2017-12-14T16:39:46Z

@theKashey Thanks! I could take a look on tuesday, not before.

gaearon · 2017-12-14T17:42:03Z

I want to note that I still don't really understand what this is doing, so I'd appreciate a more in-depth explanation of why this works, why it is better than the existing approach, and what are the potential downsides and things that could break.

That said I don't want to block you on my understanding :-) If you're sure this is solid and give it a lot of testing in large codebases (I'm sure plenty of people would be happy to volunteer trying an alpha) then let's roll with it.

theKashey · 2017-12-14T22:01:45Z

Ok, let me explain.

The biggest constraint of V3 is a literal unusability for lots of people. All these folks who use decorators, recompose, and quite happy about it - they are all off the boat.

V1 injects update code on the each-file-basic, thus allows it to perform point updates, and thus allows it to work with a complex code. But actually, it does not respect any complete code dependencies - module self-accept is a dangerous thing.

V3 performs the work much more gently, rendering from top to bottom, relying on registered classes to perform updates. This is the power, this is a weakness. It is just impossible to register everything, but you have to.

Reconciler is just another way to detect changes.
The idea is simply - you have a rendered React Three, and you are updating a single class. And you want only a single component to be updated (as V1 do).

Solution:

(hydrate)Traverse the existing tree, to store component type positions. Like - I know that Button was inside Form, inside Page12. This part actually borrows code from react-deep-force-update.
(hotReplacementRender)Traverse the new tree, to spot the changes to update proxies. I this part I have to explain in deep.

Reconsiler:

To make reconciler works RHL have to wrap all components with proxy. In real - not all, but all stateless, and all classes with injected "REGENERATE_METHOD". As long standin-proxy did not affect performance - we can do it.
Call render method on real component instance, obtained from hydrate.
Traverse the result, comparing component types.
3.1 In case of DOM node perform monkey patching (mergeInject) combining type from rendered child, and children from hydrated one.
3.2 In case of function compare the new and the old one
3.2.1 If types are equal - continue render
3.2.1 If types are equal from proxy point of view (registed) - continue render
3.2.1 If types are swappable - update proxy and continue render.

And swappable is the key of this solution.
In v3 swappable means that types have been exported from a same file and same name.
Here - it does mean - types were found in the same tree position, have the same display name and also some similarity in code.

similarity - full-text similarity, or 20% by Levenshtein distance (fuzzyCompare, up to O(n^2) ). For classes text similarity are tested on method level, allowing you add or remove one method, and change another. (has no effect on the registered types).

And there is a small trick here - reconsiler does not respect a possible change in props. It does not render the true react tree. It tries to re-render application as it was, to spot and swap changes.
If you are:

rendering a different component – it will don't enter that leaf
rendering component in a different order - it will swap types, and render them in the old order
rendering with different props, resulting a different nested tree - it will update as many proxies as it could, and the nested tree will be completely updated in any case.

So, this is not a real render, this is just another way to rehydrate hot updated code.
It is still important to have a registered types, to perform major updates upon them, but now RHL can stand any decorator or function composition between these registered types.
Hooray?

gregberge

What is the behaviour if I remove an hoc:

// from
compose(one, two, three)(Component)
// to
compose(one, three)(Component)

gregberge · 2017-12-19T09:27:23Z

packages/react-hot-loader/src/AppContainer.dev.js

@@ -99,6 +112,7 @@ AppContainer.propTypes = {
  },
  errorReporter: PropTypes.oneOfType([PropTypes.node, PropTypes.func]),
  warnings: PropTypes.bool,
+  reconciler: PropTypes.bool,


I think we should always activate reconciler. Why making it optional?

gregberge · 2017-12-19T09:27:49Z

packages/react-hot-loader/src/AppContainer.dev.js

@@ -99,6 +112,7 @@ AppContainer.propTypes = {
  },
  errorReporter: PropTypes.oneOfType([PropTypes.node, PropTypes.func]),
  warnings: PropTypes.bool,


So we can drop this option isn't it?

theKashey · 2017-12-19T10:24:03Z

RE:

// from
compose(one, two, three)(Component)
// to
compose(one, three)(Component)

Reconciler will do it's best, and update all the proxies. But react-dom will not stand a tree change.
WE could invent a way, to rehydrate the old instances into a new components, but I'll prefer not to hack React (more deeply).

theKashey added 6 commits December 4, 2017 23:00

traverse

860244f

Merge branch 'lerna' into reconcile

0b58428

add reconciler

1599990

Merge remote-tracking branch 'original/lerna' into reconcile

197f9c9

store ID inside proxy

80fb057

add reconciler

d5cc94f

theKashey requested a review from gregberge December 6, 2017 13:40

theKashey commented Dec 6, 2017

View reviewed changes

gregberge reviewed Dec 6, 2017

View reviewed changes

theKashey added 3 commits December 7, 2017 18:49

refactor reconsiler

06d8944

move fuzzy compare under prop

a69e937

linting

3ebb2fe

theKashey mentioned this pull request Dec 12, 2017

HotLoader won't work after adding >=2 levels of HoC. #650

Closed

theKashey added 2 commits December 14, 2017 18:14

reconsiler tests

28fd841

linting

7877556

theKashey added this to the v4 milestone Dec 18, 2017

gregberge approved these changes Dec 19, 2017

View reviewed changes

theKashey merged commit 150f1d2 into gaearon:lerna Dec 21, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

The Reconciler #711

The Reconciler #711

theKashey commented Dec 6, 2017

theKashey Dec 6, 2017

gregberge commented Dec 6, 2017

gregberge Dec 6, 2017

gregberge Dec 6, 2017

gregberge Dec 6, 2017

gregberge Dec 6, 2017

theKashey commented Dec 6, 2017

gregberge commented Dec 6, 2017

gregberge commented Dec 6, 2017

gaearon commented Dec 6, 2017

theKashey commented Dec 6, 2017

gaearon commented Dec 6, 2017

theKashey commented Dec 7, 2017

theKashey commented Dec 14, 2017

gregberge commented Dec 14, 2017

gaearon commented Dec 14, 2017

theKashey commented Dec 14, 2017

gregberge left a comment

gregberge Dec 19, 2017

gregberge Dec 19, 2017

theKashey commented Dec 19, 2017

The Reconciler #711

The Reconciler #711

Conversation

theKashey commented Dec 6, 2017

theKashey Dec 6, 2017

Choose a reason for hiding this comment

gregberge commented Dec 6, 2017

gregberge Dec 6, 2017

Choose a reason for hiding this comment

gregberge Dec 6, 2017

Choose a reason for hiding this comment

gregberge Dec 6, 2017

Choose a reason for hiding this comment

gregberge Dec 6, 2017

Choose a reason for hiding this comment

theKashey commented Dec 6, 2017

gregberge commented Dec 6, 2017

gregberge commented Dec 6, 2017

gaearon commented Dec 6, 2017

theKashey commented Dec 6, 2017

gaearon commented Dec 6, 2017

theKashey commented Dec 7, 2017

theKashey commented Dec 14, 2017

gregberge commented Dec 14, 2017

gaearon commented Dec 14, 2017

theKashey commented Dec 14, 2017

gregberge left a comment

Choose a reason for hiding this comment

gregberge Dec 19, 2017

Choose a reason for hiding this comment

gregberge Dec 19, 2017

Choose a reason for hiding this comment

theKashey commented Dec 19, 2017