From 0113b04d513da4d7cbbd3dfa38ca23f2678147a5 Mon Sep 17 00:00:00 2001
From: GuiCodron <guilhem.codron@gmail.com>
Date: Thu, 13 Oct 2022 13:46:24 -0500
Subject: [PATCH] Bug: Put contentRef in state to let EventStack track changes

contentRef is passed to EventStack to subscribe for mouse events.
Passing a `contentRef` created using `React.createRef()` does not allow
EventStack to keep track of contentRef changes (since only
contentRef.current changes).
Ultimately when PortalInner unmount contentRef.current is set to undefined
and EventStack `unsubscribe` handlers with `undefined` as
a target instead of unsubscribing with real content.
This leads to the event-stack Map keeping a reference to the real contentRef
thus leaking all dom tree related to the node related to contentRef.

Putting contentRef in the state and passing the element allow for
EventStack to detect change in props and execute the componentDidUpdate
routine which unsubscribe from the previous element on unmount.
---
 src/addons/Portal/Portal.js             | 27 +++++++++++++++----------
 test/specs/addons/Portal/Portal-test.js |  4 ++--
 2 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/src/addons/Portal/Portal.js b/src/addons/Portal/Portal.js
index e987e36f95..18c6ed46ff 100644
--- a/src/addons/Portal/Portal.js
+++ b/src/addons/Portal/Portal.js
@@ -24,7 +24,6 @@ const debug = makeDebugger('portal')
  * @see Confirm
  */
 class Portal extends Component {
-  contentRef = React.createRef()
   triggerRef = React.createRef()
   latestDocumentMouseDownEvent = null
 
@@ -44,15 +43,15 @@ class Portal extends Component {
 
   handleDocumentClick = (e) => {
     const { closeOnDocumentClick } = this.props
+    const { contentRef } = this.state
     const currentMouseDownEvent = this.latestDocumentMouseDownEvent
     this.latestDocumentMouseDownEvent = null
 
     if (
-      !this.contentRef.current || // no portal
+      !contentRef || // no portal
       doesNodeContainClick(this.triggerRef.current, e) || // event happened in trigger (delegate to trigger handlers)
-      (currentMouseDownEvent &&
-        doesNodeContainClick(this.contentRef.current, currentMouseDownEvent)) || // event originated in the portal but was ended outside
-      doesNodeContainClick(this.contentRef.current, e) // event happened in the portal
+      (currentMouseDownEvent && doesNodeContainClick(contentRef, currentMouseDownEvent)) || // event originated in the portal but was ended outside
+      doesNodeContainClick(contentRef, e) // event happened in the portal
     ) {
       return
     } // ignore the click
@@ -76,12 +75,13 @@ class Portal extends Component {
   // ----------------------------------------
 
   handlePortalMouseLeave = (e) => {
+    const { contentRef } = this.state
     const { closeOnPortalMouseLeave, mouseLeaveDelay } = this.props
 
     if (!closeOnPortalMouseLeave) return
 
     // Do not close the portal when 'mouseleave' is triggered by children
-    if (e.target !== this.contentRef.current) return
+    if (e.target !== contentRef) return
 
     debug('handlePortalMouseLeave()')
     this.mouseLeaveTimer = this.closeWithTimeout(e, mouseLeaveDelay)
@@ -99,6 +99,7 @@ class Portal extends Component {
   }
 
   handleTriggerBlur = (e, ...rest) => {
+    const { contentRef } = this.state
     const { trigger, closeOnTriggerBlur } = this.props
 
     // Call original event handler
@@ -107,7 +108,7 @@ class Portal extends Component {
     // IE 11 doesn't work with relatedTarget in blur events
     const target = e.relatedTarget || document.activeElement
     // do not close if focus is given to the portal
-    const didFocusPortal = _.invoke(this.contentRef.current, 'contains', target)
+    const didFocusPortal = _.invoke(contentRef, 'contains', target)
 
     if (!closeOnTriggerBlur || didFocusPortal) return
 
@@ -225,9 +226,13 @@ class Portal extends Component {
     handleRef(this.props.triggerRef, c)
   }
 
+  updateContentRef = (ref) => {
+    this.setState({ contentRef: ref })
+  }
+
   render() {
     const { children, eventPool, mountNode, trigger } = this.props
-    const { open } = this.state
+    const { open, contentRef } = this.state
 
     /* istanbul ignore else */
     if (process.env.NODE_ENV !== 'production') {
@@ -239,7 +244,7 @@ class Portal extends Component {
         {open && (
           <>
             <PortalInner
-              innerRef={this.contentRef}
+              innerRef={this.updateContentRef}
               mountNode={mountNode}
               onMount={this.handleMount}
               onUnmount={this.handleUnmount}
@@ -251,13 +256,13 @@ class Portal extends Component {
               name='mouseleave'
               on={this.handlePortalMouseLeave}
               pool={eventPool}
-              target={this.contentRef}
+              target={contentRef}
             />
             <EventStack
               name='mouseenter'
               on={this.handlePortalMouseEnter}
               pool={eventPool}
-              target={this.contentRef}
+              target={contentRef}
             />
             <EventStack name='mousedown' on={this.handleDocumentMouseDown} pool={eventPool} />
             <EventStack name='click' on={this.handleDocumentClick} pool={eventPool} />
diff --git a/test/specs/addons/Portal/Portal-test.js b/test/specs/addons/Portal/Portal-test.js
index 383e8bbcfc..8b0f25f446 100644
--- a/test/specs/addons/Portal/Portal-test.js
+++ b/test/specs/addons/Portal/Portal-test.js
@@ -171,7 +171,7 @@ describe('Portal', () => {
           <p />
         </Portal>,
       )
-      wrapper.instance().contentRef.current.tagName.should.equal('P')
+      wrapper.instance().state.contentRef.tagName.should.equal('P')
     })
 
     it('maintains ref to DOM node with React component', () => {
@@ -182,7 +182,7 @@ describe('Portal', () => {
           <EmptyComponent />
         </Portal>,
       )
-      wrapper.instance().contentRef.current.tagName.should.equal('P')
+      wrapper.instance().state.contentRef.tagName.should.equal('P')
     })
   })