diff --git a/packages/material-ui/src/Portal/Portal.js b/packages/material-ui/src/Portal/Portal.js index 88dceb978a62cb..67b8a8a9a6af41 100644 --- a/packages/material-ui/src/Portal/Portal.js +++ b/packages/material-ui/src/Portal/Portal.js @@ -20,7 +20,7 @@ const useEnhancedEffect = typeof window !== 'undefined' ? React.useLayoutEffect const Portal = React.forwardRef(function Portal(props, ref) { const { children, container, disablePortal = false, onRendered } = props; const [mountNode, setMountNode] = React.useState(null); - const handleRef = useForkRef(children.ref, ref); + const handleRef = useForkRef(React.isValidElement(children) ? children.ref : null, ref); useEnhancedEffect(() => { if (!disablePortal) { @@ -46,10 +46,12 @@ const Portal = React.forwardRef(function Portal(props, ref) { }, [onRendered, mountNode, disablePortal]); if (disablePortal) { - React.Children.only(children); - return React.cloneElement(children, { - ref: handleRef, - }); + if (React.isValidElement(children)) { + return React.cloneElement(children, { + ref: handleRef, + }); + } + return children; } return mountNode ? ReactDOM.createPortal(children, mountNode) : mountNode; diff --git a/packages/material-ui/src/Portal/Portal.test.js b/packages/material-ui/src/Portal/Portal.test.js index 2c903303f16685..49d9bce0a55b22 100644 --- a/packages/material-ui/src/Portal/Portal.test.js +++ b/packages/material-ui/src/Portal/Portal.test.js @@ -1,22 +1,18 @@ /* eslint-disable react/prop-types */ import React from 'react'; -import { assert } from 'chai'; +import { expect } from 'chai'; import { spy } from 'sinon'; -import { createMount, createRender } from '@material-ui/core/test-utils'; +import { createRender } from '@material-ui/core/test-utils'; import consoleErrorMock from 'test/utils/consoleErrorMock'; +import { createClientRender } from 'test/utils/createClientRender'; import Portal from './Portal'; describe('', () => { - let mount; - let render; + let serverRender; + const render = createClientRender({ strict: true }); before(() => { - mount = createMount({ strict: true }); - render = createRender(); - }); - - after(() => { - mount.cleanUp(); + serverRender = createRender(); }); describe('server-side', () => { @@ -34,64 +30,65 @@ describe('', () => { }); it('render nothing on the server', () => { - const markup1 = render(
Bar
); - assert.strictEqual(markup1.text(), 'Bar'); + const markup1 = serverRender(
Bar
); + expect(markup1.text()).to.equal('Bar'); - const markup2 = render( + const markup2 = serverRender(
Bar
, ); - assert.strictEqual(markup2.text(), ''); + expect(markup2.text()).to.equal(''); }); }); describe('ref', () => { it('should have access to the mountNode when disabledPortal={false}', () => { const refSpy = spy(); - const wrapper = mount( + const { unmount } = render(

Foo

, ); - assert.deepEqual(refSpy.args, [[document.body]]); - wrapper.unmount(); - assert.deepEqual(refSpy.args, [[document.body], [null]]); + expect(refSpy.args).to.deep.equal([[document.body]]); + unmount(); + expect(refSpy.args).to.deep.equal([[document.body], [null]]); }); it('should have access to the mountNode when disabledPortal={true}', () => { const refSpy = spy(); - const wrapper = mount( + const { unmount } = render(

Foo

, ); const mountNode = document.querySelector('.woofPortal'); - assert.deepEqual(refSpy.args, [[mountNode]]); - wrapper.unmount(); - assert.deepEqual(refSpy.args, [[mountNode], [null]]); + expect(refSpy.args).to.deep.equal([[mountNode]]); + unmount(); + expect(refSpy.args).to.deep.equal([[mountNode], [null]]); }); it('should have access to the mountNode when switching disabledPortal', () => { const refSpy = spy(); - const wrapper = mount( + const { setProps, unmount } = render(

Foo

, ); const mountNode = document.querySelector('.woofPortal'); - assert.deepEqual(refSpy.args, [[mountNode]]); - wrapper.setProps({ + expect(refSpy.args).to.deep.equal([[mountNode]]); + setProps({ disablePortal: false, + ref: refSpy, }); - assert.deepEqual(refSpy.args, [[mountNode], [null], [document.body]]); - wrapper.unmount(); - assert.deepEqual(refSpy.args, [[mountNode], [null], [document.body], [null]]); + expect(refSpy.args).to.deep.equal([[mountNode], [null], [document.body]]); + unmount(); + expect(refSpy.args).to.deep.equal([[mountNode], [null], [document.body], [null]]); }); }); it('should render in a different node', () => { - mount( + render(

Foo

@@ -100,8 +97,8 @@ describe('', () => {
, ); const rootElement = document.querySelector('#test1'); - assert.strictEqual(rootElement.contains(document.querySelector('.woofPortal1')), true); - assert.strictEqual(rootElement.contains(document.querySelector('.woofPortal2')), false); + expect(rootElement.contains(document.querySelector('.woofPortal1'))).to.equal(true); + expect(rootElement.contains(document.querySelector('.woofPortal2'))).to.equal(false); }); it('should unmount when parent unmounts', () => { @@ -122,29 +119,30 @@ describe('', () => { ); } - const wrapper = mount(); - assert.strictEqual(document.querySelectorAll('#test1').length, 1); - wrapper.setProps({ show: false }); - assert.strictEqual(document.querySelectorAll('#test1').length, 0); + const { setProps } = render(); + expect(document.querySelectorAll('#test1').length).to.equal(1); + setProps({ show: false }); + expect(document.querySelectorAll('#test1').length).to.equal(0); }); it('should render overlay into container (document)', () => { - mount( + render( -
+
+
, ); - assert.strictEqual(document.querySelectorAll('#test2').length, 1); + expect(document.querySelectorAll('.test2').length).to.equal(2); }); it('should render overlay into container (DOMNode)', () => { const container = document.createElement('div'); - mount( + render(
, ); - assert.strictEqual(container.querySelectorAll('#test2').length, 1); + expect(container.querySelectorAll('#test2').length).to.equal(1); }); it('should change container on prop change', () => { @@ -165,82 +163,82 @@ describe('', () => { ); } - const wrapper = mount(); - assert.strictEqual(document.querySelector('#test3').parentElement.nodeName, 'SPAN', 'c'); - wrapper.setProps({ + const { setProps } = render(); + expect(document.querySelector('#test3').parentElement.nodeName).to.equal('SPAN'); + setProps({ containerElement: true, + disablePortal: true, }); - assert.strictEqual(document.querySelector('#test3').parentElement.nodeName, 'SPAN', 'a'); - wrapper.setProps({ + expect(document.querySelector('#test3').parentElement.nodeName).to.equal('SPAN'); + setProps({ + containerElement: true, disablePortal: false, }); - assert.strictEqual(document.querySelector('#test3').parentElement.nodeName, 'STRONG', 'c'); - wrapper.setProps({ + expect(document.querySelector('#test3').parentElement.nodeName).to.equal('STRONG'); + setProps({ containerElement: false, + disablePortal: false, }); - assert.strictEqual(document.querySelector('#test3').parentElement.nodeName, 'BODY', 'b'); + expect(document.querySelector('#test3').parentElement.nodeName).to.equal('BODY'); }); it('should call onRendered', () => { const ref = React.createRef(); const handleRendered = spy(); - mount( + render( { handleRendered(); - assert.strictEqual(ref.current !== null, true); + expect(ref.current !== null).to.equal(true); }} >
, ); - assert.strictEqual(handleRendered.callCount, 1); + expect(handleRendered.callCount).to.equal(1); }); - it('should call onRendered after child componentDidUpdate', () => { + it('should call ref after child effect', () => { + const callOrder = []; + const handleRef = node => { + if (node) { + callOrder.push('ref'); + } + }; + const updateFunction = () => { + callOrder.push('effect'); + }; + function Test(props) { - const { updateFunction, container, onRendered } = props; + const { container } = props; React.useEffect(() => { updateFunction(); - }, [container, updateFunction]); + }, [container]); return ( - +
); } - const callOrder = []; - const onRendered = () => { - callOrder.push('onRendered'); - }; - const updateFunction = () => { - callOrder.push('cDU'); - }; - - const container1 = document.createElement('div'); - const container2 = document.createElement('div'); - - const wrapper = mount( - , - ); - - wrapper.setProps({ container: null }); - wrapper.setProps({ container: container2 }); - wrapper.setProps({ container: null }); - - assert.deepEqual(callOrder, [ - 'cDU', - 'onRendered', - 'cDU', - 'onRendered', - 'cDU', - 'onRendered', - 'cDU', - 'onRendered', + const { setProps } = render(); + + setProps({ container: null }); + setProps({ container: document.createElement('div') }); + setProps({ container: null }); + + expect(callOrder).to.deep.equal([ + 'effect', + 'ref', + 'effect', + 'ref', + 'effect', + 'ref', + 'effect', + 'ref', ]); }); });