chore(Step): use React.forwardRef() (#4240)

Author: layershifter

src/elements/Step/Step.js

@@ -1,7 +1,7 @@
 import cx from 'clsx'
 import _ from 'lodash'
 import PropTypes from 'prop-types'
-import React, { Component } from 'react'
+import React from 'react'
 
 import {
   childrenUtils,
@@ -10,6 +10,7 @@ import {
   getElementType,
   getUnhandledProps,
   useKeyOnly,
+  useEventCallback,
 } from '../../lib'
 import Icon from '../Icon'
 import StepContent from './StepContent'
@@ -20,70 +21,69 @@ import StepTitle from './StepTitle'
 /**
  * A step shows the completion status of an activity in a series of activities.
  */
-class Step extends Component {
-  computeElementType = () => {
-    const { onClick } = this.props
-
-    if (onClick) return 'a'
-  }
-
-  handleClick = (e) => {
-    const { disabled } = this.props
-
-    if (!disabled) _.invoke(this.props, 'onClick', e, this.props)
-  }
-
-  render() {
-    const {
-      active,
-      children,
-      className,
-      completed,
-      content,
-      description,
-      disabled,
-      href,
-      icon,
-      link,
-      title,
-    } = this.props
-
-    const classes = cx(
-      useKeyOnly(active, 'active'),
-      useKeyOnly(completed, 'completed'),
-      useKeyOnly(disabled, 'disabled'),
-      useKeyOnly(link, 'link'),
-      'step',
-      className,
-    )
-    const rest = getUnhandledProps(Step, this.props)
-    const ElementType = getElementType(Step, this.props, this.computeElementType)
-
-    if (!childrenUtils.isNil(children)) {
-      return (
-        <ElementType {...rest} className={classes} href={href} onClick={this.handleClick}>
-          {children}
-        </ElementType>
-      )
+const Step = React.forwardRef(function StepInner(props, ref) {
+  const {
+    active,
+    children,
+    className,
+    completed,
+    content,
+    description,
+    disabled,
+    href,
+    onClick,
+    icon,
+    link,
+    title,
+  } = props
+
+  const handleClick = useEventCallback((e) => {
+    if (!disabled) {
+      _.invoke(props, 'onClick', e, props)
     }
-
-    if (!childrenUtils.isNil(content)) {
-      return (
-        <ElementType {...rest} className={classes} href={href} onClick={this.handleClick}>
-          {content}
-        </ElementType>
-      )
+  })
+
+  const classes = cx(
+    useKeyOnly(active, 'active'),
+    useKeyOnly(completed, 'completed'),
+    useKeyOnly(disabled, 'disabled'),
+    useKeyOnly(link, 'link'),
+    'step',
+    className,
+  )
+
+  const rest = getUnhandledProps(Step, props)
+  const ElementType = getElementType(Step, props, () => {
+    if (onClick) {
+      return 'a'
     }
+  })
 
+  if (!childrenUtils.isNil(children)) {
     return (
-      <ElementType {...rest} className={classes} href={href} onClick={this.handleClick}>
-        {Icon.create(icon, { autoGenerateKey: false })}
-        {StepContent.create({ description, title }, { autoGenerateKey: false })}
+      <ElementType {...rest} className={classes} href={href} onClick={handleClick} ref={ref}>
+        {children}
       </ElementType>
     )
   }
-}
 
+  if (!childrenUtils.isNil(content)) {
+    return (
+      <ElementType {...rest} className={classes} href={href} onClick={handleClick} ref={ref}>
+        {content}
+      </ElementType>
+    )
+  }
+
+  return (
+    <ElementType {...rest} className={classes} href={href} onClick={handleClick} ref={ref}>
+      {Icon.create(icon, { autoGenerateKey: false })}
+      {StepContent.create({ description, title }, { autoGenerateKey: false })}
+    </ElementType>
+  )
+})
+
+Step.displayName = 'Step'
 Step.propTypes = {
   /** An element type to render as (string or function). */
   as: PropTypes.elementType,

src/elements/Step/StepContent.js

@@ -15,35 +15,37 @@ import StepTitle from './StepTitle'
 /**
  * A step can contain a content.
  */
-function StepContent(props) {
+const StepContent = React.forwardRef(function StepContentInner(props, ref) {
   const { children, className, content, description, title } = props
   const classes = cx('content', className)
   const rest = getUnhandledProps(StepContent, props)
   const ElementType = getElementType(StepContent, props)
 
   if (!childrenUtils.isNil(children)) {
     return (
-      <ElementType {...rest} className={classes}>
+      <ElementType {...rest} className={classes} ref={ref}>
         {children}
       </ElementType>
     )
   }
+
   if (!childrenUtils.isNil(content)) {
     return (
-      <ElementType {...rest} className={classes}>
+      <ElementType {...rest} className={classes} ref={ref}>
         {content}
       </ElementType>
     )
   }
 
   return (
-    <ElementType {...rest} className={classes}>
+    <ElementType {...rest} className={classes} ref={ref}>
       {StepTitle.create(title, { autoGenerateKey: false })}
       {StepDescription.create(description, { autoGenerateKey: false })}
     </ElementType>
   )
-}
+})
 
+StepContent.displayName = 'StepContent'
 StepContent.propTypes = {
   /** An element type to render as (string or function). */
   as: PropTypes.elementType,

src/elements/Step/StepDescription.js

@@ -10,19 +10,20 @@ import {
   getUnhandledProps,
 } from '../../lib'
 
-function StepDescription(props) {
+const StepDescription = React.forwardRef(function StepDescriptionInner(props, ref) {
   const { children, className, content } = props
   const classes = cx('description', className)
   const rest = getUnhandledProps(StepDescription, props)
   const ElementType = getElementType(StepDescription, props)
 
   return (
-    <ElementType {...rest} className={classes}>
+    <ElementType {...rest} className={classes} ref={ref}>
       {childrenUtils.isNil(children) ? content : children}
     </ElementType>
   )
-}
+})
 
+StepDescription.displayName = 'StepDescription'
 StepDescription.propTypes = {
   /** An element type to render as (string or function). */
   as: PropTypes.elementType,

src/elements/Step/StepGroup.js

@@ -22,7 +22,7 @@ const numberMap = _.pickBy(numberToWordMap, (val, key) => key <= 8)
 /**
  * A set of steps.
  */
-function StepGroup(props) {
+const StepGroup = React.forwardRef(function StepGroupInner(props, ref) {
   const {
     attached,
     children,
@@ -55,26 +55,27 @@ function StepGroup(props) {
 
   if (!childrenUtils.isNil(children)) {
     return (
-      <ElementType {...rest} className={classes}>
+      <ElementType {...rest} className={classes} ref={ref}>
         {children}
       </ElementType>
     )
   }
   if (!childrenUtils.isNil(content)) {
     return (
-      <ElementType {...rest} className={classes}>
+      <ElementType {...rest} className={classes} ref={ref}>
         {content}
       </ElementType>
     )
   }
 
   return (
-    <ElementType {...rest} className={classes}>
+    <ElementType {...rest} className={classes} ref={ref}>
       {_.map(items, (item) => Step.create(item))}
     </ElementType>
   )
-}
+})
 
+StepGroup.displayName = 'StepGroup'
 StepGroup.propTypes = {
   /** An element type to render as (string or function). */
   as: PropTypes.elementType,

src/elements/Step/StepTitle.js

@@ -13,19 +13,20 @@ import {
 /**
  * A step can contain a title.
  */
-function StepTitle(props) {
+const StepTitle = React.forwardRef(function StepTitleInner(props, ref) {
   const { children, className, content } = props
   const classes = cx('title', className)
   const rest = getUnhandledProps(StepTitle, props)
   const ElementType = getElementType(StepTitle, props)
 
   return (
-    <ElementType {...rest} className={classes}>
+    <ElementType {...rest} className={classes} ref={ref}>
       {childrenUtils.isNil(children) ? content : children}
     </ElementType>
   )
-}
+})
 
+StepTitle.displayName = 'StepTitle'
 StepTitle.propTypes = {
   /** An element type to render as (string or function). */
   as: PropTypes.elementType,

src/lib/hooks/useEventCallback.js

@@ -0,0 +1,35 @@
+import * as React from 'react'
+import useIsomorphicLayoutEffect from './useIsomorphicLayoutEffect'
+
+/**
+ * https://reactjs.org/docs/hooks-faq.html#how-to-read-an-often-changing-value-from-usecallback
+ *
+ * Modified `useCallback` that can be used when dependencies change too frequently. Can occur when:
+ * e.g. user props are depedencies which could change on every render
+ * e.g. volatile values (i.e. useState/useDispatch) are dependencies which could change frequently
+ *
+ * This should not be used often, but can be a useful re-render optimization since the callback is
+ * a ref and will not be invalidated between rerenders.
+ *
+ * @param {Function} fn The callback function that will be used
+ */
+export default function useEventCallback(fn) {
+  const callbackRef = React.useRef(() => {
+    if (process.env.NODE_ENV !== 'production') {
+      throw new Error('Cannot call an event handler while rendering...')
+    }
+  })
+
+  useIsomorphicLayoutEffect(() => {
+    callbackRef.current = fn
+  }, [fn])
+
+  return React.useCallback(
+    (...args) => {
+      const callback = callbackRef.current
+
+      return callback(...args)
+    },
+    [callbackRef],
+  )
+}

src/lib/index.js

@@ -47,3 +47,4 @@ export { makeDebugger }
 //
 
 export useClassNamesOnNode from './hooks/useClassNamesOnNode'
+export useEventCallback from './hooks/useEventCallback'

test/specs/addons/Confirm/Confirm-test.js

@@ -26,7 +26,7 @@ describe('Confirm', () => {
     if (wrapper && wrapper.unmount) wrapper.unmount()
   })
 
-  common.isConformant(Confirm)
+  common.isConformant(Confirm, { rendersPortal: true })
 
   common.implementsShorthandProp(Confirm, {
     autoGenerateKey: false,

test/specs/addons/TransitionablePortal/TransitionablePortal-test.js

@@ -10,7 +10,10 @@ const requiredProps = {
 }
 
 describe('TransitionablePortal', () => {
-  common.isConformant(TransitionablePortal, { requiredProps })
+  common.isConformant(TransitionablePortal, {
+    rendersPortal: true,
+    requiredProps,
+  })
 
   describe('children', () => {
     it('renders a Transition', () => {

test/specs/collections/Form/FormInput-test.js

@@ -6,6 +6,48 @@ import * as common from 'test/specs/commonTests'
 
 describe('FormInput', () => {
   common.isConformant(FormInput, {
+    eventTargets: {
+      // keyboard
+      onKeyDown: 'input',
+      onKeyPress: 'input',
+      onKeyUp: 'input',
+
+      // focus
+      onFocus: 'input',
+      onBlur: 'input',
+
+      // form
+      onChange: 'input',
+      onInput: 'input',
+
+      // mouse
+      onClick: 'input',
+      onContextMenu: 'input',
+      onDrag: 'input',
+      onDragEnd: 'input',
+      onDragEnter: 'input',
+      onDragExit: 'input',
+      onDragLeave: 'input',
+      onDragOver: 'input',
+      onDragStart: 'input',
+      onDrop: 'input',
+      onMouseDown: 'input',
+      onMouseEnter: 'input',
+      onMouseLeave: 'input',
+      onMouseMove: 'input',
+      onMouseOut: 'input',
+      onMouseOver: 'input',
+      onMouseUp: 'input',
+
+      // selection
+      onSelect: 'input',
+
+      // touch
+      onTouchCancel: 'input',
+      onTouchEnd: 'input',
+      onTouchMove: 'input',
+      onTouchStart: 'input',
+    },
     ignoredTypingsProps: ['label', 'error'],
   })
   common.labelImplementsHtmlForProp(FormInput)