Material-ui: [Modal] DisablePortal and Open=True SSR Issues

[lmuller18]

In a clean Gatsby project with only Material-UI installed, creating a dialog as such:

<div ref={modalRef}>
  <Dialog open keepMounted disablePortal container={modalRef}>
       This is dialog content. Portal is disabled, default opened, provided
       container.
  </Dialog>
</div>

gatsby build will fail since neither node or document exist in Modal.js:116:25.

• [x] This is not a v0.x issue.
• [x] I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior 🤔

Dialog is present in the DOM at compile time using gatsby build.

Current Behavior 😯

Build process fails:

 1 | function ownerDocument(node) {
> 2 |   return node && node.ownerDocument || document;
    | ^
  3 | }
  4 | 
  5 | export default ownerDocument;


  WebpackError: ReferenceError: document is not defined

  - ownerDocument.js:2 ownerDocument
    node_modules/@material-ui/core/esm/utils/ownerDocument.js:2:1

  - Modal.js:116 getDoc
    node_modules/@material-ui/core/esm/Modal/Modal.js:116:25

  - TrapFocus.js:41 
    node_modules/@material-ui/core/esm/Modal/TrapFocus.js:41:1

  - TrapFocus.js:36 TrapFocus
    node_modules/@material-ui/core/esm/Modal/TrapFocus.js:36:8

Steps to Reproduce 🕹

Link: https://codesandbox.io/embed/gatsby-starter-default-hnfqq

1. Initalize new Gatsby project.
2. Add @material-ui/core @material-ui/styles gatsby-plugin-material-ui
3. Create Dialog with props: open disablePortal
4. yarn build

Context 🔦

I am trying to render a pre-opened dialog using Gatsby so that when the project is compiled, the dialog and its content is present in the compiled HTML ahead of time instead of dynamically rendered through JS.

Your Environment 🌎

| Tech | Version |
|--------------|---------|
| Material-UI | v^4.3.0 |
| React | v^16.8.6 |
| Browser | Any |
| gatsby | v^2.13.42 |
| gatsby-plugin-material-ui | ^2.1.4 |

[oliviertassinari]

It seems that we had a couple of regressions since #14285. It has been broken for 2 months, probably the sign that it's not very important.

I would propose the following changes:

diff --git a/packages/material-ui/src/Modal/Modal.js b/packages/material-ui/src/Modal/Modal.js
index 98d22e672..a8638c489 100644
--- a/packages/material-ui/src/Modal/Modal.js
+++ b/packages/material-ui/src/Modal/Modal.js
@@ -112,6 +112,11 @@ const Modal = React.forwardRef(function Modal(props, ref) {
     }
   });

+  const isTopModal = React.useCallback(
+    () => manager.isTopModal(getModal(modal, mountNodeRef, modalRef)),
+    [manager],
+  );
+
   const handlePortalRef = useEventCallback(node => {
     mountNodeRef.current = node;

@@ -123,7 +128,7 @@ const Modal = React.forwardRef(function Modal(props, ref) {
       onRendered();
     }

-    if (open) {
+    if (open && isTopModal()) {
       handleMounted();
     } else {
       ariaHidden(modalRef.current, true);
@@ -148,11 +153,6 @@ const Modal = React.forwardRef(function Modal(props, ref) {
     }
   }, [open, handleClose, hasTransition, closeAfterTransition, handleOpen]);

-  const isTopModal = React.useCallback(
-    () => manager.isTopModal(getModal(modal, mountNodeRef, modalRef)),
-    [manager],
-  );
-
   if (!keepMounted && !open && (!hasTransition || exited)) {
     return null;
   }
diff --git a/packages/material-ui/src/Modal/TrapFocus.js b/packages/material-ui/src/Modal/TrapFocus.js
index 45ccc8ee8..9cb863a6f 100644
--- a/packages/material-ui/src/Modal/TrapFocus.js
+++ b/packages/material-ui/src/Modal/TrapFocus.js
@@ -36,7 +36,7 @@ function TrapFocus(props) {
   // ⚠️ You may rely on React.useMemo as a performance optimization, not as a semantic guarantee.
   // https://reactjs.org/docs/hooks-reference.html#usememo
   React.useMemo(() => {
-    if (!open) {
+    if (!open || typeof window === 'undefined') {
       return;
     }

Also, I think that a test case with:

import Modal from '@material-ui/core/Modal';

export default function App() {
  return (
    <Modal open disablePortal>
      <div>Server</div>
    </Modal>
  );
}

would help.

@lmuller18 Do you want to work on it? :)

[lmuller18]

I can start looking into implementing those changes tomorrow. Thank you for the pointers.

[oliviertassinari]

Awesome, if you need any help, you can join me on Gitter.