[styles] Forward refs in with* hocs

mui · Mar 5, 2019 · 4998d69 · 4998d69
1 parent 8d46415
commit 4998d69
Show file tree

Hide file tree

Showing 8 changed files with 171 additions and 52 deletions.
diff --git a/docs/src/modules/components/AppDrawerNavItem.js b/docs/src/modules/components/AppDrawerNavItem.js
@@ -82,9 +82,9 @@ class AppDrawerNavItem extends React.Component {
       return (
         <ListItem className={classes.itemLeaf} disableGutters {...other}>
           <Button
-            component={props => (
-              <Link naked activeClassName={classes.active} href={href} {...props} />
-            )}
+            component={React.forwardRef((props, ref) => (
+              <Link naked activeClassName={classes.active} href={href} ref={ref} {...props} />
+            ))}
             className={clsx(classes.buttonLeaf, `depth-${depth}`)}
             disableRipple
             onClick={onClick}

diff --git a/docs/src/modules/components/HomeSteps.js b/docs/src/modules/components/HomeSteps.js
@@ -15,6 +15,14 @@ import NoSsr from '@material-ui/core/NoSsr';
 import Link from 'docs/src/modules/components/Link';
 import compose from 'docs/src/modules/utils/compose';
 
+const InstallationLink = React.forwardRef((buttonProps, ref) => (
+  <Link naked prefetch href="/getting-started/installation" ref={ref} {...buttonProps} />
+));
+
+const UsageLink = React.forwardRef((buttonProps, ref) => (
+  <Link naked prefetch href="/getting-started/usage" ref={ref} {...buttonProps} />
+));
+
 const styles = theme => ({
   step: {
     border: `12px solid ${theme.palette.background.paper}`,
@@ -124,13 +132,7 @@ function HomeSteps(props) {
           />
         </div>
         <Divider className={classes.divider} />
-        <Button
-          component={buttonProps => (
-            <Link naked prefetch href="/getting-started/installation" {...buttonProps} />
-          )}
-        >
-          {t('installButton')}
-        </Button>
+        <Button component={InstallationLink}>{t('installButton')}</Button>
       </Grid>
       <Grid item xs={12} md={4} className={classes.step}>
         <div className={classes.stepTitle}>
@@ -158,13 +160,7 @@ function HomeSteps(props) {
           />
         </div>
         <Divider className={classes.divider} />
-        <Button
-          component={buttonProps => (
-            <Link naked prefetch href="/getting-started/usage" {...buttonProps} />
-          )}
-        >
-          {t('usageButton')}
-        </Button>
+        <Button component={UsageLink}>{t('usageButton')}</Button>
       </Grid>
       <Grid item xs={12} md={4} className={clsx(classes.step, classes.rightStep)}>
         <div className={classes.stepTitle}>

diff --git a/docs/src/pages/css-in-js/api/api.md b/docs/src/pages/css-in-js/api/api.md
@@ -221,8 +221,9 @@ This `classes` object contains the name of the class names injected in the DOM.
 
 Some implementation details that might be interesting to being aware of:
 - It adds a `classes` property so you can override the injected class names from the outside.
-- It adds an `innerRef` property so you can get a reference to the wrapped component. The usage of `innerRef` is identical to `ref`.
-- It forwards *non React static* properties so this HOC is more "transparent".
+- It forwards refs to the inner component.
+- The `innerRef` prop is deprecated. Use `ref` instead.
+- It does **not** copy over statics.
 For instance, it can be used to defined a `getInitialProps()` static method (next.js).
 
 #### Arguments
@@ -296,7 +297,7 @@ in the render method.
 
 #### Returns
 
-`Component`: The new component created.
+`Component`: The new component created. Does forward refs to the inner component.
 
 #### Examples
 

diff --git a/packages/material-ui-styles/src/RefHolder.js b/packages/material-ui-styles/src/RefHolder.js
diff --git a/packages/material-ui-styles/src/withStyles.js b/packages/material-ui-styles/src/withStyles.js
@@ -2,9 +2,8 @@ import React from 'react';
 import PropTypes from 'prop-types';
 import warning from 'warning';
 import hoistNonReactStatics from 'hoist-non-react-statics';
-import { getDisplayName } from '@material-ui/utils';
+import { chainPropTypes, getDisplayName } from '@material-ui/utils';
 import makeStyles from './makeStyles';
-import RefHolder from './RefHolder';
 import getThemeProps from './getThemeProps';
 import useTheme from './useTheme';
 
@@ -67,11 +66,7 @@ const withStyles = (stylesOrCreator, options = {}) => Component => {
       }
     }
 
-    return (
-      <RefHolder ref={ref}>
-        <Component ref={innerRef} classes={classes} {...more} />
-      </RefHolder>
-    );
+    return <Component ref={innerRef || ref} classes={classes} {...more} />;
   });
 
   WithStyles.propTypes = {
@@ -80,9 +75,19 @@ const withStyles = (stylesOrCreator, options = {}) => Component => {
      */
     classes: PropTypes.object,
     /**
+     * @deprecated
      * Use that property to pass a ref callback to the decorated component.
      */
-    innerRef: PropTypes.oneOfType([PropTypes.func, PropTypes.object]),
+    innerRef: chainPropTypes(PropTypes.oneOfType([PropTypes.func, PropTypes.object]), props => {
+      if (props.innerRef == null) {
+        return null;
+      }
+
+      return new Error(
+        'Material-UI: The `innerRef` prop is deprecated and will be removed in v5. ' +
+          'Refs are now automatically forwarded to the inner component.',
+      );
+    }),
   };
 
   if (process.env.NODE_ENV !== 'production') {

diff --git a/packages/material-ui-styles/src/withStyles.test.js b/packages/material-ui-styles/src/withStyles.test.js
@@ -7,6 +7,7 @@ import { Input } from '@material-ui/core';
 import { createMount } from '@material-ui/core/test-utils';
 import { isMuiElement } from '@material-ui/core/utils/reactHelpers';
 import createMuiTheme from '@material-ui/core/styles/createMuiTheme';
+import consoleErrorMock from 'test/utils/consoleErrorMock';
 import StylesProvider from './StylesProvider';
 import ThemeProvider from './ThemeProvider';
 import withStyles from './withStyles';
@@ -38,6 +39,68 @@ describe('withStyles', () => {
     assert.strictEqual(isMuiElement(<StyledInput />, ['Input']), true);
   });
 
+  describe('refs', () => {
+    it('forwards ref to class components', () => {
+      // eslint-disable-next-line react/prefer-stateless-function
+      class TargetComponent extends React.Component {
+        render() {
+          return null;
+        }
+      }
+      const StyledTarget = withStyles({})(TargetComponent);
+
+      const ref = React.createRef();
+      mount(
+        <>
+          <StyledTarget ref={ref} />
+        </>,
+      );
+      assert.instanceOf(ref.current, TargetComponent);
+    });
+
+    it('forwards refs to React.forwardRef types', () => {
+      const StyledTarget = withStyles({})(
+        // eslint-disable-next-line react/no-multi-comp
+        React.forwardRef((props, ref) => <div {...props} ref={ref} />),
+      );
+
+      const ref = React.createRef();
+      mount(
+        <>
+          <StyledTarget ref={ref} />
+        </>,
+      );
+      assert.strictEqual(ref.current.nodeName, 'DIV');
+    });
+
+    describe('innerRef', () => {
+      beforeEach(() => {
+        consoleErrorMock.spy();
+      });
+
+      afterEach(() => {
+        consoleErrorMock.reset();
+        PropTypes.resetWarningCache();
+      });
+
+      it('is deprecated', () => {
+        const ThemedDiv = withStyles({})('div');
+
+        mount(
+          <>
+            <ThemedDiv innerRef={React.createRef()} />
+          </>,
+        );
+
+        assert.strictEqual(consoleErrorMock.callCount(), 1);
+        assert.include(
+          consoleErrorMock.args()[0][0],
+          'Warning: Failed prop type: Material-UI: The `innerRef` prop is deprecated',
+        );
+      });
+    });
+  });
+
   it('should forward the properties', () => {
     const Test = props => <div>{props.foo}</div>;
     Test.propTypes = {

diff --git a/packages/material-ui-styles/src/withTheme.js b/packages/material-ui-styles/src/withTheme.js
@@ -1,9 +1,8 @@
 import React from 'react';
 import PropTypes from 'prop-types';
 import hoistNonReactStatics from 'hoist-non-react-statics';
-import { getDisplayName } from '@material-ui/utils';
+import { chainPropTypes, getDisplayName } from '@material-ui/utils';
 import useTheme from './useTheme';
-import RefHolder from './RefHolder';
 
 // Provide the theme object as a property to the input component.
 // It's an alternative API to useTheme().
@@ -21,18 +20,24 @@ const withTheme = Component => {
   const WithTheme = React.forwardRef(function WithTheme(props, ref) {
     const { innerRef, ...other } = props;
     const theme = useTheme();
-    return (
-      <RefHolder ref={ref}>
-        <Component theme={theme} ref={innerRef} {...other} />
-      </RefHolder>
-    );
+    return <Component theme={theme} ref={innerRef || ref} {...other} />;
   });
 
   WithTheme.propTypes = {
     /**
+     * @deprecated
      * Use that property to pass a ref callback to the decorated component.
      */
-    innerRef: PropTypes.oneOfType([PropTypes.func, PropTypes.object]),
+    innerRef: chainPropTypes(PropTypes.oneOfType([PropTypes.func, PropTypes.object]), props => {
+      if (props.innerRef == null) {
+        return null;
+      }
+
+      return new Error(
+        'Material-UI: The `innerRef` prop is deprecated and will be removed in v5. ' +
+          'Refs are now automatically forwarded to the inner component.',
+      );
+    }),
   };
 
   if (process.env.NODE_ENV !== 'production') {

diff --git a/packages/material-ui-styles/src/withTheme.test.js b/packages/material-ui-styles/src/withTheme.test.js
@@ -1,9 +1,11 @@
+/* eslint-disable react/no-multi-comp */
 import React from 'react';
 import { assert } from 'chai';
 import { createMount } from '@material-ui/core/test-utils';
 import { Input } from '@material-ui/core';
 import { isMuiElement } from '@material-ui/core/utils/reactHelpers';
 import PropTypes from 'prop-types';
+import consoleErrorMock from 'test/utils/consoleErrorMock';
 import withTheme from './withTheme';
 import ThemeProvider from './ThemeProvider';
 
@@ -52,6 +54,69 @@ describe('withTheme', () => {
     assert.strictEqual(isMuiElement(<ThemedInput />, ['Input']), true);
   });
 
+  describe('refs', () => {
+    it('forwards ref to class components', () => {
+      // eslint-disable-next-line react/prefer-stateless-function
+      class TargetComponent extends React.Component {
+        render() {
+          return null;
+        }
+      }
+      const ThemedTarget = withTheme(TargetComponent);
+
+      const ref = React.createRef();
+      mount(
+        <>
+          <ThemedTarget ref={ref} />
+        </>,
+      );
+
+      assert.instanceOf(ref.current, TargetComponent);
+    });
+
+    it('forwards refs to React.forwardRef types', () => {
+      const ThemedTarget = withTheme(
+        React.forwardRef((props, ref) => <div {...props} ref={ref} />),
+      );
+
+      const ref = React.createRef();
+      mount(
+        <>
+          <ThemedTarget ref={ref} />
+        </>,
+      );
+
+      assert.strictEqual(ref.current.nodeName, 'DIV');
+    });
+
+    describe('innerRef', () => {
+      beforeEach(() => {
+        consoleErrorMock.spy();
+      });
+
+      afterEach(() => {
+        consoleErrorMock.reset();
+        PropTypes.resetWarningCache();
+      });
+
+      it('is deprecated', () => {
+        const ThemedDiv = withTheme('div');
+
+        mount(
+          <>
+            <ThemedDiv innerRef={React.createRef()} />
+          </>,
+        );
+
+        assert.strictEqual(consoleErrorMock.callCount(), 1);
+        assert.include(
+          consoleErrorMock.args()[0][0],
+          'Warning: Failed prop type: Material-UI: The `innerRef` prop is deprecated',
+        );
+      });
+    });
+  });
+
   it('should throw is the import is invalid', () => {
     assert.throw(
       () => withTheme(undefined),