From 9967fde306d64341cce0fb59efa3fd947ddcff23 Mon Sep 17 00:00:00 2001 From: Anton Korzunov Date: Thu, 14 Feb 2019 20:37:58 +1100 Subject: [PATCH] feat: make errors retryable to mitigate hooks update --- README.md | 13 ++++- src/AppContainer.dev.js | 8 ++- src/errorReporter.js | 73 +++++++++++++++++++----- src/internal/stack/hydrateLegacyStack.js | 1 + src/reconciler/proxyAdapter.js | 16 ++++-- src/webpack/patch.js | 4 ++ test/reconciler.test.js | 2 +- 7 files changed, 95 insertions(+), 22 deletions(-) diff --git a/README.md b/README.md index d87dfaf85..5b198b6e0 100644 --- a/README.md +++ b/README.md @@ -428,10 +428,17 @@ You may add one more babel-loader, with only one React-Hot-Loader plugin inside ##### React-Hooks -* React-Hooks should work out of the box if you are using version 4.6.0 or above (`pureSFC` is enabled by default). -* Having dom-patch enabled would solve any possible issue (`ignoreSFC` option is enabled) +React hooks are not _really_ supported by React-Hot-Loader. Mostly due to our internal +processes of re-rendering React Tree, which is requited to reconcile the updated application +before React will try to rerender it, and purge everything by the way. -You can always `cold` component, but any update then would cause a state loss. +* hooks **should work** for versions 4.6.0 and above (`pureSFC` is enabled by default). +* hooks will produce **errors** on every hot-update without patches to `react-dom`. +* hooks **may loss the state** without patches to `react-dom`. +* hooks does not support adding new hooks on the fly + +The most safe way to use hooks - `cold` them, so make them not hot-reloadable, but they would work +without pain (any update then would cause a state loss) * _cold_ components using hooks. diff --git a/src/AppContainer.dev.js b/src/AppContainer.dev.js index 55c881379..6741eda86 100644 --- a/src/AppContainer.dev.js +++ b/src/AppContainer.dev.js @@ -56,6 +56,10 @@ class AppContainer extends React.Component { }) } + retryHotLoaderError = () => { + this.setState({ error: null }) + } + render() { const { error, errorInfo } = this.state @@ -65,7 +69,9 @@ class AppContainer extends React.Component { } = this.props if (error && this.props.errorBoundary) { - return + return ( + + ) } if (this.hotComponentUpdate) { diff --git a/src/errorReporter.js b/src/errorReporter.js index 660805b42..0515d2df7 100644 --- a/src/errorReporter.js +++ b/src/errorReporter.js @@ -1,12 +1,14 @@ /* global document */ /* eslint-disable react/no-array-index-key */ /* eslint-disable jsx-a11y/accessible-emoji */ +/* eslint-disable no-use-before-define */ import React from 'react' import ReactDom from 'react-dom' import configuration from './configuration' import { getComponentDisplayName } from './internal/reactUtils' +import { enterHotUpdate } from './global/generation' let lastError = [] @@ -21,7 +23,7 @@ const overlayStyle = { color: '#000', fontFamily: '-apple-system, BlinkMacSystemFont, "Segoe UI", "Roboto", "Oxygen", "Ubuntu", "Fira Sans", "Droid Sans", "Helvetica Neue", sans-serif', - + fontSize: '12px', margin: 0, padding: '16px', maxHeight: '50%', @@ -32,6 +34,11 @@ const inlineErrorStyle = { backgroundColor: '#FEE', } +const liCounter = { + position: 'absolute', + left: '10px', +} + const listStyle = {} export const EmptyErrorPlaceholder = ({ component }) => ( @@ -39,19 +46,36 @@ export const EmptyErrorPlaceholder = ({ component }) => ( ⚛️🔥🤕 ({component ? getComponentDisplayName(component.constructor || component) : 'Unknown location'}) + {component && + component.retryHotLoaderError && ( + + )} ) +const errorHeader = (component, componentStack) => { + if (component || componentStack) { + return ( + + ( + {component + ? getComponentDisplayName(component.constructor || component) + : 'Unknown location'} + {component && ', '} + {componentStack && componentStack.split('\n').filter(Boolean)[0]} + ) + + ) + } + return null +} + const mapError = ({ error, errorInfo, component }) => ( -
+

- {component && ( - - ({component - ? getComponentDisplayName(component.constructor || component) - : 'Unknown location'}) - - )} + {errorHeader(component, errorInfo && errorInfo.componentStack)}{' '} {error.toString ? error.toString() : error.message || 'undefined error'}

{errorInfo && errorInfo.componentStack ? ( @@ -62,6 +86,7 @@ const mapError = ({ error, errorInfo, component }) => ( .split('\n') .slice(1, 2) .map((line, i) =>
  • {line}
    • )} +
    {errorInfo.componentStack .split('\n') .filter(Boolean) @@ -80,7 +105,7 @@ const mapError = ({ error, errorInfo, component }) => (
    ) )} - + ) class ErrorOverlay extends React.Component { @@ -90,6 +115,20 @@ class ErrorOverlay extends React.Component { toggle = () => this.setState({ visible: !this.state.visible }) + retry = () => + this.setState(() => { + const { errors } = this.props + enterHotUpdate() + clearExceptions() + errors + .map(({ component }) => component) + .filter(Boolean) + .filter(({ retryHotLoaderError }) => !!retryHotLoaderError) + .forEach(component => component.retryHotLoaderError()) + + return {} + }) + render() { const { errors } = this.props if (!errors.length) { @@ -103,10 +142,18 @@ class ErrorOverlay extends React.Component { + {visible && (
      - {errors.map((err, i) =>
    • {mapError(err)}
      • )} + {errors.map((err, i) => ( +
    • + + ({i + 1}/{errors.length}) + + {mapError(err)} +
      • + ))}
    )} @@ -132,14 +179,14 @@ const initErrorOverlay = () => { } } -export const clearExceptions = () => { +export function clearExceptions() { if (lastError.length) { lastError = [] initErrorOverlay() } } -export const logException = (error, errorInfo, component) => { +export function logException(error, errorInfo, component) { // do not suppress error /* eslint-disable no-console */ diff --git a/src/internal/stack/hydrateLegacyStack.js b/src/internal/stack/hydrateLegacyStack.js index ccc77e114..94fb91632 100644 --- a/src/internal/stack/hydrateLegacyStack.js +++ b/src/internal/stack/hydrateLegacyStack.js @@ -2,6 +2,7 @@ function pushState(stack, type, instance) { stack.type = type + stack.elementType = type stack.children = [] stack.instance = instance || stack diff --git a/src/reconciler/proxyAdapter.js b/src/reconciler/proxyAdapter.js index 9daab74eb..0293d4a4e 100644 --- a/src/reconciler/proxyAdapter.js +++ b/src/reconciler/proxyAdapter.js @@ -68,16 +68,14 @@ const OLD_RENDER = 'react-hot-loader-original-render' function componentDidCatch(error, errorInfo) { this[ERROR_STATE] = { + location: 'boundary', error, errorInfo, generation: getGeneration(), } Object.getPrototypeOf(this)[ERROR_STATE] = this[ERROR_STATE] if (!configuration.errorReporter) { - logException({ - error, - errorInfo, - }) + logException(error, errorInfo, this) } this.forceUpdate() } @@ -95,6 +93,7 @@ function componentRender() { return this[OLD_RENDER].render.call(this) } catch (renderError) { this[ERROR_STATE] = { + location: 'render', error: renderError, generation: getGeneration(), } @@ -105,6 +104,11 @@ function componentRender() { } } +function retryHotLoaderError() { + delete this[ERROR_STATE] + this.forceUpdate() +} + setComparisonHooks( () => ({}), ({ prototype }) => { @@ -118,9 +122,11 @@ setComparisonHooks( render: prototype.render, } prototype.componentDidCatch = componentDidCatch + prototype.retryHotLoaderError = retryHotLoaderError prototype.render = componentRender } + delete prototype[ERROR_STATE] }, () => forEachKnownClass(({ prototype }) => { @@ -132,11 +138,13 @@ setComparisonHooks( // keep render hooked } else { delete prototype.componentDidCatch + delete prototype.retryHotLoaderError if (!prototype[OLD_RENDER].descriptor) { delete prototype.render } else { prototype.render = prototype[OLD_RENDER].descriptor } + delete prototype[ERROR_STATE] delete prototype[OLD_RENDER] } } diff --git a/src/webpack/patch.js b/src/webpack/patch.js index 035a254d2..90325f0aa 100644 --- a/src/webpack/patch.js +++ b/src/webpack/patch.js @@ -93,6 +93,10 @@ function transform(source) { // early reject return source; } + if (source.indexOf(sign) >= 0) { + // already patched + return; + } for (const key in injectionStart) { if ( source.indexOf(injectionStart[key][0]) > 0 && diff --git a/test/reconciler.test.js b/test/reconciler.test.js index 11adee0cd..9aebec30b 100644 --- a/test/reconciler.test.js +++ b/test/reconciler.test.js @@ -549,7 +549,7 @@ describe('reconciler', () => { console.error.mockRestore() }) - it('should catch error to the boundary', async () => { + it('should catch error to the boundary', () => { if (!React.Suspense) { // this test is unstable on React 15 expect(true).toBe(true)