[Modal][Portal] Deprecate onRendered

docs/pages/api-docs/modal.md

@@ -53,7 +53,7 @@ This component shares many concepts with [react-overlays](https://react-bootstra
 | <span class="prop-name">onBackdropClick</span> | <span class="prop-type">func</span> |  | Callback fired when the backdrop is clicked. |
 | <span class="prop-name">onClose</span> | <span class="prop-type">func</span> |  | Callback fired when the component requests to be closed. The `reason` parameter can optionally be used to control the response to `onClose`.<br><br>**Signature:**<br>`function(event: object, reason: string) => void`<br>*event:* The event source of the callback.<br>*reason:* Can be: `"escapeKeyDown"`, `"backdropClick"`. |
 | <span class="prop-name">onEscapeKeyDown</span> | <span class="prop-type">func</span> |  | Callback fired when the escape key is pressed, `disableEscapeKeyDown` is false and the modal is in focus. |
-| <span class="prop-name">onRendered</span> | <span class="prop-type">func</span> |  | Callback fired once the children has been mounted into the `container`. It signals that the `open={true}` prop took effect.<br>This prop will be deprecated and removed in v5, the ref can be used instead. |
+| ~~<span class="prop-name">onRendered</span>~~ | <span class="prop-type">func</span> |  | *Deprecated*. Use the ref instead.<br><br>Callback fired once the children has been mounted into the `container`. It signals that the `open={true}` prop took effect.<br>This prop will be removed in v5, the ref can be used instead. |
 | <span class="prop-name required">open<abbr title="required">*</abbr></span> | <span class="prop-type">bool</span> |  | If `true`, the modal is open. |
 
 The `ref` is forwarded to the root element.

docs/pages/api-docs/portal.md

@@ -30,7 +30,7 @@ that exists outside the DOM hierarchy of the parent component.
 | <span class="prop-name">children</span> | <span class="prop-type">node</span> |  | The children to render into the `container`. |
 | <span class="prop-name">container</span> | <span class="prop-type">HTML element<br>&#124;&nbsp;React.Component<br>&#124;&nbsp;func</span> |  | A HTML element, component instance, or function that returns either. The `container` will have the portal children appended to it.<br>By default, it uses the body of the top-level document object, so it's simply `document.body` most of the time. |
 | <span class="prop-name">disablePortal</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | Disable the portal behavior. The children stay within it's parent DOM hierarchy. |
-| <span class="prop-name">onRendered</span> | <span class="prop-type">func</span> |  | Callback fired once the children has been mounted into the `container`.<br>This prop will be deprecated and removed in v5, the ref can be used instead. |
+| ~~<span class="prop-name">onRendered</span>~~ | <span class="prop-type">func</span> |  | *Deprecated*. Use the ref instead.<br><br>Callback fired once the children has been mounted into the `container`.<br>This prop will be removed in v5, the ref can be used instead. |
 
 The component cannot hold a ref.
 

packages/material-ui/src/Modal/Modal.d.ts

@@ -31,7 +31,17 @@ export interface ModalProps
     bivarianceHack(event: {}, reason: 'backdropClick' | 'escapeKeyDown'): void;
   }['bivarianceHack'];
   onEscapeKeyDown?: React.ReactEventHandler<{}>;
+  /**
+   * Callback fired once the children has been mounted into the `container`.
+   * It signals that the `open={true}` prop took effect.
+   *
+   * This prop will be removed in v5, the ref can be used instead.
+   * @deprecated Use the ref instead.
+   */
   onRendered?: PortalProps['onRendered'];
+  /**
+   * If `true`, the modal is open.
+   */
   open: boolean;
 }
 

packages/material-ui/src/Modal/Modal.js

@@ -4,6 +4,7 @@ import PropTypes from 'prop-types';
 import { getThemeProps, useTheme } from '@material-ui/styles';
 import { elementAcceptingRef, HTMLElementType } from '@material-ui/utils';
 import ownerDocument from '../utils/ownerDocument';
+import deprecatedPropType from '../utils/deprecatedPropType';
 import Portal from '../Portal';
 import createChainedFunction from '../utils/createChainedFunction';
 import useForkRef from '../utils/useForkRef';
@@ -359,9 +360,9 @@ Modal.propTypes = {
    * Callback fired once the children has been mounted into the `container`.
    * It signals that the `open={true}` prop took effect.
    *
-   * This prop will be deprecated and removed in v5, the ref can be used instead.
+   * This prop will be removed in v5, the ref can be used instead.
    */
-  onRendered: PropTypes.func,
+  onRendered: deprecatedPropType(PropTypes.func, 'Use the ref instead.'),
   /**
    * If `true`, the modal is open.
    */

packages/material-ui/src/Modal/Modal.test.js

@@ -527,8 +527,17 @@ describe('<Modal />', () => {
     });
   });
 
-  describe('prop: onRendered', () => {
-    it('should fire', () => {
+  describe('v5 deprecations', () => {
+    beforeEach(() => {
+      PropTypes.resetWarningCache();
+      consoleErrorMock.spy();
+    });
+
+    afterEach(() => {
+      consoleErrorMock.reset();
+    });
+
+    it('should call onRendered', () => {
       const handleRendered = spy();
 
       render(
@@ -538,6 +547,10 @@ describe('<Modal />', () => {
       );
 
       expect(handleRendered).to.have.property('callCount', 1);
+      expect(console.error.callCount).to.equal(1);
+      expect(console.error.firstCall.args[0]).to.contain(
+        'The prop `onRendered` of `ForwardRef(Modal)` is deprecated. Use the ref instead',
+      );
     });
   });
 

packages/material-ui/src/Portal/Portal.d.ts

@@ -21,7 +21,8 @@ export interface PortalProps {
   /**
    * Callback fired once the children has been mounted into the `container`.
    *
-   * This prop will be deprecated and removed in v5, the ref can be used instead.
+   * This prop will be removed in v5, the ref can be used instead.
+   * @deprecated Use the ref instead.
    */
   onRendered?: () => void;
 }

packages/material-ui/src/Portal/Portal.js

@@ -2,6 +2,7 @@ import * as React from 'react';
 import * as ReactDOM from 'react-dom';
 import PropTypes from 'prop-types';
 import { exactProp, HTMLElementType } from '@material-ui/utils';
+import deprecatedPropType from '../utils/deprecatedPropType';
 import setRef from '../utils/setRef';
 import useForkRef from '../utils/useForkRef';
 
@@ -86,9 +87,10 @@ Portal.propTypes = {
   /**
    * Callback fired once the children has been mounted into the `container`.
    *
-   * This prop will be deprecated and removed in v5, the ref can be used instead.
+   * This prop will be removed in v5, the ref can be used instead.
+   * @deprecated Use the ref instead.
    */
-  onRendered: PropTypes.func,
+  onRendered: deprecatedPropType(PropTypes.func, 'Use the ref instead.'),
 };
 
 if (process.env.NODE_ENV !== 'production') {

packages/material-ui/src/Portal/Portal.test.js

@@ -1,6 +1,7 @@
 import * as React from 'react';
 import { expect } from 'chai';
 import { spy } from 'sinon';
+import PropTypes from 'prop-types';
 import createServerRender from 'test/utils/createServerRender';
 import consoleErrorMock from 'test/utils/consoleErrorMock';
 import { createClientRender } from 'test/utils/createClientRender';
@@ -177,21 +178,36 @@ describe('<Portal />', () => {
     expect(document.querySelector('#test3').parentElement.nodeName).to.equal('BODY');
   });
 
-  it('should call onRendered', () => {
-    const ref = React.createRef();
-    const handleRendered = spy();
-    render(
-      <Portal
-        ref={ref}
-        onRendered={() => {
-          handleRendered();
-          expect(ref.current !== null).to.equal(true);
-        }}
-      >
-        <div />
-      </Portal>,
-    );
-    expect(handleRendered.callCount).to.equal(1);
+  describe('v5 deprecations', () => {
+    beforeEach(() => {
+      PropTypes.resetWarningCache();
+      consoleErrorMock.spy();
+    });
+
+    afterEach(() => {
+      consoleErrorMock.reset();
+    });
+
+    it('should call onRendered', () => {
+      const ref = React.createRef();
+      const handleRendered = spy();
+      render(
+        <Portal
+          ref={ref}
+          onRendered={() => {
+            handleRendered();
+            expect(ref.current !== null).to.equal(true);
+          }}
+        >
+          <div />
+        </Portal>,
+      );
+      expect(handleRendered.callCount).to.equal(1);
+      expect(console.error.callCount).to.equal(1);
+      expect(console.error.firstCall.args[0]).to.contain(
+        'The prop `onRendered` of `ForwardRef(Portal)` is deprecated. Use the ref instead',
+      );
+    });
   });
 
   it('should call ref after child effect', () => {