fix(Portal): throw an error if React.Fragment passed as trigger (#3998)

Author: layershifter

src/addons/Portal/Portal.js

@@ -3,14 +3,15 @@ import { handleRef, Ref } from '@stardust-ui/react-component-ref'
 import keyboardKey from 'keyboard-key'
 import _ from 'lodash'
 import PropTypes from 'prop-types'
-import React, { cloneElement, createRef, Fragment } from 'react'
+import React from 'react'
 
 import {
   ModernAutoControlledComponent as Component,
   customPropTypes,
   doesNodeContainClick,
   makeDebugger,
 } from '../../lib'
+import validateTrigger from './utils/validateTrigger'
 import PortalInner from './PortalInner'
 
 const debug = makeDebugger('portal')
@@ -126,8 +127,8 @@ class Portal extends Component {
 
   static Inner = PortalInner
 
-  contentRef = createRef()
-  triggerRef = createRef()
+  contentRef = React.createRef()
+  triggerRef = React.createRef()
   latestDocumentMouseDownEvent = null
 
   componentWillUnmount() {
@@ -335,10 +336,15 @@ class Portal extends Component {
     const { children, eventPool, mountNode, trigger } = this.props
     const { open } = this.state
 
+    /* istanbul ignore else */
+    if (process.env.NODE_ENV !== 'production') {
+      validateTrigger(trigger)
+    }
+
     return (
-      <Fragment>
+      <>
         {open && (
-          <Fragment>
+          <>
             <PortalInner
               innerRef={this.contentRef}
               mountNode={mountNode}
@@ -363,11 +369,11 @@ class Portal extends Component {
             <EventStack name='mousedown' on={this.handleDocumentMouseDown} pool={eventPool} />
             <EventStack name='click' on={this.handleDocumentClick} pool={eventPool} />
             <EventStack name='keydown' on={this.handleEscape} pool={eventPool} />
-          </Fragment>
+          </>
         )}
         {trigger && (
           <Ref innerRef={this.handleTriggerRef}>
-            {cloneElement(trigger, {
+            {React.cloneElement(trigger, {
               onBlur: this.handleTriggerBlur,
               onClick: this.handleTriggerClick,
               onFocus: this.handleTriggerFocus,
@@ -376,7 +382,7 @@ class Portal extends Component {
             })}
           </Ref>
         )}
-      </Fragment>
+      </>
     )
   }
 }

src/addons/Portal/utils/validateTrigger.js

@@ -0,0 +1,15 @@
+import React from 'react'
+import * as ReactIs from 'react-is'
+
+/**
+ * Asserts that a passed element can be used cloned a props will be applied properly.
+ */
+export default function validateTrigger(element) {
+  if (element) {
+    React.Children.only(element)
+
+    if (ReactIs.isFragment(element)) {
+      throw new Error('An "React.Fragment" cannot be used as a `trigger`.')
+    }
+  }
+}

src/modules/Popup/Popup.js

@@ -341,7 +341,9 @@ export default class Popup extends Component {
     } = this.props
     const { closed, portalRestProps } = this.state
 
-    if (closed || disabled) return trigger
+    if (closed || disabled) {
+      return trigger
+    }
 
     const modifiers = _.merge(
       {

test/specs/addons/Portal/Portal-test.js

@@ -1,6 +1,6 @@
 import _ from 'lodash'
 import PropTypes from 'prop-types'
-import React, { Component } from 'react'
+import React from 'react'
 
 import * as common from 'test/specs/commonTests'
 import { domEvent, sandbox } from 'test/utils'
@@ -10,7 +10,7 @@ import PortalInner from 'src/addons/Portal/PortalInner'
 let wrapper
 
 const createHandlingComponent = (eventName) =>
-  class HandlingComponent extends Component {
+  class HandlingComponent extends React.Component {
     static propTypes = {
       handler: PropTypes.func,
     }

test/specs/addons/Portal/utils/validateTrigger-test.js

@@ -0,0 +1,13 @@
+import React from 'react'
+
+import validateTrigger from 'src/addons/Portal/utils/validateTrigger'
+
+describe('validateTrigger', () => {
+  it('throws on multiple elements passed', () => {
+    expect(() => validateTrigger([<button key='trigger1' />, <button key='trigger2' />])).to.throw()
+  })
+
+  it('throws on React.Fragment passed', () => {
+    expect(() => validateTrigger(React.createElement(React.Fragment))).to.throw()
+  })
+})