fix: use safe defineProperty

Also warn about problems caused by `hot` setup.

docs/Troubleshooting.md

@@ -287,3 +287,27 @@ modules with `hot` exported components – functions, constants, anything NOT RE
 #### RHL is not working with Webpack DLL
 
 React Hot Loader rely on a Babel transformation that register all exports in a global. That's why dependencies included in [Webpack DLL](https://webpack.js.org/plugins/dll-plugin/#dllplugin) will not work.
+
+#### React-hot-loader: fatal error caused by XXX - no instrumentation found.
+
+React-hot-loader found an Element without instrumentation due to a wrong configuration.
+To fix this issue - just require RHL before React.
+
+Example of a wrong configuration:
+
+```js
+import * as React from 'react'
+import { hot } from 'react-hot-loader' // React is not patched
+```
+
+Example of correct configurations:
+
+```js
+import { hot } from 'react-hot-loader'
+import * as React from 'react' // React is now patched
+```
+
+```js
+import React from 'react'
+import { hot } from 'react-hot-loader' // React is now patched
+```

examples/styled-components/src/App.js

@@ -1,15 +1,15 @@
+import { hot } from 'react-hot-loader'
 import * as React from 'react'
 import styled from 'styled-components'
 import emoStyled from 'react-emotion'
-import { hot } from 'react-hot-loader'
 import Counter from './Counter'
 
 const BigText = styled.div`
   font-size: 20px;
 `
 
 const SmallText = emoStyled('div')`
-  font-size: 12px;
+  font-size: 22px;
 `
 
 const App = () => (

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

@@ -199,14 +199,24 @@ const hotReplacementRender = (instance, stack) => {
     } else {
       // unwrap proxy
       const childType = getElementType(child)
+      if (!stackChild.type[PROXY_KEY]) {
+        /* eslint-disable no-console */
+        logger.error(
+          'React-hot-loader: fatal error caused by ',
+          stackChild.type,
+          ' - no instrumentation found. ',
+          'Please require react-hot-loader before React. More in troubleshooting.',
+        )
+        throw new Error('React-hot-loader: wrong configuration')
+      }
 
       if (child.type === stackChild.type) {
         next(stackChild.instance)
       } else if (isSwappable(childType, stackChild.type)) {
         // they are both registered, or have equal code/displayname/signature
 
         // update proxy using internal PROXY_KEY
-        updateProxyById(stackChild.instance[PROXY_KEY], childType)
+        updateProxyById(stackChild.type[PROXY_KEY], childType)
 
         next(stackChild.instance)
       } else if (reactHotLoader.debug) {

packages/react-stand-in/src/createClassProxy.js

@@ -1,7 +1,12 @@
 import { Component } from 'react'
 import transferStaticProps from './transferStaticProps'
 import { GENERATION, PROXY_KEY, UNWRAP_PROXY } from './constants'
-import { getDisplayName, isReactClass, identity } from './utils'
+import {
+  getDisplayName,
+  isReactClass,
+  identity,
+  safeDefineProperty,
+} from './utils'
 import { inject, checkLifeCycleMethods, mergeComponents } from './inject'
 
 const proxies = new WeakMap()
@@ -38,7 +43,6 @@ function createClassProxy(InitialComponent, proxyKey, wrapResult = identity) {
       super(props, context)
 
       this[GENERATION] = 0
-      this[PROXY_KEY] = proxyKey
 
       // As long we can't override constructor
       // every class shall evolve from a base class
@@ -75,12 +79,28 @@ function createClassProxy(InitialComponent, proxyKey, wrapResult = identity) {
     return CurrentComponent
   }
 
-  ProxyComponent.toString = function toString() {
-    return CurrentComponent.toString()
-  }
+  safeDefineProperty(ProxyComponent, UNWRAP_PROXY, {
+    configurable: false,
+    writable: false,
+    enumerable: false,
+    value: getCurrent,
+  })
 
-  ProxyComponent[UNWRAP_PROXY] = getCurrent
-  ProxyComponent.RHL_PROXY_ID = proxyKey
+  safeDefineProperty(ProxyComponent, PROXY_KEY, {
+    configurable: false,
+    writable: false,
+    enumerable: false,
+    value: proxyKey,
+  })
+
+  safeDefineProperty(ProxyComponent, 'toString', {
+    configurable: true,
+    writable: false,
+    enumerable: false,
+    value: function toString() {
+      return String(CurrentComponent)
+    },
+  })
 
   function update(NextComponent) {
     if (typeof NextComponent !== 'function') {
@@ -110,13 +130,9 @@ function createClassProxy(InitialComponent, proxyKey, wrapResult = identity) {
     const displayName = getDisplayName(CurrentComponent)
     ProxyComponent.displayName = displayName
 
-    try {
-      Object.defineProperty(ProxyComponent, 'name', {
-        value: displayName,
-      })
-    } catch (err) {
-      // Ignore error, it is not very important
-    }
+    safeDefineProperty(ProxyComponent, 'name', {
+      value: displayName,
+    })
 
     savedDescriptors = transferStaticProps(
       ProxyComponent,

packages/react-stand-in/src/inject.js

@@ -4,6 +4,7 @@ import {
   safeReactConstructor,
   getOwnKeys,
   shallowStringsEqual,
+  deepPrototypeUpdate,
 } from './utils'
 import { REGENERATE_METHOD, PREFIX, GENERATION } from './constants'
 import config from './config'
@@ -20,7 +21,7 @@ function mergeComponents(
 
     try {
       // Bypass babel class inheritance checking
-      InitialComponent.prototype = NextComponent.prototype
+      deepPrototypeUpdate(InitialComponent, NextComponent)
     } catch (e) {
       // It was ES6 class
     }

packages/react-stand-in/src/transferStaticProps.js

@@ -1,4 +1,6 @@
 import shallowEqual from 'shallowequal'
+import { safeDefineProperty } from './utils'
+import { PROXY_KEY, UNWRAP_PROXY } from './constants'
 
 const RESERVED_STATICS = [
   'length',
@@ -9,6 +11,8 @@ const RESERVED_STATICS = [
   'prototype',
   'toString',
   'valueOf',
+  PROXY_KEY,
+  UNWRAP_PROXY,
 ]
 
 function transferStaticProps(
@@ -26,7 +30,7 @@ function transferStaticProps(
     const savedDescriptor = savedDescriptors[key]
 
     if (!shallowEqual(prevDescriptor, savedDescriptor)) {
-      Object.defineProperty(NextComponent, key, prevDescriptor)
+      safeDefineProperty(NextComponent, key, prevDescriptor)
     }
   })
 
@@ -46,12 +50,12 @@ function transferStaticProps(
       savedDescriptor &&
       !shallowEqual(savedDescriptor, prevDescriptor)
     ) {
-      Object.defineProperty(NextComponent, key, prevDescriptor)
+      safeDefineProperty(NextComponent, key, prevDescriptor)
       return
     }
 
     if (prevDescriptor && !savedDescriptor) {
-      Object.defineProperty(ProxyComponent, key, prevDescriptor)
+      safeDefineProperty(ProxyComponent, key, prevDescriptor)
       return
     }
 
@@ -61,7 +65,7 @@ function transferStaticProps(
     }
 
     savedDescriptors[key] = nextDescriptor
-    Object.defineProperty(ProxyComponent, key, nextDescriptor)
+    safeDefineProperty(ProxyComponent, key, nextDescriptor)
   })
 
   // Remove static methods and properties that are no longer defined
@@ -93,7 +97,7 @@ function transferStaticProps(
       return
     }
 
-    Object.defineProperty(ProxyComponent, key, {
+    safeDefineProperty(ProxyComponent, key, {
       value: undefined,
     })
   })

packages/react-stand-in/src/utils.js

@@ -1,3 +1,5 @@
+import config from './config'
+
 export function getDisplayName(Component) {
   const displayName = Component.displayName || Component.name
   return displayName && displayName !== 'ReactComponent'
@@ -57,3 +59,22 @@ export function shallowStringsEqual(a, b) {
   }
   return true
 }
+
+export function deepPrototypeUpdate(dest, source) {
+  const deepDest = Object.getPrototypeOf(dest)
+  const deepSrc = Object.getPrototypeOf(source)
+  if (deepDest && deepSrc && deepSrc !== deepDest) {
+    deepPrototypeUpdate(deepDest, deepSrc)
+  }
+  if (source.prototype && source.prototype !== dest.prototype) {
+    dest.prototype = source.prototype
+  }
+}
+
+export function safeDefineProperty(target, key, props) {
+  try {
+    Object.defineProperty(target, key, props)
+  } catch (e) {
+    config.logger.warn('Error while wrapping', key, ' -> ', e)
+  }
+}

packages/react-stand-in/test/consistency.test.js

@@ -2,7 +2,7 @@
 /* eslint-disable react/no-render-return-value */
 import React from 'react'
 import { createMounter, ensureNoWarnings } from './helper'
-import createProxy from '../lib'
+import createProxy, { setConfig } from '../lib'
 
 const createFixtures = () => ({
   modern: {
@@ -71,7 +71,16 @@ describe('consistency', () => {
       let Bar
       let Baz
       let Foo
+      let logger
+
       beforeEach(() => {
+        logger = {
+          debug: jest.fn(),
+          log: jest.fn(),
+          warn: jest.fn(),
+          error: jest.fn(),
+        }
+        setConfig({ logger })
         ;({ Bar, Baz, Foo } = createFixtures()[type])
       })
 
@@ -173,6 +182,19 @@ describe('consistency', () => {
         expect(barInstance instanceof BazProxy).toBe(true)
       })
 
+      it('replaces toString (if readable)', () => {
+        const barProxy = createProxy(Bar, 'bar')
+        const BarProxy = barProxy.get()
+        expect(BarProxy.toString()).toMatch(/^(class|function) Bar/)
+
+        Object.defineProperty(Foo, 'toString', {
+          configurable: false,
+          value: Foo.toString,
+          writable: false,
+        })
+        createProxy(Foo, 'foo')
+      })
+
       it('sets up displayName from displayName or name', () => {
         const proxy = createProxy(Bar)
         const Proxy = proxy.get()