From b37bf1243445a2f9ccd482a1508ea345199b4a8f Mon Sep 17 00:00:00 2001 From: Anton Korzunov Date: Tue, 15 May 2018 19:56:18 +1000 Subject: [PATCH 1/5] disable pureSFC --- README.md | 2 ++ src/configuration.js | 1 + src/proxy/createClassProxy.js | 13 ++++++++----- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 848e25e17..9a25c1f5f 100644 --- a/README.md +++ b/README.md @@ -343,6 +343,8 @@ Set a new configuration for React Hot Loader. Available options are: * `logLevel`: specify log level, default to `"error"`, available values are: `['debug', 'log', 'warn', 'error']` +* `pureSFC`: enable Stateless Functional Component. If disabled they will be converted to React Components. + Default value: false. ```js import { setConfig } from 'react-hot-loader' diff --git a/src/configuration.js b/src/configuration.js index 20f37c84e..e4e4bfdac 100644 --- a/src/configuration.js +++ b/src/configuration.js @@ -1,5 +1,6 @@ const configuration = { logLevel: 'error', + pureSFC: false, } export default configuration diff --git a/src/proxy/createClassProxy.js b/src/proxy/createClassProxy.js index 380bf67b1..02d06fe5f 100644 --- a/src/proxy/createClassProxy.js +++ b/src/proxy/createClassProxy.js @@ -16,6 +16,7 @@ import { proxyClassCreator, } from './utils' import { inject, checkLifeCycleMethods, mergeComponents } from './inject' +import config from '../configuration' const has = Object.prototype.hasOwnProperty @@ -239,12 +240,14 @@ function createClassProxy(InitialComponent, proxyKey, options) { const result = CurrentComponent(props, context) // simple SFC - if (!CurrentComponent.contextTypes) { - if (!ProxyFacade.isStatelessFunctionalProxy) { - setSFPFlag(ProxyFacade, true) - } + if (config.pureSFC) { + if (!CurrentComponent.contextTypes) { + if (!ProxyFacade.isStatelessFunctionalProxy) { + setSFPFlag(ProxyFacade, true) + } - return renderOptions.componentDidRender(result) + return renderOptions.componentDidRender(result) + } } setSFPFlag(ProxyFacade, false) From cbc98eac541e5f121d228eb2be7f2da269dc00d4 Mon Sep 17 00:00:00 2001 From: Anton Korzunov Date: Tue, 15 May 2018 19:56:42 +1000 Subject: [PATCH 2/5] React.Context and Fragment related changes --- src/reconciler/hotReplacementRender.js | 33 +++++- test/proxy/consistency.test.js | 25 +++-- test/reconciler.test.js | 138 +++++++++++++++---------- 3 files changed, 130 insertions(+), 66 deletions(-) diff --git a/src/reconciler/hotReplacementRender.js b/src/reconciler/hotReplacementRender.js index 92a25e360..0958f3fab 100644 --- a/src/reconciler/hotReplacementRender.js +++ b/src/reconciler/hotReplacementRender.js @@ -22,6 +22,8 @@ const stackReport = () => { logger.warn('in', rev[0].name, rev) } +const REACT_CONTEXT_CURRENT_VALUE = '_currentValue' + const areNamesEqual = (a, b) => a === b || (UNDEFINED_NAMES[a] && UNDEFINED_NAMES[b]) const isReactClass = fn => fn && !!fn.render @@ -214,8 +216,13 @@ const mergeInject = (a, b, instance) => { const transformFlowNode = flow => flow.reduce((acc, node) => { - if (isFragmentNode(node) && node.props && node.props.children) { - return [...acc, ...filterNullArray(asArray(node.props.children))] + if (node && isFragmentNode(node)) { + if (node.props && node.props.children) { + return [...acc, ...filterNullArray(asArray(node.props.children))] + } + if (node.children) { + return [...acc, ...filterNullArray(asArray(node.children))] + } } return [...acc, node] }, []) @@ -279,6 +286,15 @@ const hotReplacementRender = (instance, stack) => { // text node if (typeof child !== 'object' || !stackChild || !stackChild.instance) { + if (stackChild && stackChild.children && stackChild.children.length) { + logger.error( + 'React-hot-loader: reconciliation failed', + 'could not dive into [', + child, + '] while some elements are still present in the tree.', + ) + stackReport() + } return } @@ -296,11 +312,20 @@ const hotReplacementRender = (instance, stack) => { return } - if (typeof child.type !== 'function') { + // React context consumer + if (child.type && typeof child.type === 'object' && child.type.Consumer) { + next({ + children: (child.props ? child.props.children : child.children[0])( + child.type[REACT_CONTEXT_CURRENT_VALUE], + ), + }) + } else if (typeof child.type !== 'function') { next( // move types from render to the instances of hydrated tree mergeInject( - asArray(child.props ? child.props.children : child.children), + transformFlowNode( + asArray(child.props ? child.props.children : child.children), + ), stackChild.instance.children, stackChild.instance, ), diff --git a/test/proxy/consistency.test.js b/test/proxy/consistency.test.js index a7f28e139..bc6b90b43 100644 --- a/test/proxy/consistency.test.js +++ b/test/proxy/consistency.test.js @@ -3,6 +3,7 @@ import React from 'react' import { createMounter, ensureNoWarnings } from './helper' import createProxy from '../../src/proxy' +import configuration from '../../src/configuration' const createFixtures = () => ({ modern: { @@ -349,13 +350,25 @@ describe('consistency', () => { expect(instance instanceof App).toBe(true) }) - it('should wrap SFC by SFC', () => { - const App = () =>
+ describe('should wrap SFC by SFC', () => { + it('should wrap SFC by SFC Component', () => { + const App = () =>
- const Proxy = createProxy(App).get() - expect('isStatelessFunctionalProxy' in Proxy).toBe(false) - mount().instance() - expect(Proxy.isStatelessFunctionalProxy).toBe(true) + const Proxy = createProxy(App).get() + expect('isStatelessFunctionalProxy' in Proxy).toBe(false) + mount().instance() + expect(Proxy.isStatelessFunctionalProxy).toBe(false) + }) + + it('should wrap SFC by SFC Pure', () => { + const App = () =>
+ configuration.pureSFC = true + const Proxy = createProxy(App).get() + expect('isStatelessFunctionalProxy' in Proxy).toBe(false) + mount().instance() + configuration.pureSFC = false + expect(Proxy.isStatelessFunctionalProxy).toBe(true) + }) }) it('should wrap SFC with Context by Proxy', () => { diff --git a/test/reconciler.test.js b/test/reconciler.test.js index 139165067..37e4733e0 100644 --- a/test/reconciler.test.js +++ b/test/reconciler.test.js @@ -5,6 +5,7 @@ import { increment as incrementGeneration } from '../src/global/generation' import { areComponentsEqual } from '../src/utils.dev' import logger from '../src/logger' import reactHotLoader from '../src/reactHotLoader' +import configuration from '../src/configuration' jest.mock('../src/logger') @@ -361,72 +362,97 @@ describe('reconciler', () => { expect(wrapper.text()).toContain(43) }) - it('should assmeble props for nested children', () => { - const RenderChildren = ({ children }) =>
{children}
- const RenderProp = jest - .fn() - .mockImplementation(({ prop }) =>
{prop}
) - const DefaultProp = jest.fn().mockImplementation(({ prop }) => ( -
- {prop ? ( -
42
- ) : ( -
-
24
-
- )} -
- )) - DefaultProp.defaultProps = { - prop: 'defaultValue', - } - - const App = () => ( - + describe('should assmeble props for nested children', () => { + const testSuite = () => { + const RenderChildren = ({ children }) =>
{children}
+ const RenderProp = jest + .fn() + .mockImplementation(({ prop }) =>
{prop}
) + const DefaultProp = jest.fn().mockImplementation(({ prop }) => (
- -
-
-
- + {prop ? ( +
42
+ ) : ( +
+
24
+
+ )} +
+ )) + DefaultProp.defaultProps = { + prop: 'defaultValue', + } + + const App = () => ( + +
+ +
+
+
+ +
-
-
-
- - +
+
+ + +
-
- -
-
- ) + +
+ + ) - logger.warn.mockClear() + logger.warn.mockClear() - const wrapper = mount( - -
- -
-
, - ) + const wrapper = mount( + +
+ +
+
, + ) - incrementGeneration() - wrapper.setProps({ update: 'now' }) + incrementGeneration() + wrapper.setProps({ update: 'now' }) + return { RenderProp, DefaultProp } + } + + it('for Component SFC', () => { + const { RenderProp, DefaultProp } = testSuite() - expect(RenderProp).toHaveBeenCalledTimes(4) - expect(RenderProp.mock.calls[0][0]).toEqual({ value: 42 }) - expect(RenderProp.mock.calls[1][0]).toEqual({ value: 24 }) - expect(RenderProp.mock.calls[2][0]).toEqual({ value: 42 }) - expect(RenderProp.mock.calls[3][0]).toEqual({ value: 24 }) + expect(RenderProp).toHaveBeenCalledTimes(6) + expect(RenderProp.mock.calls[0][0]).toEqual({ value: 42 }) + expect(RenderProp.mock.calls[1][0]).toEqual({ value: 24 }) + expect(RenderProp.mock.calls[2][0]).toEqual({ value: 42 }) + expect(RenderProp.mock.calls[3][0]).toEqual({ value: 24 }) - expect(DefaultProp).toHaveBeenCalledTimes(2) - expect(DefaultProp.mock.calls[0][0]).toEqual({ prop: 'defaultValue' }) - expect(DefaultProp.mock.calls[1][0]).toEqual({ prop: 'defaultValue' }) + expect(DefaultProp).toHaveBeenCalledTimes(3) + expect(DefaultProp.mock.calls[0][0]).toEqual({ prop: 'defaultValue' }) + expect(DefaultProp.mock.calls[1][0]).toEqual({ prop: 'defaultValue' }) + + expect(logger.warn).not.toHaveBeenCalled() + }) - expect(logger.warn).not.toHaveBeenCalled() + it('for Pure SFC', () => { + configuration.pureSFC = true + const { RenderProp, DefaultProp } = testSuite() + configuration.pureSFC = false + + expect(RenderProp).toHaveBeenCalledTimes(4) + expect(RenderProp.mock.calls[0][0]).toEqual({ value: 42 }) + expect(RenderProp.mock.calls[1][0]).toEqual({ value: 24 }) + expect(RenderProp.mock.calls[2][0]).toEqual({ value: 42 }) + expect(RenderProp.mock.calls[3][0]).toEqual({ value: 24 }) + + expect(DefaultProp).toHaveBeenCalledTimes(2) + expect(DefaultProp.mock.calls[0][0]).toEqual({ prop: 'defaultValue' }) + expect(DefaultProp.mock.calls[1][0]).toEqual({ prop: 'defaultValue' }) + + expect(logger.warn).not.toHaveBeenCalled() + }) }) describe('when an error occurs in render', () => { From c81ca5c54b7ca1d27dbf5ed40233c7919086478e Mon Sep 17 00:00:00 2001 From: Anton Korzunov Date: Tue, 15 May 2018 20:05:33 +1000 Subject: [PATCH 3/5] secure context render --- src/reconciler/hotReplacementRender.js | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/reconciler/hotReplacementRender.js b/src/reconciler/hotReplacementRender.js index 0958f3fab..2d94d4ce8 100644 --- a/src/reconciler/hotReplacementRender.js +++ b/src/reconciler/hotReplacementRender.js @@ -314,11 +314,15 @@ const hotReplacementRender = (instance, stack) => { // React context consumer if (child.type && typeof child.type === 'object' && child.type.Consumer) { - next({ - children: (child.props ? child.props.children : child.children[0])( - child.type[REACT_CONTEXT_CURRENT_VALUE], - ), - }) + try { + next({ + children: (child.props ? child.props.children : child.children[0])( + child.type[REACT_CONTEXT_CURRENT_VALUE], + ), + }) + } catch (e) { + // do nothing, yet + } } else if (typeof child.type !== 'function') { next( // move types from render to the instances of hydrated tree From 9ebf0afea1c57e671b1cfca89cd1b686d252de74 Mon Sep 17 00:00:00 2001 From: Anton Korzunov Date: Tue, 15 May 2018 21:29:24 +1000 Subject: [PATCH 4/5] add tests for Fragments inside div --- test/AppContainer.dev.test.js | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/test/AppContainer.dev.test.js b/test/AppContainer.dev.test.js index fd38a4f65..be765185b 100644 --- a/test/AppContainer.dev.test.js +++ b/test/AppContainer.dev.test.js @@ -1970,7 +1970,13 @@ describe(`AppContainer (dev)`, () => { const App = () => ( ) @@ -1998,7 +2004,13 @@ describe(`AppContainer (dev)`, () => { const App = () => ( ) @@ -2008,7 +2020,7 @@ describe(`AppContainer (dev)`, () => { } expect(unmount).toHaveBeenCalledTimes(0) - expect(wrapper.update().text()).toBe('another textinternal') + expect(wrapper.update().text()).toBe('another text internal') } else { // React 15 is always ok expect(true).toBe(true) From 56fdd05a948a81624d80c1afa3b420b1c952cda7 Mon Sep 17 00:00:00 2001 From: Anton Korzunov Date: Wed, 16 May 2018 21:17:31 +1000 Subject: [PATCH 5/5] Support React.Context, impliments #979 --- src/internal/reactUtils.js | 12 +++++++++ src/reconciler/hotReplacementRender.js | 34 +++++++++++++++++++++++--- 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/src/internal/reactUtils.js b/src/internal/reactUtils.js index ed751cd67..9c80ec72e 100644 --- a/src/internal/reactUtils.js +++ b/src/internal/reactUtils.js @@ -22,3 +22,15 @@ export const updateInstance = instance => { export const isFragmentNode = ({ type }) => React.Fragment && type === React.Fragment + +const ContextType = React.createContext ? React.createContext() : null +const ConsumerType = ContextType && ContextType.Consumer.$$typeof +const ProviderType = ContextType && ContextType.Provider.$$typeof + +export const CONTEXT_CURRENT_VALUE = '_currentValue' + +export const isContextConsumer = ({ type }) => + type && typeof type === 'object' && type.$$typeof === ConsumerType +export const isContextProvider = ({ type }) => + type && typeof type === 'object' && type.$$typeof === ProviderType +export const getContextProvider = type => type && type._context diff --git a/src/reconciler/hotReplacementRender.js b/src/reconciler/hotReplacementRender.js index 2d94d4ce8..b77710752 100644 --- a/src/reconciler/hotReplacementRender.js +++ b/src/reconciler/hotReplacementRender.js @@ -5,6 +5,10 @@ import { updateInstance, getComponentDisplayName, isFragmentNode, + isContextConsumer, + isContextProvider, + getContextProvider, + CONTEXT_CURRENT_VALUE, } from '../internal/reactUtils' import reactHotLoader from '../reactHotLoader' import logger from '../logger' @@ -22,7 +26,9 @@ const stackReport = () => { logger.warn('in', rev[0].name, rev) } -const REACT_CONTEXT_CURRENT_VALUE = '_currentValue' +const emptyMap = new Map() +const stackContext = () => + (renderStack[renderStack.length - 1] || {}).context || emptyMap const areNamesEqual = (a, b) => a === b || (UNDEFINED_NAMES[a] && UNDEFINED_NAMES[b]) @@ -253,10 +259,12 @@ const scheduleInstanceUpdate = instance => { const hotReplacementRender = (instance, stack) => { if (isReactClass(instance)) { const type = getElementType(stack) + renderStack.push({ name: getComponentDisplayName(type), type, props: stack.instance.props, + context: stackContext(), }) } const flow = transformFlowNode(filterNullArray(asArray(render(instance)))) @@ -312,18 +320,35 @@ const hotReplacementRender = (instance, stack) => { return } - // React context consumer - if (child.type && typeof child.type === 'object' && child.type.Consumer) { + // React context + if (isContextConsumer(child)) { try { next({ children: (child.props ? child.props.children : child.children[0])( - child.type[REACT_CONTEXT_CURRENT_VALUE], + stackContext().get(child.type) || child.type[CONTEXT_CURRENT_VALUE], ), }) } catch (e) { // do nothing, yet } } else if (typeof child.type !== 'function') { + // React + let childName = child.type ? getComponentDisplayName(child.type) : 'empty' + let extraContext = stackContext() + + if (isContextProvider(child)) { + extraContext = new Map(extraContext) + extraContext.set(getContextProvider(child.type), child.props.value) + childName = 'ContextProvider' + } + + renderStack.push({ + name: childName, + type: child.type, + props: stack.instance.props, + context: extraContext, + }) + next( // move types from render to the instances of hydrated tree mergeInject( @@ -334,6 +359,7 @@ const hotReplacementRender = (instance, stack) => { stackChild.instance, ), ) + renderStack.pop() } else { // unwrap proxy const childType = getElementType(child)