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

Server rendering breaks #2013

Closed
ghost opened this issue Oct 30, 2015 · 21 comments · Fixed by #2021
Closed

Server rendering breaks #2013

ghost opened this issue Oct 30, 2015 · 21 comments · Fixed by #2021
Labels
component: pickers This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation

Comments

@ghost
Copy link

ghost commented Oct 30, 2015

In my code I added

import {DatePicker} from 'material-ui';

Then i've got error
ReferenceError: document is not defined
at [object Object].componentWillMount (~/node_modules/material-ui/lib/overlay.js:64:34)

I also change my import to
const DatePicker = require('material-ui/lib/date-picker/date-picker');

Still no luck

@mgibeau
Copy link
Contributor

mgibeau commented Oct 30, 2015

Getting this error too with the latest release, hash that introduced this
d25d122#diff-26176e848c2cae459bdd7e0ce928752b

@oliviertassinari
Copy link
Member

Any idea on how to fix it?

@mgibeau
Copy link
Contributor

mgibeau commented Oct 30, 2015

Hard to say without having the context of was the issue, but reverting to componentDidMount works on my end. @mmckelvy Can you validate?

@mmckelvy
Copy link
Contributor

@mimcabrera @mgibeau Odd that document would be undefined. Are you by any chance rendering on the server?

@oliviertassinari
Copy link
Member

@mmckelvy @mimcabrera @mgibeau Could you have a look at #2021?

@izziaraffaele
Copy link
Contributor

Still getting this error but in a different place (overlay.jsx).... Please guys don't close the issue if your are not sure it's solved and merged. You just confuse people

@oliviertassinari
Copy link
Member

@izziaraffaele Is this the same error? If not, could you open a new issue with the log so that we can have a look. Thank.

@izziaraffaele
Copy link
Contributor

The issue is always the same... you use document in componentWillMount which brakes server side rendering since the DOM is not available on server side.

in overlay.jsx

componentWillMount() {
    this._originalBodyOverflow = document.getElementsByTagName('body')[0].style.overflow;
  },

@oliviertassinari
Copy link
Member

I see, this is not from the master branch. See https://github.com/callemall/material-ui/blob/master/src/overlay.jsx#L57.

@izziaraffaele
Copy link
Contributor

Exactly what I meant... I did npm installl from scratch, got the error "document not defined", found the problem on componentWillMount, I came here to report the issue but I saw you already solved the problem and close the issue.

Remember that what people get is this https://github.com/callemall/material-ui/blob/v0.13.1/src/overlay.jsx, so even if you fix the issue keep it open till you'll merge those changes in the npm package.

@oliviertassinari
Copy link
Member

@izziaraffaele I see your point. Well, It's a matter of convention. Do we close an issue once it's merged to master or once it's released? IMO we should do it once it's merged to master.

@oliviertassinari
Copy link
Member

I did npm installl from scratch, got the error "document not defined"

Sorry for that. @shaurya947 Can we do a hotfix release?

@izziaraffaele
Copy link
Contributor

How many people are using your master branch? how many popole are using your stable release instead? For how many people this is still an issue?

It doesn't matter if you solve it in the master branch, in you local machine or in the branch XYZ... Is it an issue for most people using your package? Then it should stay open...

@oliviertassinari
Copy link
Member

@izziaraffaele I disagree. I'm following the same convention as facebook with react. For instance facebook/react#5337.
The other convention may be better, let's ask @hai-cea or @shaurya947 their point of view.

@izziaraffaele
Copy link
Contributor

@oliviertassinari not sure if it's a "convention" or it's just what they did in that case ( BTW it's a feature not a bug ;) ). I would like to read some more info about it if you have others.

Anyways it's up to you.... I just think that the other approach lead to lot less questions, duplicated issues and headache for people using your package.

@oliviertassinari
Copy link
Member

@izziaraffaele Alright, since it's not released and people keep opening issue for this, I'm reoponing the issue until we release the fix. (I can't)

@oliviertassinari oliviertassinari changed the title DatePicker component does not work. Server rendering breaks Nov 5, 2015
@shaurya947
Copy link
Contributor

@oliviertassinari Let's keep this issue open. We should release early next week and then we can close this.

@liviuignat
Copy link

Hi guys,

the fix for this issue is to move it on componentDidUpdate, and it works correctly with server side rendering also.

  componentWillMount: function componentWillMount() {
    //this._originalBodyOverflow = document.getElementsByTagName('body')[0].style.overflow;
  },

  componentDidUpdate: function componentDidUpdate() {
    this._originalBodyOverflow = document.getElementsByTagName('body')[0].style.overflow;
    if (this.props.autoLockScrolling) {
      if (this.props.show) {
        this._preventScrolling();
      } else {
        this._allowScrolling();
      }
    }
  }

@rewop
Copy link

rewop commented Nov 9, 2015

HI guys, I came across the same issue while trying the framework. +1 to solve this one asap.

@shaurya947
Copy link
Contributor

Releasing soon, should fix this..

@oliviertassinari
Copy link
Member

The fix was release with v0.13.2.

@zannager zannager added docs Improvements or additions to the documentation component: pickers This is the name of the generic UI component, not the React module! labels Dec 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: pickers This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants