Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Popper.js document is not defined with SSR #18373

Closed
MisterQH opened this issue Nov 14, 2019 · 3 comments · Fixed by #18381
Closed

Popper.js document is not defined with SSR #18373

MisterQH opened this issue Nov 14, 2019 · 3 comments · Fixed by #18381
Labels
component: Popper The React component. See <Popup> for the latest version. good first issue Great for first contributions. Enable to learn the contribution process.

Comments

@MisterQH
Copy link
Contributor

I need help with SSR using the Popper component.

When I access to my page via a link from another page, the page is rendered as it should be.

But when I try to access the same page directly from its url, I get an HTTP error 500 and this error message:

Error running template: ReferenceError: document is not defined
    at flipPlacement (/Users/xxx/Documents/dev/epotek/microservices/www/node_modules/@material-ui/core/Popper/Popper.js:37:52)
    at Object.Popper (/Users/xxx/Documents/dev/epotek/microservices/www/node_modules/@material-ui/core/Popper/Popper.js:112:22)
    at ReactDOMServerRenderer.render (/Users/xxx/Documents/dev/epotek/microservices/www/node_modules/react-dom/cjs/react-dom-server.node.development.js:3758:44)
    at ReactDOMServerRenderer.read (/Users/xxx/Documents/dev/epotek/microservices/www/node_modules/react-dom/cjs/react-dom-server.node.development.js:3538:29)
    at renderToString (/Users/xxx/Documents/dev/epotek/microservices/www/node_modules/react-dom/cjs/react-dom-server.node.development.js:4247:27)
    at Promise.asyncApply (imports/startup/server/ssr-server.js:33:5)
    at /Users/xxx/.meteor/packages/promise/.0.11.2.c7kmck.hukra++os+web.browser+web.browser.legacy+web.cordova/npm/node_modules/meteor-promise/fiber_pool.js:43:40
@oliviertassinari
Copy link
Member

oliviertassinari commented Nov 14, 2019

This concern is related to #15798 (comment). In this context, I would suggest this diff:

diff --git a/packages/material-ui/src/Popper/Popper.js b/packages/material-ui/src/Popper/Popper.js
index 80f5f9909..da7f77e6c 100644
--- a/packages/material-ui/src/Popper/Popper.js
+++ b/packages/material-ui/src/Popper/Popper.js
@@ -13,7 +13,7 @@ import ownerWindow from '../utils/ownerWindow';
  * @param {string} placement
  */
 function flipPlacement(placement) {
-  const direction = (typeof window !== 'undefined' && document.body.getAttribute('dir')) || 'ltr';
+  const direction = (typeof document !== 'undefined' && document.body.getAttribute('dir')) || 'ltr';

   if (direction !== 'rtl') {
     return placement;

Would it help in your case? (you likely shouldn't define a window on the server).
Alternatively, we could start to use canUseDOM, this would probably be a more resilient solution.

@oliviertassinari oliviertassinari added component: Popper The React component. See <Popup> for the latest version. good first issue Great for first contributions. Enable to learn the contribution process. labels Nov 14, 2019
@MisterQH
Copy link
Contributor Author

Yes thank you that fixed my issue !

@eps1lon
Copy link
Member

eps1lon commented Nov 15, 2019

I'm a bit concerned about branching this since it makes searching for these branches in the codebase very hard. Could you explain in which environment window is defined but not document?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: Popper The React component. See <Popup> for the latest version. good first issue Great for first contributions. Enable to learn the contribution process.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants