From 50f22627ef91772456b36f52786c7cf2f1774db5 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 15 Mar 2019 16:03:45 +0000 Subject: [PATCH 1/5] Throw away old shallow renderer state on type change This worked in function components but was broken for classes. It incorrectly retained the old instance even if the type was different. --- .../src/ReactShallowRenderer.js | 7 ++++ .../__tests__/ReactShallowRenderer-test.js | 42 +++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/packages/react-test-renderer/src/ReactShallowRenderer.js b/packages/react-test-renderer/src/ReactShallowRenderer.js index 95081009a0ea5..5fae5e4cba464 100644 --- a/packages/react-test-renderer/src/ReactShallowRenderer.js +++ b/packages/react-test-renderer/src/ReactShallowRenderer.js @@ -178,6 +178,10 @@ class ReactShallowRenderer { }; constructor() { + this._reset(); + } + + _reset() { this._context = null; this._element = null; this._instance = null; @@ -514,6 +518,9 @@ class ReactShallowRenderer { if (this._rendering) { return; } + if (this._element != null && this._element.type !== element.type) { + this._reset(); + } const elementType = isMemo(element.type) ? element.type.type : element.type; const previousElement = this._element; diff --git a/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js b/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js index df7a08d9d5c0d..4332a60a028f7 100644 --- a/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js +++ b/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js @@ -1565,4 +1565,46 @@ describe('ReactShallowRenderer', () => { 'forwardRef requires a render function but was given object.', ); }); + + it('should let you change type', () => { + function Foo({prop}) { + return
Foo {prop}
; + } + function Bar({prop}) { + return
Bar {prop}
; + } + + const shallowRenderer = createRenderer(); + shallowRenderer.render(); + expect(shallowRenderer.getRenderOutput()).toEqual(
Foo {'foo1'}
); + shallowRenderer.render(); + expect(shallowRenderer.getRenderOutput()).toEqual(
Foo {'foo2'}
); + shallowRenderer.render(); + expect(shallowRenderer.getRenderOutput()).toEqual(
Bar {'bar1'}
); + shallowRenderer.render(); + expect(shallowRenderer.getRenderOutput()).toEqual(
Bar {'bar2'}
); + }); + + it('should let you change class type', () => { + class Foo extends React.Component { + render() { + return
Foo {this.props.prop}
; + } + } + class Bar extends React.Component { + render() { + return
Bar {this.props.prop}
; + } + } + + const shallowRenderer = createRenderer(); + shallowRenderer.render(); + expect(shallowRenderer.getRenderOutput()).toEqual(
Foo {'foo1'}
); + shallowRenderer.render(); + expect(shallowRenderer.getRenderOutput()).toEqual(
Foo {'foo2'}
); + shallowRenderer.render(); + expect(shallowRenderer.getRenderOutput()).toEqual(
Bar {'bar1'}
); + shallowRenderer.render(); + expect(shallowRenderer.getRenderOutput()).toEqual(
Bar {'bar2'}
); + }); }); From 57d2911e49e471db9e827c39f8dee3a2c37b32bb Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 15 Mar 2019 16:08:17 +0000 Subject: [PATCH 2/5] Remove _previousComponentIdentity We only needed this because we didn't correctly reset based on type. Now we do so this can go away. --- .../react-test-renderer/src/ReactShallowRenderer.js | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/packages/react-test-renderer/src/ReactShallowRenderer.js b/packages/react-test-renderer/src/ReactShallowRenderer.js index 5fae5e4cba464..d321ca978bfb6 100644 --- a/packages/react-test-renderer/src/ReactShallowRenderer.js +++ b/packages/react-test-renderer/src/ReactShallowRenderer.js @@ -198,7 +198,6 @@ class ReactShallowRenderer { this._renderPhaseUpdates = null; this._currentlyRenderingComponent = null; this._numberOfReRenders = 0; - this._previousComponentIdentity = null; } _context: null | Object; @@ -213,7 +212,6 @@ class ReactShallowRenderer { _workInProgressHook: null | Hook; _firstWorkInProgressHook: null | Hook; _currentlyRenderingComponent: null | Object; - _previousComponentIdentity: null | Object; _renderPhaseUpdates: Map, Update> | null; _isReRender: boolean; _didScheduleRenderPhaseUpdate: boolean; @@ -446,14 +444,7 @@ class ReactShallowRenderer { } _prepareToUseHooks(componentIdentity: Object): void { - if ( - this._previousComponentIdentity !== null && - this._previousComponentIdentity !== componentIdentity - ) { - this._firstWorkInProgressHook = null; - } this._currentlyRenderingComponent = componentIdentity; - this._previousComponentIdentity = componentIdentity; } _finishHooks(element: ReactElement, context: null | Object) { @@ -635,7 +626,6 @@ class ReactShallowRenderer { } this._firstWorkInProgressHook = null; - this._previousComponentIdentity = null; this._context = null; this._element = null; this._newState = null; From a052618dc61a4984f2e8195ff76b14c832c86ba4 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 15 Mar 2019 18:43:25 +0000 Subject: [PATCH 3/5] Use _reset when unmounting --- packages/react-test-renderer/src/ReactShallowRenderer.js | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/packages/react-test-renderer/src/ReactShallowRenderer.js b/packages/react-test-renderer/src/ReactShallowRenderer.js index d321ca978bfb6..4b62a4be1ae81 100644 --- a/packages/react-test-renderer/src/ReactShallowRenderer.js +++ b/packages/react-test-renderer/src/ReactShallowRenderer.js @@ -624,13 +624,7 @@ class ReactShallowRenderer { this._instance.componentWillUnmount(); } } - - this._firstWorkInProgressHook = null; - this._context = null; - this._element = null; - this._newState = null; - this._rendered = null; - this._instance = null; + this._reset(); } _mountClassComponent( From 8aee192e27f37ab2080dc2ba0a177e1ed851cd69 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 15 Mar 2019 19:40:42 +0000 Subject: [PATCH 4/5] Use arbitrary componentIdentity There was no particular reason it was set to element.type. We just wanted to check if something is a render phase update. --- packages/react-test-renderer/src/ReactShallowRenderer.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react-test-renderer/src/ReactShallowRenderer.js b/packages/react-test-renderer/src/ReactShallowRenderer.js index 4b62a4be1ae81..9f2db658ffaad 100644 --- a/packages/react-test-renderer/src/ReactShallowRenderer.js +++ b/packages/react-test-renderer/src/ReactShallowRenderer.js @@ -574,7 +574,6 @@ class ReactShallowRenderer { let shouldRender = true; if ( isMemo(element.type) && - elementType === this._previousComponentIdentity && previousElement !== null ) { // This is a Memo component that is being re-rendered. @@ -586,7 +585,8 @@ class ReactShallowRenderer { if (shouldRender) { const prevDispatcher = ReactCurrentDispatcher.current; ReactCurrentDispatcher.current = this._dispatcher; - this._prepareToUseHooks(elementType); + const componentIdentity = {}; + this._prepareToUseHooks(componentIdentity); try { // elementType could still be a ForwardRef if it was // nested inside Memo. From 4972adbc71bfc6c8bbe05c3b6da1a09fda4043b5 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 15 Mar 2019 20:12:45 +0000 Subject: [PATCH 5/5] Support Hook state updates in shallow renderer --- .../src/ReactShallowRenderer.js | 110 ++++++------ .../ReactShallowRendererHooks-test.js | 166 ++++++++++++++++++ 2 files changed, 224 insertions(+), 52 deletions(-) diff --git a/packages/react-test-renderer/src/ReactShallowRenderer.js b/packages/react-test-renderer/src/ReactShallowRenderer.js index 9f2db658ffaad..120fe54383ea0 100644 --- a/packages/react-test-renderer/src/ReactShallowRenderer.js +++ b/packages/react-test-renderer/src/ReactShallowRenderer.js @@ -31,7 +31,7 @@ type Update = { }; type UpdateQueue = { - last: Update | null, + first: Update | null, dispatch: any, }; @@ -196,7 +196,6 @@ class ReactShallowRenderer { this._isReRender = false; this._didScheduleRenderPhaseUpdate = false; this._renderPhaseUpdates = null; - this._currentlyRenderingComponent = null; this._numberOfReRenders = 0; } @@ -211,7 +210,6 @@ class ReactShallowRenderer { _dispatcher: DispatcherType; _workInProgressHook: null | Hook; _firstWorkInProgressHook: null | Hook; - _currentlyRenderingComponent: null | Object; _renderPhaseUpdates: Map, Update> | null; _isReRender: boolean; _didScheduleRenderPhaseUpdate: boolean; @@ -219,7 +217,7 @@ class ReactShallowRenderer { _validateCurrentlyRenderingComponent() { invariant( - this._currentlyRenderingComponent !== null, + this._rendering && !this._instance, 'Hooks can only be called inside the body of a function component. ' + '(https://fb.me/react-invalid-hook-call)', ); @@ -234,33 +232,44 @@ class ReactShallowRenderer { this._validateCurrentlyRenderingComponent(); this._createWorkInProgressHook(); const workInProgressHook: Hook = (this._workInProgressHook: any); + if (this._isReRender) { - // This is a re-render. Apply the new render phase updates to the previous - // current hook. + // This is a re-render. const queue: UpdateQueue = (workInProgressHook.queue: any); const dispatch: Dispatch = (queue.dispatch: any); - if (this._renderPhaseUpdates !== null) { - // Render phase updates are stored in a map of queue -> linked list - const firstRenderPhaseUpdate = this._renderPhaseUpdates.get(queue); - if (firstRenderPhaseUpdate !== undefined) { - (this._renderPhaseUpdates: any).delete(queue); - let newState = workInProgressHook.memoizedState; - let update = firstRenderPhaseUpdate; - do { - // Process this render phase update. We don't have to check the - // priority because it will always be the same as the current - // render's. - const action = update.action; - newState = reducer(newState, action); - update = update.next; - } while (update !== null); - - workInProgressHook.memoizedState = newState; - - return [newState, dispatch]; + if (this._numberOfReRenders > 0) { + // Apply the new render phase updates to the previous current hook. + if (this._renderPhaseUpdates !== null) { + // Render phase updates are stored in a map of queue -> linked list + const firstRenderPhaseUpdate = this._renderPhaseUpdates.get(queue); + if (firstRenderPhaseUpdate !== undefined) { + (this._renderPhaseUpdates: any).delete(queue); + let newState = workInProgressHook.memoizedState; + let update = firstRenderPhaseUpdate; + do { + const action = update.action; + newState = reducer(newState, action); + update = update.next; + } while (update !== null); + workInProgressHook.memoizedState = newState; + return [newState, dispatch]; + } } + return [workInProgressHook.memoizedState, dispatch]; } - return [workInProgressHook.memoizedState, dispatch]; + // Process updates outside of render + let newState = workInProgressHook.memoizedState; + let update = queue.first; + if (update !== null) { + do { + const action = update.action; + newState = reducer(newState, action); + update = update.next; + } while (update !== null); + queue.first = null; + workInProgressHook.memoizedState = newState; + } + return [newState, dispatch]; } else { let initialState; if (reducer === basicStateReducer) { @@ -275,16 +284,12 @@ class ReactShallowRenderer { } workInProgressHook.memoizedState = initialState; const queue: UpdateQueue = (workInProgressHook.queue = { - last: null, + first: null, dispatch: null, }); const dispatch: Dispatch< A, - > = (queue.dispatch = (this._dispatchAction.bind( - this, - (this._currentlyRenderingComponent: any), - queue, - ): any)); + > = (queue.dispatch = (this._dispatchAction.bind(this, queue): any)); return [workInProgressHook.memoizedState, dispatch]; } }; @@ -375,18 +380,14 @@ class ReactShallowRenderer { }; } - _dispatchAction( - componentIdentity: Object, - queue: UpdateQueue, - action: A, - ) { + _dispatchAction(queue: UpdateQueue, action: A) { invariant( this._numberOfReRenders < RE_RENDER_LIMIT, 'Too many re-renders. React limits the number of renders to prevent ' + 'an infinite loop.', ); - if (componentIdentity === this._currentlyRenderingComponent) { + if (this._rendering) { // This is a render phase update. Stash it in a lazily-created map of // queue -> linked list of updates. After this render pass, we'll restart // and apply the stashed updates on top of the work-in-progress hook. @@ -411,9 +412,24 @@ class ReactShallowRenderer { lastRenderPhaseUpdate.next = update; } } else { - // This means an update has happened after the function component has - // returned. On the server this is a no-op. In React Fiber, the update - // would be scheduled for a future render. + const update: Update = { + action, + next: null, + }; + + // Append the update to the end of the list. + let last = queue.first; + if (last === null) { + queue.first = update; + } else { + while (last.next !== null) { + last = last.next; + } + last.next = update; + } + + // Re-render now. + this.render(this._element, this._context); } } @@ -443,10 +459,6 @@ class ReactShallowRenderer { return this._workInProgressHook; } - _prepareToUseHooks(componentIdentity: Object): void { - this._currentlyRenderingComponent = componentIdentity; - } - _finishHooks(element: ReactElement, context: null | Object) { if (this._didScheduleRenderPhaseUpdate) { // Updates were scheduled during the render phase. They are stored in @@ -461,7 +473,6 @@ class ReactShallowRenderer { this._rendering = false; this.render(element, context); } else { - this._currentlyRenderingComponent = null; this._workInProgressHook = null; this._renderPhaseUpdates = null; this._numberOfReRenders = 0; @@ -572,10 +583,7 @@ class ReactShallowRenderer { this._mountClassComponent(elementType, element, this._context); } else { let shouldRender = true; - if ( - isMemo(element.type) && - previousElement !== null - ) { + if (isMemo(element.type) && previousElement !== null) { // This is a Memo component that is being re-rendered. const compare = element.type.compare || shallowEqual; if (compare(previousElement.props, element.props)) { @@ -585,8 +593,6 @@ class ReactShallowRenderer { if (shouldRender) { const prevDispatcher = ReactCurrentDispatcher.current; ReactCurrentDispatcher.current = this._dispatcher; - const componentIdentity = {}; - this._prepareToUseHooks(componentIdentity); try { // elementType could still be a ForwardRef if it was // nested inside Memo. diff --git a/packages/react-test-renderer/src/__tests__/ReactShallowRendererHooks-test.js b/packages/react-test-renderer/src/__tests__/ReactShallowRendererHooks-test.js index 29fa94433db2c..6b8a962ad5b14 100644 --- a/packages/react-test-renderer/src/__tests__/ReactShallowRendererHooks-test.js +++ b/packages/react-test-renderer/src/__tests__/ReactShallowRendererHooks-test.js @@ -90,6 +90,61 @@ describe('ReactShallowRenderer with hooks', () => { ); }); + it('should work with updating a derived value from useState', () => { + let _updateName; + + function SomeComponent({defaultName}) { + const [name, updateName] = React.useState(defaultName); + const [prevName, updatePrevName] = React.useState(defaultName); + const [letter, updateLetter] = React.useState(name[0]); + + _updateName = updateName; + + if (name !== prevName) { + updatePrevName(name); + updateLetter(name[0]); + } + + return ( +
+

+ Your name is: {name + ' (' + letter + ')'} +

+
+ ); + } + + const shallowRenderer = createRenderer(); + let result = shallowRenderer.render( + , + ); + expect(result).toEqual( +
+

+ Your name is: Sophie (S) +

+
, + ); + + result = shallowRenderer.render(); + expect(result).toEqual( +
+

+ Your name is: Sophie (S) +

+
, + ); + + _updateName('Dan'); + expect(shallowRenderer.getRenderOutput()).toEqual( +
+

+ Your name is: Dan (D) +

+
, + ); + }); + it('should work with useReducer', () => { function reducer(state, action) { switch (action.type) { @@ -322,4 +377,115 @@ describe('ReactShallowRenderer with hooks', () => { expect(firstResult).toEqual(secondResult); }); + + it('should update a value from useState outside the render', () => { + let _dispatch; + + function SomeComponent({defaultName}) { + const [count, dispatch] = React.useReducer( + (s, a) => (a === 'inc' ? s + 1 : s), + 0, + ); + const [name, updateName] = React.useState(defaultName); + _dispatch = () => dispatch('inc'); + + return ( +
updateName('Dan')}> +

+ Your name is: {name} ({count}) +

+
+ ); + } + + const shallowRenderer = createRenderer(); + const element = ; + const result = shallowRenderer.render(element); + expect(result.props.children).toEqual( +

+ Your name is: Dominic ({0}) +

, + ); + + result.props.onClick(); + let updated = shallowRenderer.render(element); + expect(updated.props.children).toEqual( +

+ Your name is: Dan ({0}) +

, + ); + + _dispatch('foo'); + updated = shallowRenderer.render(element); + expect(updated.props.children).toEqual( +

+ Your name is: Dan ({1}) +

, + ); + + _dispatch('inc'); + updated = shallowRenderer.render(element); + expect(updated.props.children).toEqual( +

+ Your name is: Dan ({2}) +

, + ); + }); + + it('should ignore a foreign update outside the render', () => { + let _updateCountForFirstRender; + + function SomeComponent() { + const [count, updateCount] = React.useState(0); + if (!_updateCountForFirstRender) { + _updateCountForFirstRender = updateCount; + } + return count; + } + + const shallowRenderer = createRenderer(); + const element = ; + let result = shallowRenderer.render(element); + expect(result).toEqual(0); + _updateCountForFirstRender(1); + result = shallowRenderer.render(element); + expect(result).toEqual(1); + + shallowRenderer.unmount(); + result = shallowRenderer.render(element); + expect(result).toEqual(0); + _updateCountForFirstRender(1); // Should be ignored. + result = shallowRenderer.render(element); + expect(result).toEqual(0); + }); + + it('should not forget render phase updates', () => { + let _updateCount; + + function SomeComponent() { + const [count, updateCount] = React.useState(0); + _updateCount = updateCount; + if (count < 5) { + updateCount(x => x + 1); + } + return count; + } + + const shallowRenderer = createRenderer(); + const element = ; + let result = shallowRenderer.render(element); + expect(result).toEqual(5); + + _updateCount(10); + result = shallowRenderer.render(element); + expect(result).toEqual(10); + + _updateCount(x => x + 1); + result = shallowRenderer.render(element); + expect(result).toEqual(11); + + _updateCount(x => x - 10); + result = shallowRenderer.render(element); + expect(result).toEqual(5); + }); });