diff --git a/packages/material-ui/src/GridListTile/GridListTile.test.js b/packages/material-ui/src/GridListTile/GridListTile.test.js index c2a5b9e3bf3271..0fafafe04bdfa0 100644 --- a/packages/material-ui/src/GridListTile/GridListTile.test.js +++ b/packages/material-ui/src/GridListTile/GridListTile.test.js @@ -3,7 +3,6 @@ import { assert } from 'chai'; import { spy, useFakeTimers } from 'sinon'; import { createMount, getClasses } from '@material-ui/core/test-utils'; import describeConformance from '../test-utils/describeConformance'; -import { setRef } from '../utils/reactHelpers'; import GridListTile from './GridListTile'; describe('', () => { @@ -50,15 +49,9 @@ describe('', () => { function mountMockImage(imgEl) { const Image = React.forwardRef((props, ref) => { - return ( - test { - setRef(ref, imgEl); - }} - {...props} - /> - ); + React.useImperativeHandle(ref, () => imgEl, []); + + return test; }); Image.muiName = 'Image'; diff --git a/packages/material-ui/src/useScrollTrigger/useScrollTrigger.test.js b/packages/material-ui/src/useScrollTrigger/useScrollTrigger.test.js index 7eaee31d08178d..9b6cb4acfcbecd 100644 --- a/packages/material-ui/src/useScrollTrigger/useScrollTrigger.test.js +++ b/packages/material-ui/src/useScrollTrigger/useScrollTrigger.test.js @@ -68,7 +68,7 @@ describe('useScrollTrigger', () => { assert.strictEqual(values.callCount, 1); }); - it('should be false by default when using setRef', () => { + it('should be false by default when using ref', () => { const TestDefaultWithRef = () => { const [container, setContainer] = React.useState(); const trigger = useScrollTrigger({ diff --git a/packages/material-ui/src/utils/reactHelpers.d.ts b/packages/material-ui/src/utils/reactHelpers.d.ts index 6e84ea4eef78a9..93906bad3409f2 100644 --- a/packages/material-ui/src/utils/reactHelpers.d.ts +++ b/packages/material-ui/src/utils/reactHelpers.d.ts @@ -14,6 +14,10 @@ export function isMuiElement(element: any, muiNames: string[]): element is Named /** * passes {value} to {ref} * + * WARNING: Be sure to only call this inside a callback that is passed as a ref. + * Otherwise make sure to cleanup previous {ref} if it changes. See + * https://github.com/mui-org/material-ui/issues/13539 + * * useful if you want to expose the ref of an inner component to the public api * while still using it inside the component * @@ -23,3 +27,5 @@ export function setRef( ref: React.RefObject | ((instance: T | null) => void) | null | undefined, value: T | null, ): void; + +export function useForkRef(refA: React.Ref, refB: React.Ref): React.Ref; diff --git a/packages/material-ui/src/utils/reactHelpers.js b/packages/material-ui/src/utils/reactHelpers.js index 23af7702cc74b9..80f6c4bbf9dbef 100644 --- a/packages/material-ui/src/utils/reactHelpers.js +++ b/packages/material-ui/src/utils/reactHelpers.js @@ -4,6 +4,7 @@ export function isMuiElement(element, muiNames) { return React.isValidElement(element) && muiNames.indexOf(element.type.muiName) !== -1; } +// TODO: Make it private only in v5 export function setRef(ref, value) { if (typeof ref === 'function') { ref(value); diff --git a/packages/material-ui/test/integration/Menu.test.js b/packages/material-ui/test/integration/Menu.test.js index e77af02a4d6804..d0c6969dd28ad9 100644 --- a/packages/material-ui/test/integration/Menu.test.js +++ b/packages/material-ui/test/integration/Menu.test.js @@ -6,7 +6,7 @@ import Popover from '@material-ui/core/Popover'; import SimpleMenu from './fixtures/menus/SimpleMenu'; import Menu from '@material-ui/core/Menu'; import MenuItem from '@material-ui/core/MenuItem'; -import { setRef } from '../../src/utils/reactHelpers'; +import { useForkRef } from '@material-ui/core/utils'; import { stub } from 'sinon'; import PropTypes from 'prop-types'; import { createMuiTheme } from '@material-ui/core/styles'; @@ -154,27 +154,28 @@ describe(' integration', () => { let menuListFocusTracker = false; const tabIndexTracker = [false, false, false]; const TrackingMenuItem = React.forwardRef(({ itemIndex, ...other }, ref) => { + const handleRef = useForkRef(ref, instance => { + if (instance && !instance.stubbed) { + if (instance.tabIndex === 0) { + tabIndexTracker[itemIndex] = true; + } else if (instance.tabIndex > 0) { + tabIndexTracker[itemIndex] = instance.tabIndex; + } + const offsetTop = instance.offsetTop; + stub(instance, 'offsetTop').get(() => { + contentAnchorTracker[itemIndex] = true; + return offsetTop; + }); + instance.stubbed = true; + } + }); + return ( { focusTracker[itemIndex] = true; }} - ref={instance => { - setRef(ref, instance); - if (instance && !instance.stubbed) { - if (instance.tabIndex === 0) { - tabIndexTracker[itemIndex] = true; - } else if (instance.tabIndex > 0) { - tabIndexTracker[itemIndex] = instance.tabIndex; - } - const offsetTop = instance.offsetTop; - stub(instance, 'offsetTop').get(() => { - contentAnchorTracker[itemIndex] = true; - return offsetTop; - }); - instance.stubbed = true; - } - }} + ref={handleRef} {...other} /> );