From a5a7c9316d397e71015bb0ebbde34efe280b7ab5 Mon Sep 17 00:00:00 2001 From: Anton Korzunov Date: Fri, 6 Apr 2018 21:35:06 +1000 Subject: [PATCH 1/5] remove componentWillReceiveProps from proxies #918 --- examples/styled-components/package.json | 2 +- src/AppContainer.dev.js | 15 ++++-------- src/proxy/createClassProxy.js | 12 +++++----- src/reconciler/hotReplacementRender.js | 4 ++++ src/reconciler/index.js | 5 ++-- src/reconciler/proxyAdapter.js | 17 ++++++------- test/AppContainer.dev.test.js | 16 ++++++++----- test/proxy/consistency.test.js | 2 +- test/proxy/lifecycle-method.test.js | 15 ++++++++---- test/reconciler.test.js | 32 +++++++++++++------------ test/reconciler/proxyAdapter.test.js | 15 ++++++++++++ 11 files changed, 82 insertions(+), 53 deletions(-) diff --git a/examples/styled-components/package.json b/examples/styled-components/package.json index dfd558580..9ada42bb5 100644 --- a/examples/styled-components/package.json +++ b/examples/styled-components/package.json @@ -20,7 +20,7 @@ "react": "^16.2.0", "react-dom": "^16.2.0", "react-emotion": "^8.0.12", - "react-hot-loader": "next", + "react-hot-loader": "^4.0.1", "styled-components": "^2.4.0" } } diff --git a/src/AppContainer.dev.js b/src/AppContainer.dev.js index fd2321caa..b256b62fd 100644 --- a/src/AppContainer.dev.js +++ b/src/AppContainer.dev.js @@ -15,20 +15,15 @@ class AppContainer extends React.Component { } } - componentWillReceiveProps() { - if (this.state.generation !== getGeneration()) { + static getDerivedStateFromProps(nextProps, prevState) { + if (prevState.generation !== getGeneration()) { // Hot reload is happening. - - this.setState({ + return { error: null, generation: getGeneration(), - }) - - // perform sandboxed render to find similarities between new and old code - renderReconciler(this, true) - // it is possible to flush update out of render cycle - flushScheduledUpdates() + } } + return null } shouldComponentUpdate(prevProps, prevState) { diff --git a/src/proxy/createClassProxy.js b/src/proxy/createClassProxy.js index 883c0837c..87d2a88fd 100644 --- a/src/proxy/createClassProxy.js +++ b/src/proxy/createClassProxy.js @@ -38,8 +38,8 @@ const blackListedClassMembers = [ ] const defaultRenderOptions = { - componentWillReceiveProps: identity, componentWillRender: identity, + componentDidUpdate: result => result, componentDidRender: result => result, } @@ -129,9 +129,9 @@ function createClassProxy(InitialComponent, proxyKey, options) { target[PROXY_IS_MOUNTED] = true }, ) - const componentWillReceiveProps = lifeCycleWrapperFactory( - 'componentWillReceiveProps', - renderOptions.componentWillReceiveProps, + const componentDidUpdate = lifeCycleWrapperFactory( + 'componentDidUpdate', + renderOptions.componentDidUpdate, ) const componentWillUnmount = lifeCycleWrapperFactory( 'componentWillUnmount', @@ -157,7 +157,7 @@ function createClassProxy(InitialComponent, proxyKey, options) { result = CurrentComponent.prototype.render.call(this) } - return renderOptions.componentDidRender(result) + return renderOptions.componentDidRender.call(this, result) } function proxiedRender() { @@ -171,7 +171,7 @@ function createClassProxy(InitialComponent, proxyKey, options) { render: proxiedRender, hotComponentRender, componentDidMount, - componentWillReceiveProps, + componentDidUpdate, componentWillUnmount, }) } diff --git a/src/reconciler/hotReplacementRender.js b/src/reconciler/hotReplacementRender.js index c274ef0eb..d39c88fef 100644 --- a/src/reconciler/hotReplacementRender.js +++ b/src/reconciler/hotReplacementRender.js @@ -221,6 +221,10 @@ export const flushScheduledUpdates = () => { ) } +export const unscheduleUpdate = instance => { + scheduledUpdates = scheduledUpdates.filter(inst => inst !== instance) +} + const scheduleInstanceUpdate = instance => { scheduledUpdates.push(instance) if (!scheduledUpdate) { diff --git a/src/reconciler/index.js b/src/reconciler/index.js index d2e12f8e9..251946b4e 100644 --- a/src/reconciler/index.js +++ b/src/reconciler/index.js @@ -1,11 +1,12 @@ import getReactStack from '../internal/getReactStack' import hotReplacementRender, { - flushScheduledUpdates + flushScheduledUpdates, + unscheduleUpdate } from './hotReplacementRender' const reconcileHotReplacement = ReactInstance => hotReplacementRender(ReactInstance, getReactStack(ReactInstance)) -export { flushScheduledUpdates } +export { flushScheduledUpdates, unscheduleUpdate } export default reconcileHotReplacement diff --git a/src/reconciler/proxyAdapter.js b/src/reconciler/proxyAdapter.js index fd20a6a37..dc4eb55ab 100644 --- a/src/reconciler/proxyAdapter.js +++ b/src/reconciler/proxyAdapter.js @@ -1,7 +1,10 @@ import reactHotLoader from '../reactHotLoader' import { get as getGeneration } from '../global/generation' import { getProxyByType, setStandInOptions } from './proxies' -import reconcileHotReplacement, { flushScheduledUpdates } from './index' +import reconcileHotReplacement, { + flushScheduledUpdates, + unscheduleUpdate, +} from './index' export const RENDERED_GENERATION = 'REACT_HOT_LOADER_RENDERED_GENERATION' @@ -28,14 +31,12 @@ function asyncReconciledRender(target) { renderReconciler(target, false) } -function syncReconciledRender(target) { - if (renderReconciler(target, false)) { - flushScheduledUpdates() +export function proxyWrapper(element) { + // post wrap on post render + if (!reactHotLoader.disableProxyCreation) { + unscheduleUpdate(this) } -} -export const proxyWrapper = element => { - // post wrap on post render if (!element) { return element } @@ -55,7 +56,7 @@ export const proxyWrapper = element => { } setStandInOptions({ - componentWillReceiveProps: syncReconciledRender, componentWillRender: asyncReconciledRender, componentDidRender: proxyWrapper, + componentDidUpdate: flushScheduledUpdates, }) diff --git a/test/AppContainer.dev.test.js b/test/AppContainer.dev.test.js index ecf469ebb..44cf6f9f7 100644 --- a/test/AppContainer.dev.test.js +++ b/test/AppContainer.dev.test.js @@ -37,6 +37,7 @@ describe(`AppContainer (dev)`, () => { it('force updates the tree on receiving new children', () => { const spy = jest.fn() + const spy2 = jest.fn() class App extends Component { shouldComponentUpdate() { @@ -66,6 +67,7 @@ describe(`AppContainer (dev)`, () => { render() { spy() + spy2() return
ho
} } @@ -75,7 +77,9 @@ describe(`AppContainer (dev)`, () => { } expect(spy).toHaveBeenCalledTimes(3) - expect(wrapper.contains(
ho
)).toBe(true) + expect(spy2).toHaveBeenCalledTimes(2) + + expect(wrapper.update().contains(
ho
)).toBe(true) }) it('force updates the tree on receiving cached children', () => { @@ -115,7 +119,7 @@ describe(`AppContainer (dev)`, () => { } expect(spy).toHaveBeenCalledTimes(3) - expect(wrapper.contains(
ho
)).toBe(true) + expect(wrapper.update().contains(
ho
)).toBe(true) }) it('renders latest children on receiving cached never-rendered children', () => { @@ -653,7 +657,7 @@ describe(`AppContainer (dev)`, () => { } expect(spy).toHaveBeenCalledTimes(1 + 2) - expect(wrapper.contains(
ho
)).toBe(true) + expect(wrapper.update().contains(
ho
)).toBe(true) }) it('force updates the tree on receiving cached children', () => { @@ -691,7 +695,7 @@ describe(`AppContainer (dev)`, () => { } expect(spy).toHaveBeenCalledTimes(1 + 2) - expect(wrapper.contains(
ho
)).toBe(true) + expect(wrapper.update().contains(
ho
)).toBe(true) }) it('renders latest children on receiving cached never-rendered children', () => { @@ -830,7 +834,7 @@ describe(`AppContainer (dev)`, () => { } expect(spy).toHaveBeenCalledTimes(1 + 2) - expect(wrapper.contains(
ho
)).toBe(true) + expect(wrapper.update().contains(
ho
)).toBe(true) }) it('force updates the tree on receiving cached children', () => { @@ -869,7 +873,7 @@ describe(`AppContainer (dev)`, () => { } expect(spy).toHaveBeenCalledTimes(1 + 2) - expect(wrapper.contains(
ho
)).toBe(true) + expect(wrapper.update().contains(
ho
)).toBe(true) }) it('renders latest children on receiving cached never-rendered children', () => { diff --git a/test/proxy/consistency.test.js b/test/proxy/consistency.test.js index abc1a71b9..007263425 100644 --- a/test/proxy/consistency.test.js +++ b/test/proxy/consistency.test.js @@ -286,7 +286,7 @@ describe('consistency', () => { 'render', 'hotComponentRender', 'componentDidMount', - 'componentWillReceiveProps', + 'componentDidUpdate', 'componentWillUnmount', ]) }) diff --git a/test/proxy/lifecycle-method.test.js b/test/proxy/lifecycle-method.test.js index 028d06125..5712fb8d3 100644 --- a/test/proxy/lifecycle-method.test.js +++ b/test/proxy/lifecycle-method.test.js @@ -50,7 +50,7 @@ describe('lifecycle method', () => { } render() { - return
{this.superSecret * 2}
+ return
!{this.superSecret * 5}
} } return { App1, App2 } @@ -63,7 +63,7 @@ describe('lifecycle method', () => { return testFabric(methodName)(Component, patchedRender, spy) } - it('handle componentWillMount', () => { + it('handle componentWillMount', done => { const spy = jest.fn() const { App1, App2 } = getTestClass('componentWillMount', spy) @@ -78,7 +78,11 @@ describe('lifecycle method', () => { proxy.update(App2) wrapper.instance().forceUpdate() expect(spy).toHaveBeenCalledTimes(1) - expect(wrapper.text()).toContain('PATCHED + 6') + // first render before hot render + expect(wrapper.text()).toContain('PATCHED + !10') + wrapper.instance().forceUpdate() + expect(wrapper.text()).toContain('PATCHED + !15') + done() }) it('handle componentDidMount', () => { @@ -96,6 +100,9 @@ describe('lifecycle method', () => { proxy.update(App2) wrapper.instance().forceUpdate() expect(spy).toHaveBeenCalledTimes(1) - expect(wrapper.text()).toContain('PATCHED + 6') + // first render before hot render + expect(wrapper.text()).toContain('PATCHED + !10') + wrapper.instance().forceUpdate() + expect(wrapper.text()).toContain('PATCHED + !15') }) }) diff --git a/test/reconciler.test.js b/test/reconciler.test.js index 737db01c0..8e24be80c 100644 --- a/test/reconciler.test.js +++ b/test/reconciler.test.js @@ -146,7 +146,7 @@ describe('reconciler', () => { incrementGeneration() wrapper.setProps({ update: 'now' }) - expect(wrapper.find(.type).length).toBe(1) + expect(wrapper.update().find(.type).length).toBe(1) // first will never be unmounted expect(first.unmounted).toHaveBeenCalledTimes(0) expect(second.unmounted).toHaveBeenCalledTimes(1) @@ -231,7 +231,7 @@ describe('reconciler', () => { incrementGeneration() wrapper.setProps({ second: false }) - expect(First.rendered).toHaveBeenCalledTimes(6) + expect(First.rendered).toHaveBeenCalledTimes(5) expect(Second.rendered).toHaveBeenCalledTimes(3) expect(First.unmounted).toHaveBeenCalledTimes(0) @@ -265,29 +265,29 @@ describe('reconciler', () => { incrementGeneration() wrapper.setProps({ update: 'now' }) - expect(First.rendered).toHaveBeenCalledTimes(4) - expect(Second.rendered).toHaveBeenCalledTimes(4) + expect(First.rendered).toHaveBeenCalledTimes(3) + expect(Second.rendered).toHaveBeenCalledTimes(3) incrementGeneration() wrapper.setProps({ second: false }) - expect(First.rendered).toHaveBeenCalledTimes(7) - expect(Second.rendered).toHaveBeenCalledTimes(4) + expect(First.rendered).toHaveBeenCalledTimes(5) + expect(Second.rendered).toHaveBeenCalledTimes(3) expect(First.unmounted).toHaveBeenCalledTimes(0) expect(Second.unmounted).toHaveBeenCalledTimes(1) }) it('should handle child mounting', () => { - const First = spyComponent(() => 1, 'test', '1') - const Second = spyComponent(() => 2, 'test', '2') - const Third = spyComponent(() => 2, 'test', '2') + const First = spyComponent(() => test1, 'test1', '1') + const Second = spyComponent(() => test2, 'test2', '2') + const Third = spyComponent(() => test3, 'test3', '3') const App = ({ first, second, third }) => (
{first && } {second && [ -
, +
start
, , -
, +
middle
, third && , ]}
@@ -304,13 +304,15 @@ describe('reconciler', () => { incrementGeneration() wrapper.setProps({ second: true }) - expect(First.rendered).toHaveBeenCalledTimes(4) // +3 (reconcile + update + render) - expect(Second.rendered).toHaveBeenCalledTimes(2) // (update from first + render) + expect(First.rendered).toHaveBeenCalledTimes(3) // +3 (reconcile + update + render) + expect(Second.rendered).toHaveBeenCalledTimes(1) // (update from first + render) wrapper.setProps({ third: true }) - expect(First.rendered).toHaveBeenCalledTimes(5) - expect(Second.rendered).toHaveBeenCalledTimes(3) + expect(First.rendered).toHaveBeenCalledTimes(4) + expect(Second.rendered).toHaveBeenCalledTimes(2) expect(Third.rendered).toHaveBeenCalledTimes(1) + + expect(wrapper.update().html()).toMatch(/test3/) }) it('should handle function as a child', () => { diff --git a/test/reconciler/proxyAdapter.test.js b/test/reconciler/proxyAdapter.test.js index 5f9293a1d..3cdfe1ae3 100644 --- a/test/reconciler/proxyAdapter.test.js +++ b/test/reconciler/proxyAdapter.test.js @@ -1,8 +1,11 @@ /* eslint-env browser */ import { proxyWrapper } from '../../src/reconciler/proxyAdapter' import * as proxies from '../../src/reconciler/proxies' +import reactHotLoader from '../../src/reactHotLoader' +import { unscheduleUpdate } from '../../src/reconciler/hotReplacementRender' jest.mock('../../src/reconciler/proxies') +jest.mock('../../src/reconciler/hotReplacementRender') proxies.getProxyByType.mockReturnValue({ get: () => 'proxy' }) @@ -27,4 +30,16 @@ describe(`proxyAdapter`, () => { prop: 42, }) }) + + it('should remove rendered proxy', () => { + const object = {} + unscheduleUpdate.mockClear() + reactHotLoader.disableProxyCreation = 1 + proxyWrapper.call(object) + expect(unscheduleUpdate).not.toHaveBeenCalled() + + reactHotLoader.disableProxyCreation = 0 + proxyWrapper.call(object) + expect(unscheduleUpdate).toHaveBeenCalledWith(object) + }) }) From 85118dcec658fb6fc6f265b624a97004ec2ddaec Mon Sep 17 00:00:00 2001 From: Anton Korzunov Date: Fri, 6 Apr 2018 22:55:39 +1000 Subject: [PATCH 2/5] add react-lifecycles-compat --- package.json | 1 + src/AppContainer.dev.js | 20 +++++++++----------- src/hot.dev.js | 2 +- src/proxy/createClassProxy.js | 1 + 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/package.json b/package.json index 2df2b6306..10807c895 100644 --- a/package.json +++ b/package.json @@ -97,6 +97,7 @@ "global": "^4.3.0", "hoist-non-react-statics": "^2.5.0", "prop-types": "^15.6.1", + "react-lifecycles-compat": "^2.0.0", "shallowequal": "^1.0.2" }, "engines": { diff --git a/src/AppContainer.dev.js b/src/AppContainer.dev.js index b256b62fd..ae96fbb30 100644 --- a/src/AppContainer.dev.js +++ b/src/AppContainer.dev.js @@ -1,20 +1,10 @@ import React from 'react' import PropTypes from 'prop-types' +import { polyfill } from 'react-lifecycles-compat' import logger from './logger' import { get as getGeneration } from './global/generation' -import { renderReconciler } from './reconciler/proxyAdapter' -import { flushScheduledUpdates } from './reconciler' class AppContainer extends React.Component { - constructor(props) { - super(props) - - this.state = { - error: null, - generation: 0, - } - } - static getDerivedStateFromProps(nextProps, prevState) { if (prevState.generation !== getGeneration()) { // Hot reload is happening. @@ -26,6 +16,12 @@ class AppContainer extends React.Component { return null } + state = { + error: null, + // eslint-disable-next-line react/no-unused-state + generation: 0, + } + shouldComponentUpdate(prevProps, prevState) { // Don't update the component if the state had an error and still has one. // This allows to break an infinite loop of error -> render -> error -> render @@ -67,4 +63,6 @@ AppContainer.propTypes = { errorReporter: PropTypes.oneOfType([PropTypes.node, PropTypes.func]), } +polyfill(AppContainer) + export default AppContainer diff --git a/src/hot.dev.js b/src/hot.dev.js index e3aeac947..5e3f72081 100644 --- a/src/hot.dev.js +++ b/src/hot.dev.js @@ -75,7 +75,7 @@ const hot = sourceModule => { return createHoc( WrappedComponent, class ExportedComponent extends Component { - componentWillMount() { + componentDidMount() { module.instances.push(this) } diff --git a/src/proxy/createClassProxy.js b/src/proxy/createClassProxy.js index 87d2a88fd..d4585e4e9 100644 --- a/src/proxy/createClassProxy.js +++ b/src/proxy/createClassProxy.js @@ -28,6 +28,7 @@ export const resetClassProxies = () => { const blackListedClassMembers = [ 'constructor', 'render', + 'componentWillMount', 'componentDidMount', 'componentWillReceiveProps', 'componentWillUnmount', From 4218e413e1436929d0a1a1265b770bfa2da1f4ce Mon Sep 17 00:00:00 2001 From: Anton Korzunov Date: Wed, 18 Apr 2018 11:35:07 +1000 Subject: [PATCH 3/5] fix React 15 Stateless components --- src/internal/stack/hydrateFiberStack.js | 2 +- src/internal/stack/hydrateLegacyStack.js | 2 +- src/proxy/createClassProxy.js | 15 ++++++++++++-- src/proxy/transferStaticProps.js | 1 + src/reconciler/hotReplacementRender.js | 26 +++++++++++++++++++++++- test/internal/getReactStack.test.js | 2 +- 6 files changed, 42 insertions(+), 6 deletions(-) diff --git a/src/internal/stack/hydrateFiberStack.js b/src/internal/stack/hydrateFiberStack.js index 5cfc90e63..12aaa86e3 100644 --- a/src/internal/stack/hydrateFiberStack.js +++ b/src/internal/stack/hydrateFiberStack.js @@ -7,7 +7,7 @@ function pushStack(stack, node) { if (!stack.instance) { stack.instance = { - SFC_fake: true, + SFC_fake: stack.type, props: {}, render: () => stack.type(stack.instance.props), } diff --git a/src/internal/stack/hydrateLegacyStack.js b/src/internal/stack/hydrateLegacyStack.js index 3aebbc671..ccc77e114 100644 --- a/src/internal/stack/hydrateLegacyStack.js +++ b/src/internal/stack/hydrateLegacyStack.js @@ -8,7 +8,7 @@ function pushState(stack, type, instance) { if (typeof type === 'function' && type.isStatelessFunctionalProxy) { // In React 15 SFC is wrapped by component. We have to detect our proxies and change the way it works stack.instance = { - SFC_fake: true, + SFC_fake: type, props: {}, render: () => type(stack.instance.props), } diff --git a/src/proxy/createClassProxy.js b/src/proxy/createClassProxy.js index d4585e4e9..a2bc66fe1 100644 --- a/src/proxy/createClassProxy.js +++ b/src/proxy/createClassProxy.js @@ -57,6 +57,14 @@ const defineClassMembers = (Class, methods) => defineClassMember(Class, methodName, methods[methodName]), ) +const setSFPFlag = (component, flag) => + safeDefineProperty(component, 'isStatelessFunctionalProxy', { + configurable: false, + writable: false, + enumerable: false, + value: flag, + }) + function createClassProxy(InitialComponent, proxyKey, options) { const renderOptions = { ...defaultRenderOptions, @@ -197,10 +205,13 @@ function createClassProxy(InitialComponent, proxyKey, options) { // simple SFC if (!CurrentComponent.contextTypes) { - ProxyFacade.isStatelessFunctionalProxy = true + if (!ProxyFacade.isStatelessFunctionalProxy) { + setSFPFlag(ProxyFacade, true) + } + return renderOptions.componentDidRender(result) } - ProxyFacade.isStatelessFunctionalProxy = false + setSFPFlag(ProxyFacade, false) // This is a Relay-style container constructor. We can't do the prototype- // style wrapping for this as we do elsewhere, so just we just pass it diff --git a/src/proxy/transferStaticProps.js b/src/proxy/transferStaticProps.js index 5650be4be..60fc042fd 100644 --- a/src/proxy/transferStaticProps.js +++ b/src/proxy/transferStaticProps.js @@ -11,6 +11,7 @@ const RESERVED_STATICS = [ 'prototype', 'toString', 'valueOf', + 'isStatelessFunctionalProxy', PROXY_KEY, UNWRAP_PROXY, ] diff --git a/src/reconciler/hotReplacementRender.js b/src/reconciler/hotReplacementRender.js index d39c88fef..a6624e920 100644 --- a/src/reconciler/hotReplacementRender.js +++ b/src/reconciler/hotReplacementRender.js @@ -15,6 +15,13 @@ const UNDEFINED_NAMES = { Component: true, } +let renderStack = [] + +const stackReport = () => { + const rev = renderStack.slice().reverse() + logger.warn('in', rev[0].name, rev) +} + const areNamesEqual = (a, b) => a === b || (UNDEFINED_NAMES[a] && UNDEFINED_NAMES[b]) const isReactClass = fn => fn && !!fn.render @@ -105,7 +112,7 @@ const render = component => { } if (isReactClass(component)) { // not calling real render method to prevent call recursion. - // stateless componets does not have hotComponentRender + // stateless components does not have hotComponentRender return component.hotComponentRender ? component.hotComponentRender() : component.render() @@ -197,6 +204,7 @@ const mergeInject = (a, b, instance) => { 'and children of ', instance, ) + stackReport() } return NO_CHILDREN } @@ -233,6 +241,14 @@ const scheduleInstanceUpdate = instance => { } const hotReplacementRender = (instance, stack) => { + if (isReactClass(instance)) { + const type = getElementType(stack) + renderStack.push({ + name: getComponentDisplayName(type), + type, + props: stack.props, + }) + } const flow = transformFlowNode(filterNullArray(asArray(render(instance)))) const { children } = stack @@ -270,6 +286,7 @@ const hotReplacementRender = (instance, stack) => { 'instead of', stackChild.type, ) + stackReport() } return } @@ -294,6 +311,7 @@ const hotReplacementRender = (instance, stack) => { ' - no instrumentation found. ', 'Please require react-hot-loader before React. More in troubleshooting.', ) + stackReport() throw new Error('React-hot-loader: wrong configuration') } @@ -315,17 +333,23 @@ const hotReplacementRender = (instance, stack) => { )} was expected. ${childType}`, ) + stackReport() } scheduleInstanceUpdate(stackChild.instance) } }) + + if (isReactClass(instance)) { + renderStack.pop() + } } export default (instance, stack) => { try { // disable reconciler to prevent upcoming components from proxying. reactHotLoader.disableProxyCreation = true + renderStack = [] hotReplacementRender(instance, stack) } catch (e) { logger.warn('React-hot-loader: reconcilation failed due to error', e) diff --git a/test/internal/getReactStack.test.js b/test/internal/getReactStack.test.js index 686751369..5e85a08af 100644 --- a/test/internal/getReactStack.test.js +++ b/test/internal/getReactStack.test.js @@ -54,7 +54,7 @@ describe('getReactStack', () => { expect(stack.children).toHaveLength(2) function expectToBeDivStack(child) { expect(child.type).toBe(Div) - expect(child.instance.SFC_fake).toBe(true) + expect(child.instance.SFC_fake).toBe(Div) expect(child.children).toHaveLength(1) expect(child.children[0].type).toBe('div') expect(child.children[0].instance).toBeDefined() From 1581cc5931394fa9ca44430003bd505e5057e2ec Mon Sep 17 00:00:00 2001 From: Anton Korzunov Date: Wed, 18 Apr 2018 13:36:25 +1000 Subject: [PATCH 4/5] fix SFC props assembling --- src/reconciler/hotReplacementRender.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/reconciler/hotReplacementRender.js b/src/reconciler/hotReplacementRender.js index a6624e920..49c87bda1 100644 --- a/src/reconciler/hotReplacementRender.js +++ b/src/reconciler/hotReplacementRender.js @@ -130,7 +130,7 @@ const render = component => { const NO_CHILDREN = { children: [] } const mapChildren = (children, instances) => ({ children: children.filter(c => c).map((child, index) => { - if (typeof child !== 'object') { + if (typeof child !== 'object' || child.isMerged) { return child } const instanceLine = instances[index] || {} @@ -147,10 +147,13 @@ const mapChildren = (children, instances) => ({ (child.props && child.props.children) || child.children || [], ) const nextChildren = - oldChildren.length && mapChildren(newChildren, oldChildren) + child.type !== 'function' && + oldChildren.length && + mapChildren(newChildren, oldChildren) return { nextProps: child.props, + isMerged: true, ...instanceLine, // actually child merge is needed only for "HTML TAG"s, and usually don't work for Components. // the children from an instance or rendered children @@ -246,7 +249,7 @@ const hotReplacementRender = (instance, stack) => { renderStack.push({ name: getComponentDisplayName(type), type, - props: stack.props, + props: stack.instance.props, }) } const flow = transformFlowNode(filterNullArray(asArray(render(instance)))) From 4f17b949dd0880e9f4ec6f294c570b81917094af Mon Sep 17 00:00:00 2001 From: Anton Korzunov Date: Wed, 18 Apr 2018 19:42:16 +1000 Subject: [PATCH 5/5] code coverage for SFC assembling #873 --- test/reconciler.test.js | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/test/reconciler.test.js b/test/reconciler.test.js index 8e24be80c..96c4488a2 100644 --- a/test/reconciler.test.js +++ b/test/reconciler.test.js @@ -361,6 +361,47 @@ describe('reconciler', () => { expect(wrapper.text()).toContain(43) }) + it('should assmeble props for nested children', () => { + const RenderChildren = ({ children }) =>
{children}
+ const RenderProp = ({ prop }) =>
{prop}
+ + const App = () => ( + +
+ +
+
+
+ +
+
+
+
+
+ +
+
+ +
+ + ) + + logger.warn.mockClear() + + const wrapper = mount( + +
+ +
+ , + ) + + incrementGeneration() + wrapper.setProps({ update: 'now' }) + + expect(logger.warn).not.toHaveBeenCalled() + }) + describe('when an error occurs in render', () => { beforeEach(() => { jest.spyOn(console, 'error').mockImplementation(() => {})