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

Incompatible with server-side rendering #15

Closed
misuba opened this issue Jan 6, 2015 · 12 comments
Closed

Incompatible with server-side rendering #15

misuba opened this issue Jan 6, 2015 · 12 comments

Comments

@misuba
Copy link
Contributor

misuba commented Jan 6, 2015

A source tree with react-modal in it, running in Node, yields the following:

/node_modules/react-modal/lib/components/Modal.js:18
    appElement: React.PropTypes.instanceOf(HTMLElement),
                                           ^
ReferenceError: HTMLElement is not defined

...and halts entirely.

How would you feel about making this propType a little more lenient (object, perhaps)?

@ryanflorence
Copy link
Contributor

How about including this into your app?

HTMLElement = typeof HTMLElement === 'undefined' ? function(){} : HTMLElement;
var appElement = ENV.CLIENT ?
  document.getElementById('appElement') :
  new HTMLElement();

@misuba
Copy link
Contributor Author

misuba commented Jan 6, 2015

I could do that (plus shim document.body.etc - HTMLElement is just the first choke point), but I'd have to run a hacked version of this package rather than just put that in my app, and I'd prefer not to do that.

I could also possibly just require react-modal on the client-side and pass either it or a server-side alternative element all the way down my app hierarchy, but... come on. Plenty of people are running React trees in Node; it'd be nice to have a modal library that didn't require all that ducking and weaving.

@nelix
Copy link

nelix commented Jan 19, 2015

I had a little peek at wrapping the offending bits in ExecutionEnvironment.canUseDOM but I could not really think up a good way to do portals without the dom.

@misuba
Copy link
Contributor Author

misuba commented Jan 19, 2015

Could you post a gist? I'm pretty sure I don't need the things to work on the server... I just need them to not crash the server.

@nelix
Copy link

nelix commented Jan 20, 2015

@misuba this is crude and untested, just designed to give you the idea, https://gist.github.com/nelix/24dd21b12d4b005880aa

you could also not mess with react-modal at all and just render null instead if the dom is not available, but I think we could come up with a server render alternative that mostly works instead of these hacks

@ncuillery
Copy link

With the @nelix 's gist augmented with the @ryanflorence 's workaround (HTMLElement = typeof HTMLElement === 'undefined' ? function(){} : HTMLElement;), it doesn't crash on server and work well in the browser.

Can we considered the issue as solved and make a new release ? :)

@cmeiller
Copy link

👍

@fdubost
Copy link

fdubost commented Feb 19, 2015

@ncuillery 👍

1 similar comment
@oziks
Copy link

oziks commented Feb 19, 2015

@ncuillery 👍

@XeeD
Copy link

XeeD commented Apr 2, 2015

I wonder if the work from BedrockStreaming#1 could be merged into this repository? I've just tried it and it works fine when rendering React on server for me.

@ncuillery
Copy link

Yes, something similar has already been done here by @misuba : #22

@mzabriskie
Copy link
Member

Resolved with #22

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

No branches or pull requests

9 participants