Skip to content

Conversation

liseki
Copy link
Contributor

@liseki liseki commented Nov 23, 2015

Handle pseudo-browser environments when rendering server-side where window may be defined as the global context like in react-rails

@STRML
Copy link
Collaborator

STRML commented Nov 23, 2015

Wow - this seems like a bug in react-rails. It shouldn't be mocking the window object for this exact reason.

If it's going to mock window and not provide any of its methods, what is the point?

@liseki liseki force-pushed the server-side-rendering-fix branch from 1cde289 to 5703e3d Compare November 24, 2015 08:09
@liseki
Copy link
Contributor Author

liseki commented Nov 24, 2015

Sorry for initially breaking a test, it seems fine now.

@STRML I absolutely agree with you on that. Looking at the original patch that issue was raised but never addressed albeit after the commit. Looking at the discussion this was partly done to make it play happy with browserify. Considering all that is this small patch acceptable or will I have to monkey patch react-rails?

@STRML
Copy link
Collaborator

STRML commented Nov 24, 2015

I suppose it is fine if it fixes your use case, but I very much disagree with what react-rails did here. Could you add a comment (so it doesn't get deleted later)? I'll merge it then.

@liseki liseki force-pushed the server-side-rendering-fix branch from 5703e3d to 217631b Compare November 24, 2015 17:34
@liseki
Copy link
Contributor Author

liseki commented Nov 24, 2015

Thanks @STRML, I think that should be ready to merge now!

@STRML
Copy link
Collaborator

STRML commented Nov 24, 2015

Thanks!

STRML added a commit that referenced this pull request Nov 24, 2015
@STRML STRML merged commit 4a656b7 into react-grid-layout:master Nov 24, 2015
@liseki liseki deleted the server-side-rendering-fix branch November 25, 2015 05:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants