chore(Image): use React.forwardRef() (#4234)

Author: layershifter

docs/src/utils/docTypes/componentInfoShape.js

@@ -17,7 +17,6 @@ const componentInfoShape = PropTypes.shape({
       name: PropTypes.string,
     }),
   ),
-  constructorName: PropTypes.string.isRequired,
   type: PropTypes.string.isRequired,
   isParent: PropTypes.bool.isRequired,
   isChild: PropTypes.bool.isRequired,

gulp/plugins/util/getComponentInfo.js

@@ -43,9 +43,11 @@ const getComponentInfo = (filepath) => {
   // remove keys we don't use
   delete info.methods
 
-  // add exported Component info
-  const Component = require(absPath).default
-  info.constructorName = _.get(Component, 'prototype.constructor.name', null)
+  if (!info.displayName) {
+    throw new Error(
+      `Please check that static property "displayName" is defined on a component in "${filepath}".`,
+    )
+  }
 
   // add component type
   info.type = componentType

src/elements/Image/Image.js

@@ -25,7 +25,7 @@ import ImageGroup from './ImageGroup'
  * An image is a graphic representation of something.
  * @see Icon
  */
-function Image(props) {
+const Image = React.forwardRef(function ImageInner(props, ref) {
   const {
     avatar,
     bordered,
@@ -68,8 +68,10 @@ function Image(props) {
     'image',
     className,
   )
+
   const rest = getUnhandledProps(Image, props)
   const [imgTagProps, rootProps] = partitionHTMLProps(rest, { htmlProps: htmlImageProps })
+
   const ElementType = getElementType(Image, props, () => {
     if (
       !_.isNil(dimmer) ||
@@ -83,33 +85,36 @@ function Image(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>
     )
   }
 
   if (ElementType === 'img') {
-    return <ElementType {...rootProps} {...imgTagProps} className={classes} />
+    return <ElementType {...rootProps} {...imgTagProps} className={classes} ref={ref} />
   }
+
   return (
     <ElementType {...rootProps} className={classes} href={href}>
       {Dimmer.create(dimmer, { autoGenerateKey: false })}
       {Label.create(label, { autoGenerateKey: false })}
-      <img {...imgTagProps} />
+
+      <img {...imgTagProps} ref={ref} />
     </ElementType>
   )
-}
+})
 
 Image.Group = ImageGroup
 
+Image.displayName = 'Image'
 Image.propTypes = {
   /** An element type to render as (string or function). */
   as: PropTypes.elementType,

src/lib/factories.js

@@ -1,6 +1,7 @@
 import cx from 'clsx'
 import _ from 'lodash'
 import * as React from 'react'
+import * as ReactIs from 'react-is'
 
 const DEPRECATED_CALLS = {}
 
@@ -21,8 +22,8 @@ const DEPRECATED_CALLS = {}
  * @returns {object|null}
  */
 export function createShorthand(Component, mapValueToProps, val, options = {}) {
-  if (typeof Component !== 'function' && typeof Component !== 'string') {
-    throw new Error('createShorthand() Component must be a string or function.')
+  if (!ReactIs.isValidElementType(Component)) {
+    throw new Error('createShorthand(): Component should be a valid element type.')
   }
 
   // short circuit noop values
@@ -157,8 +158,8 @@ export function createShorthand(Component, mapValueToProps, val, options = {}) {
  * @returns {function} A shorthand factory function waiting for `val` and `defaultProps`.
  */
 export function createShorthandFactory(Component, mapValueToProps) {
-  if (typeof Component !== 'function' && typeof Component !== 'string') {
-    throw new Error('createShorthandFactory() Component must be a string or function.')
+  if (!ReactIs.isValidElementType(Component)) {
+    throw new Error('createShorthandFactory(): Component should be a valid element type.')
   }
 
   return (val, options) => createShorthand(Component, mapValueToProps, val, options)

test/specs/commonTests/forwardsRef.js

@@ -0,0 +1,21 @@
+import React from 'react'
+import { sandbox } from 'test/utils'
+
+/**
+ * Assert a Component correctly implements a shorthand create method.
+ * @param {React.Component} Component The component to test
+ */
+export default function implementsCreateMethod(Component, options = {}) {
+  describe('forwardsRef', () => {
+    const { requiredProps = {}, tagName = 'div' } = options
+
+    it(`forwards ref to "${tagName}"`, () => {
+      const ref = sandbox.spy()
+
+      mount(<Component {...requiredProps} ref={ref} />)
+
+      ref.should.have.been.calledOnce()
+      ref.should.have.been.calledWithMatch({ tagName: tagName.toUpperCase() })
+    })
+  })
+}

test/specs/commonTests/hasValidTypings.js

@@ -2,6 +2,7 @@ import _ from 'lodash'
 
 import { customPropTypes } from 'src/lib'
 import { componentInfoContext } from 'docs/src/utils'
+import { getComponentName, getComponentProps } from 'test/utils'
 import { getNodes, getInterfaces, hasAnySignature, requireTs } from './tsHelpers'
 
 const isShorthand = (propType) =>
@@ -27,8 +28,8 @@ const shorthandMap = {
  * @param {array} [options.ignoredTypingsProps=[]] Props that will be ignored in tests.
  * @param {Object} [options.requiredProps={}] Props required to render Component without errors or warnings.
  */
-export default (Component, componentInfo, options = {}) => {
-  const { displayName, repoPath } = componentInfoContext.fromComponent(Component)
+export default function hasValidTypings(Component, componentInfo, options = {}) {
+  const { displayName, repoPath } = componentInfoContext.byDisplayName[getComponentName(Component)]
   const { ignoredTypingsProps = [], requiredProps } = options
 
   const tsFile = repoPath.replace('src/', '').replace('.js', '.d.ts')
@@ -79,7 +80,7 @@ export default (Component, componentInfo, options = {}) => {
       })
 
       it('match the typings interface', () => {
-        const componentPropTypes = _.get(Component, 'propTypes')
+        const componentPropTypes = getComponentProps(Component).propTypes
         const componentProps = _.keys(componentPropTypes)
         const interfaceProps = _.without(_.map(props, 'name'), ...ignoredTypingsProps)
 

test/specs/commonTests/implementsCreateMethod.js

@@ -1,15 +1,14 @@
-import _ from 'lodash'
 import React, { isValidElement } from 'react'
-import { consoleUtil } from 'test/utils'
+import { consoleUtil, getComponentName } from 'test/utils'
 
 /**
  * Assert a Component correctly implements a shorthand create method.
  * @param {React.Component|Function} Component The component to test.
  */
-export default (Component) => {
-  const { name } = _.get(Component, 'prototype.constructor.name')
-
+export default function implementsCreateMethod(Component) {
   describe('create shorthand method (common)', () => {
+    const name = getComponentName(Component)
+
     beforeEach(() => {
       // we generate prop values which may throw warnings
       // prevent failures due to console activity

test/specs/commonTests/index.js

@@ -1,3 +1,5 @@
+export forwardsRef from './forwardsRef'
+
 export hasSubcomponents from './hasSubcomponents'
 export hasValidTypings from './hasValidTypings'
 export hasUIClassName from './hasUIClassName'

test/specs/commonTests/isConformant.js

@@ -1,12 +1,19 @@
 import faker from 'faker'
 import _ from 'lodash'
 import React from 'react'
+import ReactIs from 'react-is'
 import ReactDOMServer from 'react-dom/server'
 import * as semanticUIReact from 'semantic-ui-react'
 
 import { componentInfoContext } from 'docs/src/utils'
-import { assertBodyContains, consoleUtil, sandbox, syntheticEvent } from 'test/utils'
-import helpers from './commonHelpers'
+import {
+  assertBodyContains,
+  consoleUtil,
+  getComponentName,
+  getComponentProps,
+  sandbox,
+  syntheticEvent,
+} from 'test/utils'
 import hasValidTypings from './hasValidTypings'
 
 /**
@@ -20,7 +27,7 @@ import hasValidTypings from './hasValidTypings'
  * @param {boolean} [options.rendersPortal=false] Does this component render a Portal powered component?
  * @param {Object} [options.requiredProps={}] Props required to render Component without errors or warnings.
  */
-export default (Component, options = {}) => {
+export default function isConformant(Component, options = {}) {
   const {
     eventTargets = {},
     nestingLevel = 0,
@@ -29,25 +36,26 @@ export default (Component, options = {}) => {
     rendersFragmentByDefault = false,
     rendersPortal = false,
   } = options
-  const { throwError } = helpers('isConformant', Component)
+  const constructorName = getComponentName(Component)
 
-  const componentType = typeof Component
-
-  // make sure components are properly exported
-  if (componentType !== 'function') {
-    throwError(`Components should export a class or function, got: ${componentType}.`)
-  }
-
-  // tests depend on Component constructor names, enforce them
-  const constructorName = Component.prototype.constructor.name
-  if (!constructorName) {
-    throwError(
-      [
-        'Component is not a named function. This should help identify it:\n\n',
-        `${ReactDOMServer.renderToStaticMarkup(<Component />)}`,
-      ].join(''),
+  it('a valid component should be exported', () => {
+    expect(ReactIs.isValidElementType(Component)).to.equal(
+      true,
+      `Components should export a class or function, got: ${typeof Component}.`,
     )
-  }
+  })
+
+  it('a component should be a function/class or "displayName" should be defined', () => {
+    if (!constructorName) {
+      throw new Error(
+        [
+          'Component is not a named function and does not have a "displayName".',
+          'This should help identify it:\n\n',
+          `${ReactDOMServer.renderToStaticMarkup(<Component {...requiredProps} />)}`,
+        ].join(''),
+      )
+    }
+  })
 
   const info = componentInfoContext.byDisplayName[constructorName]
 
@@ -192,20 +200,22 @@ export default (Component, options = {}) => {
   }
 
   describe('handles props', () => {
+    const componentProps = getComponentProps(Component)
+
     it('defines handled props in Component.handledProps', () => {
-      Component.should.have.any.keys('handledProps')
-      Component.handledProps.should.be.an('array')
+      componentProps.should.have.any.keys('handledProps')
+      componentProps.handledProps.should.be.an('array')
     })
 
     it('Component.handledProps includes all handled props', () => {
       const computedProps = _.union(
-        Component.autoControlledProps,
-        _.keys(Component.defaultProps),
-        _.keys(Component.propTypes),
+        componentProps.autoControlledProps,
+        _.keys(componentProps.defaultProps),
+        _.keys(componentProps.propTypes),
       )
       const expectedProps = _.uniq(computedProps).sort()
 
-      Component.handledProps.should.to.deep.equal(
+      componentProps.handledProps.should.to.deep.equal(
         expectedProps,
         'It seems that not all props were defined in Component.handledProps, you need to check that they are equal ' +
           'to the union of Component.autoControlledProps and keys of Component.defaultProps and Component.propTypes',
@@ -376,6 +386,7 @@ export default (Component, options = {}) => {
       })
     })
   }
+
   // ----------------------------------------
   // Test typings
   // ----------------------------------------

test/specs/elements/Image/Image-test.js

@@ -1,4 +1,5 @@
 import _ from 'lodash'
+import faker from 'faker'
 import React from 'react'
 
 import Image from 'src/elements/Image/Image'
@@ -9,6 +10,21 @@ import * as common from 'test/specs/commonTests'
 
 describe('Image', () => {
   common.isConformant(Image)
+
+  common.forwardsRef(Image, { tagName: 'img' })
+  common.forwardsRef(Image, {
+    requiredProps: { as: 'div', children: <span /> },
+    tagName: 'div',
+  })
+  common.forwardsRef(Image, {
+    requiredProps: { as: 'div', content: <span /> },
+    tagName: 'div',
+  })
+  common.forwardsRef(Image, {
+    requiredProps: { label: faker.lorem.word() },
+    tagName: 'img',
+  })
+
   common.hasSubcomponents(Image, [ImageGroup])
   common.hasUIClassName(Image)
   common.rendersChildren(Image)
@@ -40,10 +56,8 @@ describe('Image', () => {
   common.propValueOnlyToClassName(Image, 'size', SUI.SIZES)
 
   describe('as', () => {
-    it('renders an img tag', () => {
-      shallow(<Image />)
-        .type()
-        .should.equal('img')
+    it('renders "i" by default', () => {
+      mount(<Image />).should.have.tagName('img')
     })
   })
 

test/utils/getComponentName.js

@@ -0,0 +1,19 @@
+import * as ReactIs from 'react-is'
+
+/**
+ * Gets a proper `displayName` for a component.
+ *
+ * @param {React.ElementType} Component
+ * @return {String}
+ */
+export default function getComponentName(Component) {
+  if (Component.$$typeof === ReactIs.Memo) {
+    return getComponentName(Component.type)
+  }
+
+  if (Component.$$typeof === ReactIs.ForwardRef) {
+    return Component.displayName
+  }
+
+  return Component.prototype?.constructor?.name
+}

test/utils/getComponentProps.js

@@ -0,0 +1,20 @@
+import * as ReactIs from 'react-is'
+
+/**
+ * Gets proper props for a component.
+ *
+ * @param {React.ElementType} Component
+ * @return {Object}
+ */
+export default function getComponentProps(Component) {
+  if (Component.$$typeof === ReactIs.Memo) {
+    return getComponentProps(Component.type)
+  }
+
+  return {
+    autoControlledProps: Component.autoControlledProps,
+    defaultProps: Component.defaultProps,
+    handledProps: Component.handledProps,
+    propTypes: Component.propTypes,
+  }
+}

test/utils/index.js

@@ -3,6 +3,8 @@ export * from './assertNodeContains'
 export assertWithTimeout from './assertWithTimeout'
 export consoleUtil from './consoleUtil'
 export domEvent from './domEvent'
+export getComponentName from './getComponentName'
+export getComponentProps from './getComponentProps'
 export nestedShallow from './nestedShallow'
 export sandbox from './sandbox'
 export syntheticEvent from './syntheticEvent'