[styles] Improve ref forwarding

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}

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}>

docs/src/modules/components/PageTitle.js

@@ -5,18 +5,15 @@ import PageContext from 'docs/src/modules/components/PageContext';
 
 // TODO: it really wants to be named useTitle but we're not quite there yet.
 function PageTitle(props) {
-  return (
-    <PageContext.Consumer>
-      {({ activePage }) => {
-        if (!activePage) {
-          throw new Error('Missing activePage.');
-        }
+  const { activePage } = React.useContext(PageContext);
 
-        const title = activePage.title !== false ? pageToTitle(activePage) : null;
-        return props.children(title);
-      }}
-    </PageContext.Consumer>
-  );
+  if (!activePage) {
+    throw new Error('Missing activePage.');
+  }
+
+  const title = activePage.title !== false ? pageToTitle(activePage) : null;
+
+  return props.children(title);
 }
 
 PageTitle.propTypes = {

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
 

packages/material-ui-styles/src/install.js

@@ -1,4 +1,3 @@
-/* istanbul ignore file */
 /* eslint-disable no-underscore-dangle */
 
 import { ponyfillGlobal } from '@material-ui/utils';

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,20 @@ 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 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') {

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(
+        <React.Fragment>
+          <StyledTarget ref={ref} />
+        </React.Fragment>,
+      );
+      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(
+        <React.Fragment>
+          <StyledTarget ref={ref} />
+        </React.Fragment>,
+      );
+      assert.strictEqual(ref.current.nodeName, 'DIV');
+    });
+
+    // describe('innerRef', () => {
+    //   beforeEach(() => {
+    //     consoleErrorMock.spy();
+    //   });
+
+    //   afterEach(() => {
+    //     consoleErrorMock.reset();
+    //     PropTypes.resetWarningCache();
+    //   });
+
+    //   it('is deprecated', () => {
+    //     const ThemedDiv = withStyles({})('div');
+
+    //     mount(
+    //       <React.Fragment>
+    //         <ThemedDiv innerRef={React.createRef()} />
+    //       </React.Fragment>,
+    //     );
+
+    //     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 = {

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') {

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(
+        <React.Fragment>
+          <ThemedTarget ref={ref} />
+        </React.Fragment>,
+      );
+
+      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(
+        <React.Fragment>
+          <ThemedTarget ref={ref} />
+        </React.Fragment>,
+      );
+
+      assert.strictEqual(ref.current.nodeName, 'DIV');
+    });
+
+    describe('innerRef', () => {
+      beforeEach(() => {
+        consoleErrorMock.spy();
+      });
+
+      afterEach(() => {
+        consoleErrorMock.reset();
+        PropTypes.resetWarningCache();
+      });
+
+      it('is deprecated', () => {
+        const ThemedDiv = withTheme('div');
+
+        mount(
+          <React.Fragment>
+            <ThemedDiv innerRef={React.createRef()} />
+          </React.Fragment>,
+        );
+
+        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),

packages/material-ui-utils/src/chainPropTypes.js

@@ -1,5 +1,4 @@
 function chainPropTypes(propType1, propType2) {
-  /* istanbul ignore if */
   if (process.env.NODE_ENV === 'production') {
     return () => null;
   }

packages/material-ui-utils/src/exactProp.js

@@ -6,7 +6,6 @@
 export const specialProperty = 'exact-prop: \u200b';
 
 function exactProp(propTypes) {
-  /* istanbul ignore if */
   if (process.env.NODE_ENV === 'production') {
     return propTypes;
   }

packages/material-ui/src/ButtonBase/createRippleHandler.js

@@ -27,7 +27,6 @@ let createRippleHandler = (instance, eventName, action, cb) => event => {
   return true;
 };
 
-/* istanbul ignore if */
 if (typeof window === 'undefined') {
   createRippleHandler = () => () => {};
 }