From 803bdce2ae7b6ab3db306545e38ebb61b12017f6 Mon Sep 17 00:00:00 2001 From: Theodore Han Date: Fri, 17 Apr 2020 14:12:29 -0500 Subject: [PATCH 1/5] Support inner component _debugOwner in memo --- .../src/ReactFiberBeginWork.new.js | 3 +++ .../src/__tests__/ReactMemo-test.js | 16 ++++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js index 338191f23666c..874ba6baa8e3d 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.new.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.new.js @@ -429,6 +429,9 @@ function updateMemoComponent( child.ref = workInProgress.ref; child.return = workInProgress; workInProgress.child = child; + if (__DEV__) { + child._debugOwner = workInProgress; + } return child; } if (__DEV__) { diff --git a/packages/react-reconciler/src/__tests__/ReactMemo-test.js b/packages/react-reconciler/src/__tests__/ReactMemo-test.js index 24788e0e87249..27772d258384d 100644 --- a/packages/react-reconciler/src/__tests__/ReactMemo-test.js +++ b/packages/react-reconciler/src/__tests__/ReactMemo-test.js @@ -497,5 +497,21 @@ describe('memo', () => { expect(root).toMatchRenderedOutput('1'); }); }); + + it('inner component of react memo should have _debugOwner set', async () => { + const InComponent = React.forwardRef((props, ref) =>
); + const Outer = React.memo(InComponent); + const App = () => { + const ref = React.createRef(); + return Click me! ; + }; + ReactNoop.render(); + expect(Scheduler).toFlushWithoutYielding(); + const innerFiber = ReactNoop.getRoot().current.child.child.child; + const innerFiberOwner = innerFiber._debugOwner; + expect(innerFiber.type.$$typeof).toBe(Symbol.for('react.forward_ref')); + expect(innerFiberOwner).not.toBeNull(); + expect(innerFiberOwner.type.$$typeof).toBe(Symbol.for('react.memo')); + }); } }); From 68408a45bdf47eec1576061caa0f3257713807d0 Mon Sep 17 00:00:00 2001 From: Theodore Han Date: Fri, 17 Apr 2020 19:17:08 -0500 Subject: [PATCH 2/5] test with devtool context --- .../ownersListContext-test.js.snap | 23 +++++++++++ .../src/__tests__/ownersListContext-test.js | 38 +++++++++++++++++++ .../src/ReactFiberBeginWork.old.js | 3 ++ 3 files changed, 64 insertions(+) diff --git a/packages/react-devtools-shared/src/__tests__/__snapshots__/ownersListContext-test.js.snap b/packages/react-devtools-shared/src/__tests__/__snapshots__/ownersListContext-test.js.snap index 0699eb5b18dc5..7ef006fda2423 100644 --- a/packages/react-devtools-shared/src/__tests__/__snapshots__/ownersListContext-test.js.snap +++ b/packages/react-devtools-shared/src/__tests__/__snapshots__/ownersListContext-test.js.snap @@ -78,6 +78,29 @@ Array [ ] `; +exports[`OwnersListContext should include all owners for a component wrapped in react memo : owners for "Anonymous" 1`] = ` +Array [ + Object { + "displayName": "Grandparent", + "hocDisplayNames": null, + "id": 2, + "type": 5, + }, + Object { + "displayName": "Anonymous", + "hocDisplayNames": null, + "id": 3, + "type": 8, + }, + Object { + "displayName": "Anonymous", + "hocDisplayNames": null, + "id": 4, + "type": 6, + }, +] +`; + exports[`OwnersListContext should include the current element even if there are no other owners: mount 1`] = ` [root] diff --git a/packages/react-devtools-shared/src/__tests__/ownersListContext-test.js b/packages/react-devtools-shared/src/__tests__/ownersListContext-test.js index b39da70632065..b8b611ff35be6 100644 --- a/packages/react-devtools-shared/src/__tests__/ownersListContext-test.js +++ b/packages/react-devtools-shared/src/__tests__/ownersListContext-test.js @@ -206,4 +206,42 @@ describe('OwnersListContext', () => { done(); }); + + it('should include all owners for a component wrapped in react memo ', async done => { + const InnerComponent = React.forwardRef((props, ref) =>
); + const Memo = React.memo(InnerComponent); + const Grandparent = () => { + const ref = React.createRef(); + return ; + }; + + utils.act(() => + ReactDOM.render(, document.createElement('div')), + ); + + let didFinish = false; + function Suspender({owner}) { + const read = React.useContext(OwnersListContext); + const owners = read(owner.id); + didFinish = true; + expect(owners.length).toBe(3); + expect(owners).toMatchSnapshot( + `owners for "${(owner && owner.displayName) || ''}"`, + ); + return null; + } + + const wrapped = ((store.getElementAtIndex(2): any): Element); + await utils.actAsync(() => + TestRenderer.create( + + + + + , + ), + ); + expect(didFinish).toBe(true); + done(); + }); }); diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.old.js b/packages/react-reconciler/src/ReactFiberBeginWork.old.js index 4fde99d80c715..e10b4a4fe476e 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.old.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.old.js @@ -429,6 +429,9 @@ function updateMemoComponent( child.ref = workInProgress.ref; child.return = workInProgress; workInProgress.child = child; + if (__DEV__) { + child._debugOwner = workInProgress; + } return child; } if (__DEV__) { From 51a00f0628962bf1b98c5add3ba7640389cdc196 Mon Sep 17 00:00:00 2001 From: Theodore Han Date: Fri, 17 Apr 2020 19:18:37 -0500 Subject: [PATCH 3/5] remove memo test --- .../src/__tests__/ReactMemo-test.js | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/packages/react-reconciler/src/__tests__/ReactMemo-test.js b/packages/react-reconciler/src/__tests__/ReactMemo-test.js index 27772d258384d..24788e0e87249 100644 --- a/packages/react-reconciler/src/__tests__/ReactMemo-test.js +++ b/packages/react-reconciler/src/__tests__/ReactMemo-test.js @@ -497,21 +497,5 @@ describe('memo', () => { expect(root).toMatchRenderedOutput('1'); }); }); - - it('inner component of react memo should have _debugOwner set', async () => { - const InComponent = React.forwardRef((props, ref) =>
); - const Outer = React.memo(InComponent); - const App = () => { - const ref = React.createRef(); - return Click me! ; - }; - ReactNoop.render(); - expect(Scheduler).toFlushWithoutYielding(); - const innerFiber = ReactNoop.getRoot().current.child.child.child; - const innerFiberOwner = innerFiber._debugOwner; - expect(innerFiber.type.$$typeof).toBe(Symbol.for('react.forward_ref')); - expect(innerFiberOwner).not.toBeNull(); - expect(innerFiberOwner.type.$$typeof).toBe(Symbol.for('react.memo')); - }); } }); From fac6a69b9a74b075cd74e294941b0bc640bd351a Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Fri, 7 Aug 2020 15:19:56 -0400 Subject: [PATCH 4/5] Merged master; tweaked test and snapshot --- .../__snapshots__/ownersListContext-test.js.snap | 14 +++++++++----- .../src/__tests__/ownersListContext-test.js | 7 ++++--- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/packages/react-devtools-shared/src/__tests__/__snapshots__/ownersListContext-test.js.snap b/packages/react-devtools-shared/src/__tests__/__snapshots__/ownersListContext-test.js.snap index 7ef006fda2423..25e56daf6f44e 100644 --- a/packages/react-devtools-shared/src/__tests__/__snapshots__/ownersListContext-test.js.snap +++ b/packages/react-devtools-shared/src/__tests__/__snapshots__/ownersListContext-test.js.snap @@ -78,7 +78,7 @@ Array [ ] `; -exports[`OwnersListContext should include all owners for a component wrapped in react memo : owners for "Anonymous" 1`] = ` +exports[`OwnersListContext should include all owners for a component wrapped in react memo: owners for "InnerComponent" 1`] = ` Array [ Object { "displayName": "Grandparent", @@ -87,14 +87,18 @@ Array [ "type": 5, }, Object { - "displayName": "Anonymous", - "hocDisplayNames": null, + "displayName": "InnerComponent", + "hocDisplayNames": Array [ + "Memo", + ], "id": 3, "type": 8, }, Object { - "displayName": "Anonymous", - "hocDisplayNames": null, + "displayName": "InnerComponent", + "hocDisplayNames": Array [ + "ForwardRef", + ], "id": 4, "type": 6, }, diff --git a/packages/react-devtools-shared/src/__tests__/ownersListContext-test.js b/packages/react-devtools-shared/src/__tests__/ownersListContext-test.js index b8b611ff35be6..4e60bd6a55025 100644 --- a/packages/react-devtools-shared/src/__tests__/ownersListContext-test.js +++ b/packages/react-devtools-shared/src/__tests__/ownersListContext-test.js @@ -207,9 +207,10 @@ describe('OwnersListContext', () => { done(); }); - it('should include all owners for a component wrapped in react memo ', async done => { - const InnerComponent = React.forwardRef((props, ref) =>
); - const Memo = React.memo(InnerComponent); + it('should include all owners for a component wrapped in react memo', async done => { + const InnerComponent = (props, ref) =>
; + const ForwardRef = React.forwardRef(InnerComponent); + const Memo = React.memo(ForwardRef); const Grandparent = () => { const ref = React.createRef(); return ; From 8174d54710e68a662d7fe9e3ebf82f2c48b1efd2 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Fri, 7 Aug 2020 15:33:54 -0400 Subject: [PATCH 5/5] Pass owner to createFiber fn when creating a memo component. --- packages/react-reconciler/src/ReactFiber.new.js | 4 ++++ packages/react-reconciler/src/ReactFiber.old.js | 4 ++++ packages/react-reconciler/src/ReactFiberBeginWork.new.js | 5 +---- packages/react-reconciler/src/ReactFiberBeginWork.old.js | 5 +---- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiber.new.js b/packages/react-reconciler/src/ReactFiber.new.js index 586825a336209..6a90a307c222c 100644 --- a/packages/react-reconciler/src/ReactFiber.new.js +++ b/packages/react-reconciler/src/ReactFiber.new.js @@ -579,6 +579,10 @@ export function createFiberFromTypeAndProps( fiber.type = resolvedType; fiber.lanes = lanes; + if (__DEV__) { + fiber._debugOwner = owner; + } + return fiber; } diff --git a/packages/react-reconciler/src/ReactFiber.old.js b/packages/react-reconciler/src/ReactFiber.old.js index 818f001fa3376..3d896efe8e851 100644 --- a/packages/react-reconciler/src/ReactFiber.old.js +++ b/packages/react-reconciler/src/ReactFiber.old.js @@ -573,6 +573,10 @@ export function createFiberFromTypeAndProps( fiber.type = resolvedType; fiber.lanes = lanes; + if (__DEV__) { + fiber._debugOwner = owner; + } + return fiber; } diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js index d85e7bda22d30..5dbb49e3a7c02 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.new.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.new.js @@ -434,16 +434,13 @@ function updateMemoComponent( Component.type, null, nextProps, - null, + workInProgress, workInProgress.mode, renderLanes, ); child.ref = workInProgress.ref; child.return = workInProgress; workInProgress.child = child; - if (__DEV__) { - child._debugOwner = workInProgress; - } return child; } if (__DEV__) { diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.old.js b/packages/react-reconciler/src/ReactFiberBeginWork.old.js index 802f15d4c82d3..163aedb4d66bf 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.old.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.old.js @@ -434,16 +434,13 @@ function updateMemoComponent( Component.type, null, nextProps, - null, + workInProgress, workInProgress.mode, renderLanes, ); child.ref = workInProgress.ref; child.return = workInProgress; workInProgress.child = child; - if (__DEV__) { - child._debugOwner = workInProgress; - } return child; } if (__DEV__) {