From 7115cc2e92c3a74ccc65c290bf3f4fb8f73e6f4d Mon Sep 17 00:00:00 2001 From: Olivier Tassinari Date: Fri, 18 Oct 2019 17:26:06 +0200 Subject: [PATCH] [NoSsr] Fix defer prop regression I suspect an issue in React itself. It was working a year ago. --- .../pages/components/no-ssr/FrameDeferring.js | 34 +++++++++++-------- .../components/no-ssr/FrameDeferring.tsx | 34 +++++++++++-------- .../pages/components/no-ssr/SimpleNoSsr.js | 31 ++++------------- .../pages/components/no-ssr/SimpleNoSsr.tsx | 28 ++++----------- packages/material-ui/src/NoSsr/NoSsr.js | 26 +++++++++++++- packages/material-ui/src/NoSsr/NoSsr.test.js | 9 +++-- 6 files changed, 84 insertions(+), 78 deletions(-) diff --git a/docs/src/pages/components/no-ssr/FrameDeferring.js b/docs/src/pages/components/no-ssr/FrameDeferring.js index fc1b85a60e698d..191e4e96490b94 100644 --- a/docs/src/pages/components/no-ssr/FrameDeferring.js +++ b/docs/src/pages/components/no-ssr/FrameDeferring.js @@ -4,16 +4,17 @@ import NoSsr from '@material-ui/core/NoSsr'; const useStyles = makeStyles({ container: { - maxWidth: 300, - wordBreak: 'break-all', + width: 300, + display: 'flex', + flexWrap: 'wrap', }, }); function LargeTree() { - return Array.from(new Array(3000)).map((_, index) => .); + return Array.from(new Array(5000)).map((_, index) => .); } -function FrameDeferring() { +export default function FrameDeferring() { const classes = useStyles(); const [state, setState] = React.useState({ open: false, defer: false }); @@ -30,6 +31,7 @@ function FrameDeferring() { > {'Render NoSsr defer="false"'} +
- {state.open ? ( -
- Outside NoSsr - - ....Inside NoSsr - - -
- ) : null} +
+
+
+ {state.open ? ( + +
Outside NoSsr
+ + .....Inside NoSsr + + + + ) : null} +
); } - -export default FrameDeferring; diff --git a/docs/src/pages/components/no-ssr/FrameDeferring.tsx b/docs/src/pages/components/no-ssr/FrameDeferring.tsx index d70e2645f831e9..6be37c0ead0311 100644 --- a/docs/src/pages/components/no-ssr/FrameDeferring.tsx +++ b/docs/src/pages/components/no-ssr/FrameDeferring.tsx @@ -4,16 +4,17 @@ import NoSsr from '@material-ui/core/NoSsr'; const useStyles = makeStyles({ container: { - maxWidth: 300, - wordBreak: 'break-all', + width: 300, + display: 'flex', + flexWrap: 'wrap', }, }); function LargeTree(): any { - return Array.from(new Array(3000)).map((_, index) => .); + return Array.from(new Array(5000)).map((_, index) => .); } -function FrameDeferring() { +export default function FrameDeferring() { const classes = useStyles(); const [state, setState] = React.useState({ open: false, defer: false }); @@ -30,6 +31,7 @@ function FrameDeferring() { > {'Render NoSsr defer="false"'} +
- {state.open ? ( -
- Outside NoSsr - - ....Inside NoSsr - - -
- ) : null} +
+
+
+ {state.open ? ( + +
Outside NoSsr
+ + .....Inside NoSsr + + + + ) : null} +
); } - -export default FrameDeferring; diff --git a/docs/src/pages/components/no-ssr/SimpleNoSsr.js b/docs/src/pages/components/no-ssr/SimpleNoSsr.js index 3747b994775d5b..e75b9fe5f4b803 100644 --- a/docs/src/pages/components/no-ssr/SimpleNoSsr.js +++ b/docs/src/pages/components/no-ssr/SimpleNoSsr.js @@ -1,35 +1,18 @@ import React from 'react'; -import PropTypes from 'prop-types'; -import { withStyles } from '@material-ui/core/styles'; import NoSsr from '@material-ui/core/NoSsr'; -import Button from '@material-ui/core/Button'; - -const styles = theme => ({ - button: { - display: 'block', - margin: theme.spacing(2), - }, -}); - -function SimpleNoSsr(props) { - const { classes } = props; +import Box from '@material-ui/core/Box'; +export default function SimpleNoSsr() { return (
- + + Server and Client + - +
); } - -SimpleNoSsr.propTypes = { - classes: PropTypes.object.isRequired, -}; - -export default withStyles(styles)(SimpleNoSsr); diff --git a/docs/src/pages/components/no-ssr/SimpleNoSsr.tsx b/docs/src/pages/components/no-ssr/SimpleNoSsr.tsx index 2a00dead2ff75b..e75b9fe5f4b803 100644 --- a/docs/src/pages/components/no-ssr/SimpleNoSsr.tsx +++ b/docs/src/pages/components/no-ssr/SimpleNoSsr.tsx @@ -1,32 +1,18 @@ import React from 'react'; -import { withStyles, WithStyles } from '@material-ui/core/styles'; import NoSsr from '@material-ui/core/NoSsr'; -import Button from '@material-ui/core/Button'; - -const styles = (theme: any) => ({ - button: { - display: 'block', - margin: theme.spacing(2), - }, -}); - -export interface iProps extends WithStyles {} - -function SimpleNoSsr(props: iProps) { - const { classes } = props; +import Box from '@material-ui/core/Box'; +export default function SimpleNoSsr() { return (
- + + Server and Client + - +
); } - -export default withStyles(styles)(SimpleNoSsr); diff --git a/packages/material-ui/src/NoSsr/NoSsr.js b/packages/material-ui/src/NoSsr/NoSsr.js index 6e452008ab6dee..3208ea0bcebb62 100644 --- a/packages/material-ui/src/NoSsr/NoSsr.js +++ b/packages/material-ui/src/NoSsr/NoSsr.js @@ -19,6 +19,7 @@ const useEnhancedEffect = function NoSsr(props) { const { children, defer = false, fallback = null } = props; const [mountedState, setMountedState] = React.useState(false); + const mounted = React.useRef(); useEnhancedEffect(() => { if (!defer) { @@ -28,8 +29,31 @@ function NoSsr(props) { React.useEffect(() => { if (defer) { - setMountedState(true); + mounted.current = true; + // Wondering why we use two RAFs? Check this video out: + // https://www.youtube.com/watch?v=cCOL7MC4Pl0 + // + // The useEffect() method is called after the DOM nodes are inserted. + // The UI might not have rendering the changes. We request a frame. + requestAnimationFrame(() => { + // The browser should be about to render the DOM nodes + // that React committed at this point. + // We don't want to interrupt. Let's wait the next frame. + requestAnimationFrame(() => { + // The UI is up-to-date at this point. + // We can continue rendering the children. + if (mounted.current) { + setMountedState(true); + } + }); + }); + + return () => { + mounted.current = false; + }; } + + return undefined; }, [defer]); // We need the Fragment here to force react-docgen to recognise NoSsr as a component. diff --git a/packages/material-ui/src/NoSsr/NoSsr.test.js b/packages/material-ui/src/NoSsr/NoSsr.test.js index f0ec0c2f2dc59b..fd140a20814de0 100644 --- a/packages/material-ui/src/NoSsr/NoSsr.test.js +++ b/packages/material-ui/src/NoSsr/NoSsr.test.js @@ -52,13 +52,18 @@ describe('', () => { }); describe('prop: defer', () => { - it('should defer the rendering', () => { + it('should defer the rendering', done => { const wrapper = mount( Hello , ); - assert.strictEqual(wrapper.find('#client-only').exists(), true); + assert.strictEqual(wrapper.find('#client-only').exists(), false); + setTimeout(() => { + wrapper.update(); + assert.strictEqual(wrapper.find('#client-only').exists(), true); + done(); + }, 300); }); }); });