fix: Apply suggestions from code review

- Add a displayName property to each function component
- Revert DropdownItemSpec changes
- Fix rendering issues caused by callback functions via useCallback
- Reduce imperative code in Toast via useTimeout hook
- Hoist function out of Tabs render.

src/Fade.js

@@ -1,5 +1,5 @@
 import classNames from 'classnames';
-import React from 'react';
+import React, { useCallback } from 'react';
 import PropTypes from 'prop-types';
 import Transition, {
   ENTERED,
@@ -77,10 +77,13 @@ const fadeStyles = {
 };
 
 const Fade = React.forwardRef(({ className, children, ...props }, ref) => {
-  const handleEnter = node => {
-    triggerBrowserReflow(node);
-    if (props.onEnter) props.onEnter(node);
-  };
+  const handleEnter = useCallback(
+    node => {
+      triggerBrowserReflow(node);
+      if (props.onEnter) props.onEnter(node);
+    },
+    [props],
+  );
 
   return (
     <Transition
@@ -106,5 +109,6 @@ const Fade = React.forwardRef(({ className, children, ...props }, ref) => {
 
 Fade.propTypes = propTypes;
 Fade.defaultProps = defaultProps;
+Fade.displayName = 'Fade';
 
 export default Fade;

src/InputGroup.js

@@ -77,6 +77,7 @@ const InputGroupRadio = props => (
 );
 
 InputGroup.propTypes = propTypes;
+InputGroup.displayName = 'InputGroup';
 
 InputGroup.Text = InputGroupText;
 InputGroup.Radio = InputGroupRadio;

src/Jumbotron.js

@@ -45,5 +45,6 @@ const Jumbotron = React.forwardRef(
 
 Jumbotron.propTypes = propTypes;
 Jumbotron.defaultProps = defaultProps;
+Jumbotron.displayName = 'Jumbotron';
 
 export default Jumbotron;

src/ListGroup.js

@@ -61,6 +61,7 @@ const ListGroup = React.forwardRef((props, ref) => {
 
 ListGroup.propTypes = propTypes;
 ListGroup.defaultProps = defaultProps;
+ListGroup.displayName = 'ListGroup';
 
 ListGroup.Item = ListGroupItem;
 

src/ListGroupItem.js

@@ -1,5 +1,5 @@
 import classNames from 'classnames';
-import React from 'react';
+import React, { useCallback } from 'react';
 import PropTypes from 'prop-types';
 
 import AbstractNavItem from './AbstractNavItem';
@@ -71,15 +71,18 @@ const ListGroupItem = React.forwardRef(
   ) => {
     bsPrefix = useBootstrapPrefix(bsPrefix, 'list-group-item');
 
-    const handleClick = event => {
-      if (disabled) {
-        event.preventDefault();
-        event.stopPropagation();
-        return;
-      }
+    const handleClick = useCallback(
+      event => {
+        if (disabled) {
+          event.preventDefault();
+          event.stopPropagation();
+          return;
+        }
 
-      if (onClick) onClick(event);
-    };
+        if (onClick) onClick(event);
+      },
+      [disabled, onClick],
+    );
 
     return (
       <AbstractNavItem
@@ -104,5 +107,6 @@ const ListGroupItem = React.forwardRef(
 
 ListGroupItem.propTypes = propTypes;
 ListGroupItem.defaultProps = defaultProps;
+ListGroupItem.displayName = 'ListGroupItem';
 
 export default ListGroupItem;

src/SafeAnchor.js

@@ -87,5 +87,6 @@ const SafeAnchor = React.forwardRef(
 );
 
 SafeAnchor.propTypes = propTypes;
+SafeAnchor.displayName = 'SafeAnchor';
 
 export default SafeAnchor;

src/Spinner.js

@@ -84,5 +84,6 @@ const Spinner = React.forwardRef(
 );
 
 Spinner.propTypes = propTypes;
+Spinner.displayName = 'Spinner';
 
 export default Spinner;

src/SplitButton.js

@@ -105,5 +105,6 @@ const SplitButton = React.forwardRef(
 
 SplitButton.propTypes = propTypes;
 SplitButton.defaultProps = defaultProps;
+SplitButton.displayName = 'SplitButton';
 
 export default SplitButton;

src/Tabs.js

@@ -93,25 +93,25 @@ function getDefaultActiveKey(children) {
   return defaultActiveKey;
 }
 
-const Tabs = React.forwardRef((props, ref) => {
-  const renderTab = child => {
-    const { title, eventKey, disabled, tabClassName } = child.props;
-    if (title == null) {
-      return null;
-    }
+function renderTab(child) {
+  const { title, eventKey, disabled, tabClassName } = child.props;
+  if (title == null) {
+    return null;
+  }
 
-    return (
-      <NavItem
-        as={NavLink}
-        eventKey={eventKey}
-        disabled={disabled}
-        className={tabClassName}
-      >
-        {title}
-      </NavItem>
-    );
-  };
+  return (
+    <NavItem
+      as={NavLink}
+      eventKey={eventKey}
+      disabled={disabled}
+      className={tabClassName}
+    >
+      {title}
+    </NavItem>
+  );
+}
 
+const Tabs = React.forwardRef((props, ref) => {
   const {
     id,
     onSelect,
@@ -156,5 +156,6 @@ const Tabs = React.forwardRef((props, ref) => {
 
 Tabs.propTypes = propTypes;
 Tabs.defaultProps = defaultProps;
+Tabs.displayName = 'Tabs';
 
 export default Tabs;

src/Toast.js

@@ -1,7 +1,8 @@
-import React, { useEffect, useRef } from 'react';
+import React, { useEffect, useRef, useMemo, useCallback } from 'react';
 import PropTypes from 'prop-types';
 import classNames from 'classnames';
 
+import useTimeout from '@restart/hooks/useTimeout';
 import Fade from './Fade';
 import Header from './ToastHeader';
 import Body from './ToastBody';
@@ -76,26 +77,28 @@ const Toast = React.forwardRef(
     const delayRef = useRef(delay);
     const onCloseRef = useRef(onClose);
 
-    // We use refs for these, because we don't want to restart the autohide
-    // timer in case these values change.
-    delayRef.current = delay;
-    onCloseRef.current = onClose;
-
     useEffect(() => {
+      // We use refs for these, because we don't want to restart the autohide
+      // timer in case these values change.
+      delayRef.current = delay;
+      onCloseRef.current = onClose;
+    }, [delay, onClose]);
+
+    const autohideTimeout = useTimeout();
+    const autohideFunc = useCallback(() => {
       if (!(autohide && show)) {
-        return undefined;
+        return;
       }
+      onCloseRef.current();
+    }, [autohide, show]);
 
-      const autohideHandle = setTimeout(() => {
-        onCloseRef.current();
-      }, delayRef.current);
+    autohideTimeout.set(autohideFunc, delayRef.current);
 
-      return () => {
-        clearTimeout(autohideHandle);
-      };
-    }, [autohide, show]);
+    const useAnimation = useMemo(() => Transition && animation, [
+      Transition,
+      animation,
+    ]);
 
-    const useAnimation = Transition && animation;
     const toast = (
       <div
         {...props}
@@ -127,6 +130,7 @@ const Toast = React.forwardRef(
 
 Toast.propTypes = propTypes;
 Toast.defaultProps = defaultProps;
+Toast.displayName = 'Toast';
 
 Toast.Body = Body;
 Toast.Header = Header;

src/Tooltip.js

@@ -100,5 +100,6 @@ const Tooltip = React.forwardRef(
 
 Tooltip.propTypes = propTypes;
 Tooltip.defaultProps = defaultProps;
+Tooltip.displayName = 'Tooltip';
 
 export default Tooltip;

test/DropdownItemSpec.js

@@ -98,6 +98,13 @@ describe('<Dropdown.Item>', () => {
       .should.not.have.property('onSelect');
   });
 
+  it('does not pass onSelect to children', () => {
+    mount(<Dropdown.Item onSelect={() => {}}>Item</Dropdown.Item>)
+      .find('SafeAnchor')
+      .props()
+      .should.not.have.property('onSelect');
+  });
+
   it('disabled link', () => {
     const handleSelect = () => {
       throw new Error('Should not invoke onSelect event');