Repository: infernojs/inferno
Pull request #1593 (2022-01-20 18:43:16)

Functional Components for inferno-mobx
**Objective**

This PR adds support for functional components in a manner that is interchangeable with the base component for the purpose of building Inferno's VDOM.

Please do take a look at the `observerWrap` function in the file of the same name in the inferno-mobx source to see if there might be any subtle problems that I should make sure are tested for.
I have been finding the tests that were originally in place for inferno-mobx don't check all the things they should.
I will be putting all the relevant new tests in place for the other parts of the library once this PR is done.

Once all looks good let me know and I will squash the commits in my fork before the merge.


coveralls: 
[![Coverage Status](https://coveralls.io/builds/45802252/badge)](https://coveralls.io/builds/45802252)

Coverage increased (+0.04%) to 92.136% when pulling **34b13a833a518512f5d3a03b4f6300f535a847f6 on Gwenio:master** into **d0ed1ca70ef4f81491c037f5a001d86833657a74 on infernojs:master**.

Havunen: This should probably have some benchmark as this is mostly about performance?
Havunen: Is there anything missing from this PR? 
Gwenio: @Havunen 

> This should probably have some benchmark as this is mostly about performance?

It is not quite *just* about performance. Functional components are slightly different in terms of how they can be used than class components: they can have arbitrary hooks per rendered component.

It is also a step towards having ForwardRef components which are even more different: the `ref` parameter can be transparently passed on to a child VNode.

It is possible to have better performance, but then the implementation would place restrictions on how the observer component can be used.

A bench mark would be needed to evaluate if more specialized versions give enough value to be worth including. I plan to look into that once all the 'generalized' versions are working.

> Is there anything missing from this PR?

Looking back over it again, I did spot one issue that will need to be addressed.

Capturing `props` when binding `onComponentDidUpdate` and `onComponentWillUpdate`; however, capturing the call backs can be a issue if:

- the component is re-rendered with different call backs (or a different `ref` object if they actually use it internally as it is the `this` parameter when the callbacks are called).
- and onComponentShouldUpdate tells Infereno to not rerender

Which would lead to the Reaction continuing to use the old callbacks and `ref` object.

At the moment I do not think there is a good solution other than saying the Reaction will not call them when it re-renders the component.



Havunen: > Looking back over it again, I did spot one issue that will need to be addressed.
 Capturing props when binding onComponentDidUpdate and onComponentWillUpdate; however, capturing the call backs can be a issue if:

Could you add tests for this use case and possibly fix the issue? I think once this PR is "ready" we could merge it and publish new alpha version of inferno where these changes are included.
Gwenio: @Havunen 

> Could you add tests for this use case and possibly fix the issue?

I added tests for and documented how it is currently working.

There is no way to make it work the 'ideal' way with the way callbacks are handled on functional components.

For functional components it is only a problem if you are adding and removing components of the same type and same parent but each is being assigned non-interchangeable callbacks (for the will / did update ones).
Which I would expect to be a niche use case.
If this is needed and the performance is too bad without a `onComponentShouldUpdate` callback, then they need to use a class component instead.

It would be a bigger issue for ForwardRef components. With them, it means the Ref is not updated; therefore, the wrong one is forwarded.
For those, I guess the `onComponentShouldUpdate` callback will just need to be forbidden (and have an error in debug builds when one is detected).

Which by the way, I think `observer` handles ForwardRefs completely wrong currently. It seems to treat them as though it received the prototype of a class rather than the class itself.

Once this PR and one for ForwardRefs are done, I will revisit the `observer` function.
Gwenio: Hmm, looks like I will need to rebase as the type definitions exported by Inferno have change.
Havunen: Okay, thanks for the test cases. Are you using inferno mobx yourself? We could have these changes published as alpha version and then you could test that everything works as expected? I will go through all changes in v8 and try to add support for static vNodes before releasing the stable version. 
Gwenio: @Havunen 

Sure, though the personal project I was working on has kind of fallen to the wayside for the time being.

I did make a version the DBMonster benchmark from the docs folder that uses inferno-mobx.
Havunen: Hey, can you include that benchmark too :)

Patch:
diff --git a/packages/inferno-mobx/README.md b/packages/inferno-mobx/README.md
index 7ad43cbf9..df9f93003 100644
--- a/packages/inferno-mobx/README.md
+++ b/packages/inferno-mobx/README.md
@@ -6,9 +6,14 @@ The module is compatible with Inferno v1+, for older versions use [mobx-inferno]
 
 This package provides the bindings for MobX and Inferno.
 Exports `observer` and `inject` decorators, a `Provider` and some development utilities.
+
 *New*: exports `observerPatch`, a function to turn Component classes into MobX observers.
 `observerPatch` is implemented in a better manner, but is separate as it would break compatibility in some cases.
 
+*New*: exports `observerWrap`, a function to turn functional Components into MobX observers.
+Unlike `observer`, `observerWrap` does not wrap them in a class Component.
+This allows the base Component and the observer to be interchangable.
+
 ## Install
 
 ```
@@ -21,7 +26,7 @@ Also install [mobx](https://github.com/mobxjs/mobx) dependency _(required)_ if y
 npm install --save mobx
 ```
 
-## Example
+## observerPatch Examples
 
 Pass a class Component to `observerPatch` to have in automatically re-render if MobX observables read by `render` are modified.
 
@@ -183,6 +188,161 @@ Then it is restored to the class's original `render` method when `componentWillU
 
 It is strongly recommended to avoid replacing the `render` method on classes `observerPatch` is applied to.
 
+## observerWrap Examples
+
+`observerWrap` is the companion to `observerPatch`, but for functional Components.
+
+```tsx
+// MyComponent.tsx (also works with plain JavaScript)
+import { observerWrap } from 'inferno-mobx';
+
+interface CountStore {
+    readonly count: number
+}
+
+// For a good debugging experience, have named functions
+function MyComponentA({ countStore }: { countStore: CountStore }) {
+    return (<p>Current Count: {countStore.count.toString()}</p>);
+}
+
+// If you prefer arrow functions, assign them to a const before passing to observerWrap
+const MyComponentB = ({ countStore }: { countStore: CountStore }) => {
+    return (<p>Current Count: {countStore.count.toString()}</p>);
+};
+
+// This works, but if you need to debug the MobX reactions things will be less nice
+const MyComponentC = observerWrap(({ countStore }: { countStore: CountStore }) => {
+    return (<p>Current Count: {countStore.count.toString()}</p>);
+});
+
+const MyObserverB = observerWrap(MyComponentB);
+
+// double wrapping does not cause errors, but is inefficient
+// a warning will be logged for non-production builds
+const MyComponentD = observerWrap(MyComponentC);
+
+const MyComponentE = observerWrap(MyComponentA);
+
+// calling observerWrap on the same component multiple times is fine
+const MyComponentF = observerWrap(MyComponentA);
+
+// but storing the result of the first call for reuse is still better
+// as each wrapper will take memory
+const MyComponentG = MyComponentE;
+
+export {
+    // wrapping separately allow the non-observer component to be used
+    MyComponentA, // not an observer
+    MyObserverB as MyComponentB, // export observer under name of the component
+    MyComponentC,
+    MyComponentD,
+    MyComponentE, // observer version of MyComponentA
+    MyComponentF,
+    MyComponentG
+}
+```
+
+Everything mentioned for `observerPatch` about passing you observables to the components applies to `observerWrap`.
+
+As a note, the function returned by `observerWrap` should *only* be used for constructing Inferno VNodes.
+That means JSX, `createElement`, inferno-hyperscript, or `createComponentVNode`.
+For any other usage, you should use the base function directly.
+
+```tsx
+// index.tsx
+import {
+    MyComponentA,
+    MyComponentB,
+    MyComponentC,
+    MyComponentD,
+    MyComponentE,
+    MyComponentF,
+    MyComponentG
+} from './MyComponent';
+import { render } from 'inferno';
+import { action, observable } from 'mobx';
+
+const store = observable({ count: 0 });
+
+// All but MyComponentA will update when count changes
+// MyComponentA was not wrapped
+render(<div>
+  <MyComponentA countStore={store} />
+  <MyComponentB countStore={store} />
+  <MyComponentC countStore={store} />
+  <MyComponentD countStore={store} />
+  <MyComponentE countStore={store} />
+  <MyComponentF countStore={store} />
+  <MyComponentG countStore={store} />
+  <button type="button" onClick={action(() => {store.count += 1})}>Click Me</button>
+</div>, document.getElementById('components'));
+```
+
+There are a few things to say about `defaultHooks` and `defaultProps`.
+
+```typescript
+import { observerWrap } from 'inferno-mobx';
+
+// For this first component, both the base function and the wrapper will share default hooks and props
+function MyComponentA(props) { /* ... */ }
+MyComponentA.defaultHooks = { /* ... */ }
+MyComponentA.defaultProps = { /* ... */ }
+const MyObserverA = observerWrap(MyComponentA);
+// MyComponentA.defaultHooks === MyObserverA.defaultHooks &&
+// MyComponentA.defaultProps === MyObserverA.defaultProps
+
+// So the following would affect the defaultHooks for both as they refer to the same object:
+MyComponentA.defaultHooks.onComponentShouldUpdate = /* hook */;
+
+// But settting a new object only affects the one being set:
+MyObserverA.defaultProps = { /* ... */ } // Now MyComponentA.defaultProps !== MyObserverA.defaultProps
+
+// If the defaults are set after the wrapper is made, the observer will not have default Hooks nor Props
+function MyComponentB(props) { /* ... */ }
+const MyObserverB = observerWrap(MyComponentB);
+MyComponentB.defaultHooks = { /* ... */ }
+MyComponentB.defaultProps = { /* ... */ }
+// MyObserverB.defaultHooks === undefined && MyObserverB.defaultProps === undefined
+
+// Setting defaults on the observer does not affect the base component
+function MyComponentC(props) { /* ... */ }
+const MyObserverC = observerWrap(MyComponentC);
+MyObserverC.defaultHooks = { /* ... */ }
+MyObserverC.defaultProps = { /* ... */ }
+// MyComponentC.defaultHooks === undefined && MyComponentC.defaultProps === undefined
+```
+
+Generally the first example is what you want. The third is fine if you only use the observer.
+
+The most like case you would want the defaults to be different is for the `onComponentShouldUpdate` hook.
+This is because the re-rendering of the observer is more expensive than the base component.
+That does not mean you always want `onComponentShouldUpdate` for observers and never to base components.
+But it can be worth having on observers even if it is not for the base.
+
+Note:
+The MobX Reaction will call `onComponentWillUpdate` and `onComponentDidUpdate` when re-rendering, if present.
+When called by the Reaction the 'previous' and 'next' parameters will be passed the same object.
+
+*Warning*:
+The `onComponentWillUpdate` and `onComponentDidUpdate` are bound when the component is rendered by Inferno.
+The `onComponentShouldUpdate` can prevent the binding from being updated if new callbacks are assigned.
+
+To have a default hook (or property) on just the observer, you need to create a new object that is a copy of the base version.
+And then add the new hook or property.
+
+```typescript
+function MyComponent(props) { /* ... */ }
+MyComponent.defaultHooks = { /* ... */ }
+MyComponent.defaultProps = { /* ... */ }
+
+const MyObserver = observerWrap(MyComponent);
+
+MyObserver.defaultHooks = {
+    ...MyComponent.defaultHooks, // copy original
+    onComponentShouldUpdate: (prev, next) => { /* ... */ }
+}
+```
+
 ## Legacy Example
 
 You can inject props using the following syntax ( This example requires, babel decorators-legacy plugin )
@@ -264,7 +424,7 @@ The differences to be aware of when switching from `observer` to `observerPatch`
 7. `observerPatch` will not emit events through `renderReporter` that list how long `render` took
 8. `observerPatch` does not make `this.props` nor `this.state` observable
 9. `observerPatch` does not set `isMobXReactObserver = true` as a static class member
-10. `observerPatch` is only for class Components, functional Components are not supported
+10. `observerWrap` returns a functional Component, where `observer` returns a class Component.
 
 Points 1 and 2 are a simple change to call `observerPatch` after the class is defined and removing `observer`.
 
@@ -295,10 +455,7 @@ For point 9, `observerPatch` instead sets `isMobXInfernoObserver = true` as a st
 But it only does do in development builds as it is intended for use internally for sanity checks.
 The issues it is used to spot and warn about should be fixed before reaching production.
 
-Finally for point 10, the way `observer` worked with functional was to wrap them in a class Component.
-It likely was not what you wanted.
-It ignored any default hooks and being wrapped in class Component you could not add new ones.
-And if your functional Component was only ever used as an observer, then directly making it a class Component would be better.
+Finally for point 10, if you specify the Component VNode type to optimize renders, you will need to change the flags.
 
 If this seems intimidating, know that nearly all unit tests for `observer` worked as they were after replacing `observer` with `observerPatch`.
 Outside of needing to switch functional Components to class Components, that is.
@@ -311,7 +468,7 @@ However, using `inject` as a decorator along side `observerPatch` is not support
 ```tsx
 // MyInjected.tsx
 import { Component } from 'inferno';
-import { observerPatch, inject } from 'inferno-mobx';
+import { observerPatch, observerWrap, inject } from 'inferno-mobx';
 
 interface CountStore {
     readonly count: number
@@ -354,6 +511,13 @@ class MyComponentC extends Component<{ countStore?: CountStore }> {
 export const MyInjectedC = inject('countStore')(MyComponentC);
 observerPatch(MyInjectedC); // WRONG! Should be: observerPatch(MyComponentC);
 //Having observerPatch before inject lets tools detect this issue.
+
+// Functional components with observerWrap
+function MyComponentD({ countStore }: { countStore?: CountStore }) {
+    const count = (countStore as CountStore).count.toString();
+    return (<p>Current Count: {count}</p>);
+}
+export const MyInjectedD = inject(observerWrap(MyComponentD));
 ```
 
 ```tsx
@@ -361,7 +525,8 @@ observerPatch(MyInjectedC); // WRONG! Should be: observerPatch(MyComponentC);
 import {
     MyInjectedA,
     MyInjectedB,
-    MyInjectedC
+    MyInjectedC,
+    MyInjectedD
 } from './MyInjected';
 import { render } from 'inferno';
 import { Provider } from 'inferno-mobx';
@@ -377,6 +542,7 @@ render(<div>
         <MyInjectedA />
         <MyInjectedB />
         <MyInjectedC /> {/* This one will not update as MyComponentC was not made into an observer. */}
+        <MyInjectedD />
         <MyInjectedA countStore={store2} /> {/* Will not update as direct properties override injection */}
     </Provider>
   <button type="button" onClick={action(() => {store.count += 1})}>Click Me</button>
@@ -401,7 +567,7 @@ The best way to achieve this is to have calls to `observer` and `observerPatch`
 ```tsx
 // MyComponent.tsx
 import { Component } from 'inferno';
-import { observerPatch } from 'inferno-mobx';
+import { observerPatch, observerWrap } from 'inferno-mobx';
 
 interface CountStore {
     readonly count: number
@@ -413,6 +579,10 @@ export class MyComponent extends Component<{ countStore: CountStore }> {
     }
 }
 
+function MyFunctional({ countStore }: { countStore: CountStore }) {
+    return (<p>Current Count: {countStore.count.toString()}</p>);
+}
+
 // You do not have to use an environment variable nor is the specific one used important.
 // The idea would be to replace 'process.env.SERVER_SIDE' with a literal true or false.
 // This can be done when transpiling with Babel or many bundling tools have plugins that can do so.
@@ -420,6 +590,10 @@ export class MyComponent extends Component<{ countStore: CountStore }> {
 if (process.env.SERVER_SIDE !== true) {
     observerPatch(MyComponent); // The same goes can be done for 'observer' calls.
 }
+
+// For functional components
+const MyObserver = process.env.SERVER_SIDE !== true ? observerWrap(MyFunctional) : MyFunctional;
+export {MyObserver as MyFunctional};
 ```
 
 Then you can use `renderToString` and `hydrate` as you would with any other component.
diff --git a/packages/inferno-mobx/src/index.ts b/packages/inferno-mobx/src/index.ts
index ccb4f1198..cbe7644c7 100644
--- a/packages/inferno-mobx/src/index.ts
+++ b/packages/inferno-mobx/src/index.ts
@@ -2,6 +2,7 @@ import { errorsReporter, inject, Observer, observer, renderReporter, trackCompon
 import { Provider } from './Provider';
 import { EventEmitter } from './utils/EventEmitter';
 import { observerPatch } from './observerPatch';
+import { observerWrap } from './observerWrap';
 
 // THIS IS PORT OF AWESOME MOBX-REACT to INFERNO
 // LAST POINT OF PORT
@@ -9,4 +10,4 @@ import { observerPatch } from './observerPatch';
 
 const onError = (fn) => errorsReporter.on(fn);
 
-export { errorsReporter, inject, observer, onError, EventEmitter, Observer, observerPatch, Provider, renderReporter, trackComponents, useStaticRendering };
+export { errorsReporter, inject, observer, onError, EventEmitter, Observer, observerPatch, observerWrap, Provider, renderReporter, trackComponents, useStaticRendering };
diff --git a/packages/inferno-mobx/src/observerWrap.ts b/packages/inferno-mobx/src/observerWrap.ts
new file mode 100644
index 000000000..c3fd7dfb7
--- /dev/null
+++ b/packages/inferno-mobx/src/observerWrap.ts
@@ -0,0 +1,127 @@
+import { Inferno, render, VNode, createComponentVNode, _HI as normalizeRoot } from 'inferno';
+import { Reaction } from 'mobx';
+import { throwError, warning } from 'inferno-shared';
+import { VNodeFlags } from 'inferno-vnode-flags';
+
+type InfernoNode = Inferno.InfernoNode;
+
+type Render = (properties?: any, context?: Record<string, unknown>) => InfernoNode | undefined | void;
+
+function callDispose({ dispose }: { readonly dispose: () => void }): void {
+  dispose();
+}
+
+interface InnerProperties {
+  readonly context: unknown
+  readonly dispose: () => void
+  readonly props: unknown
+  readonly self: VNode
+  readonly track: (f: () => void) => void
+}
+
+function innerVNode<T>(type: (p: InnerProperties) => T, properties: InnerProperties): VNode {
+  const ref = {
+    onComponentDidUpdate: callDispose,
+    onComponentWillUnmount: properties.dispose
+  };
+  if (process.env.NODE_ENV !== 'production') {
+    Object.freeze(properties);
+    Object.freeze(ref);
+  }
+  return createComponentVNode(VNodeFlags.ComponentFunction, type, properties, undefined, ref);
+}
+
+function makeProxy(target: VNode): { $V: InfernoNode } {
+  return {
+    get $V() {
+      return target.children
+    },
+    set $V(value) {
+      target.children = value
+    }
+  };
+}
+
+type UpdateHook = (this: RefType, prev: unknown, next: unknown) => void;
+
+interface RefType {
+  readonly onComponentDidUpdate?: UpdateHook
+  readonly onComponentWillUpdate?: UpdateHook
+}
+
+function getUpdateHooks(ref: RefType | null, props: unknown): (null | (() => void))[] {
+  let onComponentDidUpdate = null;
+  let onComponentWillUpdate = null;
+  if (ref) {
+    if (ref.onComponentDidUpdate) {
+      onComponentDidUpdate = ref.onComponentDidUpdate.bind(ref, props, props);
+    }
+    if (ref.onComponentWillUpdate) {
+      onComponentWillUpdate = ref.onComponentWillUpdate.bind(ref, props, props);
+    }
+  }
+  return [onComponentDidUpdate, onComponentWillUpdate];
+}
+
+export function observerWrap<T extends Render>(base: T): typeof base {
+  if (process.env.NODE_ENV !== 'production') {
+    if (typeof base !== 'function') {
+      throwError(`observerWrap requires a function to wrap, got ${typeof base} instead`);
+    }
+    // @ts-ignore
+    if (base.prototype && base.prototype.render) {
+      throwError("observerWrap should not be applied to constructors.");
+    }
+    // @ts-ignore
+    if (base.isMobXInfernoObserver) {
+      warning("'observerWrap' was used on a component that already has 'observerWrap' applied. Please only apply once");
+    }
+  }
+  function tracked({ context, props, self, track }: InnerProperties): ReturnType<typeof base> {
+    let result;
+    let caught;
+    track(() => {
+      try {
+        result = base.call(self, props, context);
+      } catch(error) {
+        caught = error;
+      }
+    });
+    if (caught) {
+      throw caught;
+    }
+    return result;
+  }
+  function wrapper(this: VNode, props, context): VNode {
+    const [onComponentDidUpdate, onComponentWillUpdate] = getUpdateHooks(this.ref, props);
+    let proxy;
+    const reaction = new Reaction(base.name, () => {
+      let next;
+      if (onComponentWillUpdate) {
+        onComponentWillUpdate();
+      }
+      reaction.track(() => {
+        next = normalizeRoot(base.call(this, props, context));
+      });
+      if (next) {
+        // indirectly call patch as inferno does not export patch
+        render(next, proxy as any, onComponentDidUpdate, context);
+      }
+    });
+    const inner = innerVNode(tracked, {
+      context,
+      dispose: reaction.dispose.bind(reaction),
+      props,
+      self: this,
+      track: reaction.track.bind(reaction)
+    });
+    proxy = makeProxy(inner);
+    return inner;
+  }
+  wrapper.defaultProps = (base as any).defaultProps;
+  wrapper.defaultHooks = (base as any).defaultHooks;
+  if (process.env.NODE_ENV !== 'production') {
+    wrapper.isMobXInfernoObserver = true;
+  }
+  return wrapper as any;
+}
diff --git a/packages/inferno/src/index.ts b/packages/inferno/src/index.ts
index 907600c96..57f181ae7 100644
--- a/packages/inferno/src/index.ts
+++ b/packages/inferno/src/index.ts
@@ -71,7 +71,7 @@ export {
   version,
   // Internal methods, used by hydration
   createClassComponentInstance as _CI,
-  normalizeRoot as _HI,
+  normalizeRoot as _HI, // used by inferno-mobx
   mount as _M,
   mountClassComponentCallbacks as _MCCC,
   mountElement as _ME,


Test patch:
diff --git a/packages/inferno-mobx/__tests__/observerWrap.spec.jsx b/packages/inferno-mobx/__tests__/observerWrap.spec.jsx
new file mode 100644
index 000000000..5ed80f792
--- /dev/null
+++ b/packages/inferno-mobx/__tests__/observerWrap.spec.jsx
@@ -0,0 +1,536 @@
+import { Component, render } from 'inferno';
+import { inject, observer, observerWrap } from 'inferno-mobx';
+import { createElement } from 'inferno-create-element';
+import { observable, runInAction } from 'mobx';
+
+const getDNode = (obj, prop) => obj.$mobx.values[prop];
+
+const stateLessComp = ({ testProp }) => <div>result: {testProp}</div>;
+
+stateLessComp.defaultProps = {
+  testProp: 'default value for prop testProp'
+};
+
+describe('Stateless components observerWrap', () => {
+  let container;
+
+  beforeEach(function () {
+    container = document.createElement('div');
+    document.body.appendChild(container);
+  });
+
+  afterEach(function () {
+    render(null, container);
+    container.innerHTML = '';
+    document.body.removeChild(container);
+  });
+
+  it('stateless component', (done) => {
+    const StatelessCompObserver = observerWrap(stateLessComp);
+    expect(StatelessCompObserver.defaultProps.testProp).toBe('default value for prop testProp');
+    const wrapper = <StatelessCompObserver testProp={10} />;
+
+    render(<StatelessCompObserver testProp="hello world" />, container);
+
+    expect(container.textContent).toBe('result: hello world');
+    done();
+  });
+
+  it('stateless component with context support', () => {
+    const InnerComp = (_props, context) => createElement('p', {}, 'inner: ' + context.testContext);
+    const StateLessCompWithContext = ({ store: { value } }, context) => {
+      return createElement('div', {}, [
+        createElement('p', {}, 'value: ' + value + ', '),
+        createElement('p', {}, 'outer: ' + context.testContext + ', '),
+        createElement(InnerComp, {})
+      ]);
+    }
+    const StateLessCompWithContextObserver = observerWrap(StateLessCompWithContext);
+    const store = observable({
+      value: 0
+    });
+    class ContextProvider extends Component {
+      getChildContext() {
+        return {testContext: 'hello'};
+      }
+      render() {
+        return <StateLessCompWithContextObserver store={store} />;
+      }
+    }
+    render(<ContextProvider />, container);
+    expect(container.textContent.replace(/\n/, '')).toBe('value: 0, outer: hello, inner: hello');
+    store.value = 1;
+    expect(container.textContent.replace(/\n/, '')).toBe('value: 1, outer: hello, inner: hello');
+  });
+
+  it('nestedRendering', () => {
+    const store = observable({
+      todos: [
+        {
+          title: 'a',
+          completed: false
+        }
+      ]
+    });
+
+    let todoItemRenderings = 0;
+    let todoItemUnmounts = 0;
+    let todoItemUpdates = 0;
+    let todoItemWillUpdates = 0;
+    const TodoItemBase = ({ todo }) => {
+      todoItemRenderings++;
+      return <li>|{todo.title}</li>;
+    };
+    TodoItemBase.defaultHooks = {
+      onComponentDidUpdate: () => {
+        todoItemUpdates++;
+      },
+      onComponentShouldUpdate: ({ todo: { title: prev } }, { todo: { title: next } }) => {
+        return prev !== next;
+      },
+      onComponentWillUnmount: () => {
+        todoItemUnmounts++;
+      },
+      onComponentWillUpdate: () => {
+        todoItemWillUpdates++;
+      }
+    };
+    const TodoItem = observerWrap(TodoItemBase);
+
+    let todoListRenderings = 0;
+    const TodoList = observerWrap(() => {
+        todoListRenderings++;
+        const todos = store.todos;
+        return (
+          <div>
+            <p>{todos.length}</p>
+            {todos.map((todo, idx) => (
+              <TodoItem key={idx} todo={todo} />
+            ))}
+          </div>
+        );
+      });
+
+    render(<TodoList />, container);
+    expect(todoListRenderings).toEqual(1); //, 'should have rendered list once');
+    expect(container.querySelectorAll('li').length).toEqual(1);
+    expect(container.querySelector('li').textContent).toEqual('|a');
+
+    expect(todoItemRenderings).toEqual(1); // 'item1 should render once'
+
+    expect(getDNode(store, 'todos').observers.length).toBe(1);
+    expect(getDNode(store.todos[0], 'title').observers.length).toBe(1);
+
+    store.todos[0].title += 'a';
+
+    expect(todoListRenderings).toEqual(1); //, 'should have rendered list once');
+    expect(todoItemRenderings).toEqual(2); //, 'item1 should have rendered twice');
+    expect(container.querySelector('li').textContent).toEqual('|aa');
+    expect(getDNode(store, 'todos').observers.length).toBe(1); //, 'observers count shouldn\'t change');
+    expect(getDNode(store.todos[0], 'title').observers.length).toBe(1); //, 'title observers should not have increased');
+
+    store.todos.push({
+      title: 'b',
+      completed: true
+    });
+
+    expect(container.querySelectorAll('li').length).toBe(2); //, 'list should two items in in the list');
+    let expectedOutput = [];
+    let nodes = container.querySelectorAll('li');
+
+    for (let i = 0; i < nodes.length; i++) {
+      expectedOutput.push(nodes[i].textContent);
+    }
+    expect(expectedOutput).toEqual(['|aa', '|b']);
+
+    expect(todoListRenderings).toBe(2); //'should have rendered list twice');
+    expect(todoItemRenderings).toBe(3); //, 'item2 should have rendered as well');
+    expect(getDNode(store, 'todos').observers.length).toBe(1); //, 'observers count shouldn\'t change');
+    expect(getDNode(store.todos[0], 'title').observers.length).toBe(1); //, 'title observers should not have increased');
+    expect(getDNode(store.todos[1], 'title').observers.length).toBe(1); //, 'title observers should have increased');
+    expect(getDNode(store.todos[1], 'completed').observers.length).toBe(0); //, 'completed observers should not have increased');
+
+    store.todos[1].title += 'b';
+
+    expect(container.querySelectorAll('li').length).toBe(2); //, 'list should two items in in the list');
+    expectedOutput = [];
+    nodes = container.querySelectorAll('li');
+
+    for (let i = 0; i < nodes.length; i++) {
+      expectedOutput.push(nodes[i].textContent);
+    }
+    expect(expectedOutput).toEqual(['|aa', '|bb']);
+
+    expect(todoListRenderings).toBe(2); //'should have rendered list twice');
+    expect(todoItemRenderings).toBe(4); //, 'item2 should have rendered as well');
+    expect(getDNode(store, 'todos').observers.length).toBe(1); //, 'observers count shouldn\'t change');
+    expect(getDNode(store.todos[0], 'title').observers.length).toBe(1); //, 'title observers should not have increased');
+    expect(getDNode(store.todos[1], 'title').observers.length).toBe(1); //, 'title observers should have increased');
+    expect(getDNode(store.todos[1], 'completed').observers.length).toBe(0); //, 'completed observers should not have increased');
+
+    const oldTodo = store.todos.pop();
+
+    expect(todoListRenderings).toBe(3); //, 'should have rendered list another time');
+    expect(todoItemRenderings).toBe(4); //, 'item1 should not have rerendered');
+    expect(container.querySelectorAll('li').length).toBe(1); //, 'list should have only on item in list now');
+    expect(getDNode(oldTodo, 'title').observers.length).toBe(0); //, 'title observers should have decreased');
+    expect(getDNode(oldTodo, 'completed').observers.length).toBe(0); //, 'completed observers should not have decreased');
+    render(null, container);
+    expect(todoItemUnmounts).toBe(2);
+    expect(todoItemUpdates).toBe(2);
+    expect(todoItemWillUpdates).toBe(2);
+    expect(getDNode(store, 'todos').observers.length).toBe(0);
+    expect(getDNode(store.todos[0], 'title').observers.length).toBe(0);
+  });
+
+  it('nestedRendering without should update hook', () => {
+    const store = observable({
+      todos: [
+        {
+          title: 'a',
+          completed: false
+        }
+      ]
+    });
+
+    let todoItemRenderings = 0;
+    let todoItemUnmounts = 0;
+    let todoItemUpdates = 0;
+    let todoItemWillUpdates = 0;
+    const TodoItemBase = ({ todo }) => {
+      todoItemRenderings++;
+      return <li>|{todo.title}</li>;
+    };
+    TodoItemBase.defaultHooks = {
+      onComponentDidUpdate: () => {
+        todoItemUpdates++;
+      },
+      onComponentWillUnmount: () => {
+        todoItemUnmounts++;
+      },
+      onComponentWillUpdate: () => {
+        todoItemWillUpdates++;
+      }
+    };
+    const TodoItem = observerWrap(TodoItemBase);
+
+    let todoListRenderings = 0;
+    const TodoList = observerWrap(() => {
+        todoListRenderings++;
+        const todos = store.todos;
+        return (
+          <div>
+            <p>{todos.length}</p>
+            {todos.map((todo, idx) => (
+              <TodoItem key={idx} todo={todo} />
+            ))}
+          </div>
+        );
+      });
+
+    render(<TodoList />, container);
+    expect(todoListRenderings).toEqual(1); //, 'should have rendered list once');
+    expect(container.querySelectorAll('li').length).toEqual(1);
+    expect(container.querySelector('li').textContent).toEqual('|a');
+
+    expect(todoItemRenderings).toEqual(1); // 'item1 should render once'
+
+    expect(getDNode(store, 'todos').observers.length).toBe(1);
+    expect(getDNode(store.todos[0], 'title').observers.length).toBe(1);
+
+    store.todos[0].title += 'a';
+
+    expect(todoListRenderings).toEqual(1); //, 'should have rendered list once');
+    expect(todoItemRenderings).toEqual(2); //, 'item1 should have rendered twice');
+    expect(getDNode(store, 'todos').observers.length).toBe(1); //, 'observers count shouldn\'t change');
+    expect(getDNode(store.todos[0], 'title').observers.length).toBe(1); //, 'title observers should not have increased');
+
+    store.todos.push({
+      title: 'b',
+      completed: true
+    });
+
+    expect(container.querySelectorAll('li').length).toBe(2); //, 'list should two items in in the list');
+    let expectedOutput = [];
+    let nodes = container.querySelectorAll('li');
+
+    for (let i = 0; i < nodes.length; i++) {
+      expectedOutput.push(nodes[i].textContent);
+    }
+    expect(expectedOutput).toEqual(['|aa', '|b']);
+
+    expect(todoListRenderings).toBe(2); //'should have rendered list twice');
+    expect(todoItemRenderings).toBe(4); //, 'item2 should have rendered as well');
+    expect(getDNode(store, 'todos').observers.length).toBe(1); //, 'observers count shouldn\'t change');
+    expect(getDNode(store.todos[0], 'title').observers.length).toBe(1); //, 'title observers should not have increased');
+    expect(getDNode(store.todos[1], 'title').observers.length).toBe(1); //, 'title observers should have increased');
+    expect(getDNode(store.todos[1], 'completed').observers.length).toBe(0); //, 'completed observers should not have increased');
+
+    store.todos[1].title += 'b';
+
+    expect(container.querySelectorAll('li').length).toBe(2); //, 'list should two items in in the list');
+    expectedOutput = [];
+    nodes = container.querySelectorAll('li');
+
+    for (let i = 0; i < nodes.length; i++) {
+      expectedOutput.push(nodes[i].textContent);
+    }
+    expect(expectedOutput).toEqual(['|aa', '|bb']);
+
+    expect(todoListRenderings).toBe(2); //'should have rendered list twice');
+    expect(todoItemRenderings).toBe(5); //, 'item2 should have rendered as well');
+    expect(getDNode(store, 'todos').observers.length).toBe(1); //, 'observers count shouldn\'t change');
+    expect(getDNode(store.todos[0], 'title').observers.length).toBe(1); //, 'title observers should not have increased');
+    expect(getDNode(store.todos[1], 'title').observers.length).toBe(1); //, 'title observers should have increased');
+    expect(getDNode(store.todos[1], 'completed').observers.length).toBe(0); //, 'completed observers should not have increased');
+
+    const oldTodo = store.todos.pop();
+
+    expect(todoListRenderings).toBe(3); //, 'should have rendered list another time');
+    expect(todoItemRenderings).toBe(6); //, 'item1 should not have rerendered');
+    expect(container.querySelectorAll('li').length).toBe(1); //, 'list should have only on item in list now');
+    expect(getDNode(oldTodo, 'title').observers.length).toBe(0); //, 'title observers should have decreased');
+    expect(getDNode(oldTodo, 'completed').observers.length).toBe(0); //, 'completed observers should not have decreased');
+    render(null, container);
+    expect(todoItemUnmounts).toBe(2);
+    expect(todoItemUpdates).toBe(4);
+    expect(todoItemWillUpdates).toBe(4);
+    expect(getDNode(store, 'todos').observers.length).toBe(0);
+    expect(getDNode(store.todos[0], 'title').observers.length).toBe(0);
+  });
+
+  it('keep views alive', () => {
+    let yCalcCount = 0;
+    const data = observable({
+      x: 3,
+      get y() {
+        yCalcCount++;
+        return this.x * 2;
+      },
+      z: 'hi'
+    });
+
+    const TestComponent = observerWrap(() => {
+      return (
+        <div>
+          {data.z}
+          {data.y}
+        </div>
+      );
+    });
+
+    render(<TestComponent />, container);
+    expect(yCalcCount).toBe(1);
+    expect(container.textContent).toBe('hi6');
+
+    data.z = 'hello';
+    // test: rerender should not need a recomputation of data.y because the subscription is kept alive
+
+    expect(yCalcCount).toBe(1);
+
+    expect(container.textContent).toBe('hello6');
+    expect(yCalcCount).toBe(1);
+
+    expect(getDNode(data, 'y').observers.length).toBe(1);
+
+    render(<div />, container);
+
+    expect(getDNode(data, 'y').observers.length).toBe(0);
+  });
+
+  it('issue 12', function () {
+    const data = observable({
+      selected: 'coffee',
+      items: [
+        {
+          name: 'coffee'
+        },
+        {
+          name: 'tea'
+        }
+      ]
+    });
+
+    const Row = observerWrap(({ item }) => {
+      return (
+        <span>
+          {item.name}
+          {data.selected === item.name ? '!' : ''}
+        </span>
+      );
+    });
+
+    const Table = observerWrap(() => {
+      return (
+        <div>
+          {data.items.map((item) => (
+            <Row key={item.name} item={item} />
+          ))}
+        </div>
+      );
+    });
+
+    render(<Table />, container);
+
+    expect(container.querySelector('div').textContent).toBe('coffee!tea');
+
+    runInAction(() => {
+      data.items[1].name = 'boe';
+      data.items.splice(0, 2, { name: 'soup' });
+      data.selected = 'tea';
+    });
+
+    expect(container.querySelector('div').textContent).toBe('soup');
+  });
+
+  it('Callbacks are bound on render', function () {
+    const data = observable({
+      name: 'tea'
+    });
+
+    let a = 0;
+    let x = 0;
+    let y = 0;
+    let z = 0;
+
+    const ViewFn = ({ item }) => {
+      return (
+        <span>
+          {item.name}
+        </span>
+      );
+    };
+
+    const View = observerWrap(ViewFn);
+
+    const check = ({item: prev}, {item: next}) => prev !== next;
+
+    render(<View item={data} onComponentWillUpdate={(p, n) => p !== n? a++ : x++} />,
+      container);
+
+    expect(a).toBe(0);
+    expect(x).toBe(0);
+
+    runInAction(() => {
+      data.name = 'coffee';
+    });
+
+    expect(x).toBe(1);
+
+    render(<View item={data} onComponentWillUpdate={(p, n) => p !== n? a++ : y++} />,
+      container);
+
+    expect(a).toBe(1);
+    expect(x).toBe(1);
+    expect(y).toBe(0);
+
+    runInAction(() => {
+      data.name = 'soda';
+    });
+
+    expect(a).toBe(1);
+    expect(x).toBe(1);
+    expect(y).toBe(1);
+
+    render(<View item={data} onComponentWillUpdate={(p, n) => p !== n? a++ : z++} onComponentShouldUpdate={check} />,
+      container);
+
+    expect(a).toBe(1);
+    expect(z).toBe(0);
+    expect(y).toBe(1);
+
+    runInAction(() => {
+      data.name = 'water';
+    });
+
+    expect(y).toBe(2);
+
+    render(<View item={data} onComponentWillUpdate={(p, n) => p !== n? a++ : x++} />,
+      container);
+
+    expect(a).toBe(2);
+    expect(x).toBe(1);
+    expect(y).toBe(2);
+  
+    runInAction(() => {
+      data.name = 'juice';
+    });
+  
+    expect(a).toBe(2);
+    expect(y).toBe(2);
+    expect(x).toBe(2);
+  });
+
+  it('component should not be inject', () => {
+    const Foo = ({ foo }) => {
+      return (
+        <div>
+          context:
+          {foo}
+        </div>
+      );
+    };
+    // inject wraps in a class component, which will make observerWrap throw
+    expect(() => observerWrap(observer(Foo))).toThrow();
+  });
+
+  it('component should not be observer', () => {
+    const Foo = ({ foo }) => {
+      return (
+        <div>
+          context:
+          {foo}
+        </div>
+      );
+    };
+    // observer wraps in a class component, which will make observerWrap throw
+    expect(() => observerWrap(observer(Foo))).toThrow();
+  });
+
+  it('component should not be already be wrapped', () => {
+    const msg = [];
+    const baseWarn = console.error;
+    console.error = (m) => msg.push(m);
+
+    observerWrap(observerWrap(
+      ({ foo }) => {
+        return (
+          <div>
+            context:
+            {foo}
+          </div>
+        );
+      }
+    ));
+
+    expect(msg.length).toBe(1);
+    console.error = baseWarn;
+  });
+
+  it('observer component can be injected', (done) => {
+    const msg = [];
+    const baseWarn = console.error;
+    console.error = (m) => msg.push(m);
+
+    const fooA = inject('foo')(observerWrap(() => null));
+
+    render(<fooA />, container);
+
+    // N.B, the injected component will be observer since mobx-react 4.0!
+    const fooB = inject(() => {})(observerWrap(() => null));
+
+    expect(msg.length).toBe(0);
+    console.error = baseWarn;
+    done();
+  });
+
+  it('observerWrap should keep MobX from eating exceptions', () => {
+    const exception = new Error('dummy error');
+    const Faulty = observerWrap(() => {
+      throw exception;
+    });
+    expect(() => {
+      render(<Faulty />, container);
+    }).toThrow(exception);
+  });
+});