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 side rendered open modal #1071

Open
ruszki opened this issue Jun 15, 2018 · 15 comments
Open

Server side rendered open modal #1071

ruszki opened this issue Jun 15, 2018 · 15 comments

Comments

@ruszki
Copy link

ruszki commented Jun 15, 2018

Issue description

  • components: Modal
  • reactstrap version 5.0.0, but the problematic code still exists in latest version
  • import method: es with Webpack
  • react version 16.2.0
  • bootstrap version 4.0.0

What is happening?

When an open modal is server side rendered, then it is failing with "document is not defined". The exception is thrown in Modal's init method.

What should be happening?

Render as it should. Probably move checking of isOpen, and calling of init to componentDidMount from constructor.

Steps to reproduce issue

  1. Create a modal with isOpen true.
  2. Try render it with React's renderToString.

Error message in console

ReferenceError: document is not defined
    at Modal.init (...)
    at new Modal (...)
    at resolve (...)
    at ReactDOMServerRenderer.render (...)
    at ReactDOMServerRenderer.read (...)
    at renderToString (...)

Code

import {renderToString} from "react-dom/server";

renderToString(
    <Modal isOpen>
        <ModalBody>Lorem ipsum</ModalBody>
    </Modal>);
@virgofx
Copy link
Collaborator

virgofx commented Jun 15, 2018

I would say that Reactstrap works just about every with SSR; however, this is one of the cases where it will fail. I've come across this as well in my testing.

The server can't do the positioning components here (and later as well), and Portal portion.

The only solution would be to have it fall through gracefully on the server, but the client will pick it up with {true} and then automatically open .

So from a true SSR -- it's not accurate since the modal won't be in the correct position upon load.

Not sure if this is an acceptable solution or not. I'd say it's fine as opening modal's typically are done as result of an action; however, if there was a page transition that needed a modal opened ... and the server re-rendered, I'm sure it wouldn't be bad to have it just slide in (opposed to already existing in place in the DOM -- no animation and in correct location -- it just can't be done easily because of how React.createPortal works)

Let me know your thoughts on the proposed workaround (e.g. SSR fall through with immediate open on load) . If that's okay and I'll take care of this one.

For true SSR ... it's quite ugly (and not worth the effort in my opinion) The portals need to pushed into a stack and subsequently later rerendered (more work by the server to keep track of these and adds to uglier code in Reactstrap to expose refs to all the portals).

@jordan-jarolim
Copy link

Same issue here! It would be great to have a possibility of opened modal on load/init.

@virgofx
Copy link
Collaborator

virgofx commented Jul 31, 2018

Just need feedback ... everyone okay with it opening immediately via componentDidMount ???

/cc @TheSharpieOne

Effectively, this allows SSR to not BREAK which is important ... and still provides the same value 99.9%

Only downside is that the true HTML sent down from server wouldn't match the opened HTML (not really a downside just if someone cared 100% about showing the modal content being open/in correct DOM location in source -- who needs that/cares?)

It would not be noticeable and look better as when loading the page it would slide the modal down instead of already be open (no transition).

If so I'll merge fix for this ... this weekend.

@TheSharpieOne
Copy link
Member

@virgofx
Copy link
Collaborator

virgofx commented Jul 31, 2018

@TheSharpieOne That library is one of many techniques as mentioned in my above comment. We'll need to implement our solution backend agnostic (the one mentioned is very specific to node and includes a specific reference for injecting into a Node backend) --- We would have to modify those parts slightly.

Backends = NodeJs via Express, PHP via V8JS, ASP via ReactJS.NET, etc.

In general, we would have to mimic react-portal-universal functionality and provide some escape hatches to render the Modal portals separately (which we could do -- my thought is that most people that are running SSR are less likely simply executing static code and more likely hydrating on the client and thus the simplest solution would be to perform in componentDidMount()). While you could render a modal on SSR as static (not hydrating) no click handlers would work so you couldn't close it. I'm not sure how many people would do this -- it makes more sense to only statically render static components (Modals are really not one of them).

Otherwise, we would have to store all the portals specifically rendered via Reactstrap and then provide a separate hook to retrieve those ... such that the backend provider could inject as necessary.

So again .. my opinion ... in order to keep the code base as light as possible without adding extra bloat is to just allow SSR modals to work when hydrating.

@TheSharpieOne
Copy link
Member

Yeah, that makes sense. Lets go with the lighter hydrating method

@virgofx
Copy link
Collaborator

virgofx commented Aug 1, 2018

Sounds good. Will reference this issue with PR (hopefully this weekend)

@elislusarczyk
Copy link

@virgofx Is there any update on this?

@virgofx
Copy link
Collaborator

virgofx commented Oct 15, 2018

Sorry not yet ... been super busy should only be a quick fix to migrate the opening portion to trigger client side ... give me another week should have it done unless you want to submit a PR ... always welcome as well.

@seeden
Copy link

seeden commented Feb 15, 2019

any update?

@lh0x00
Copy link
Contributor

lh0x00 commented May 22, 2019

resolved at #1495

@boutdemousse
Copy link

This problem is still here (reactstrap 8.0.0) but the fix is quite simple :

<Modal isOpen={typeof(document) !== 'undefined' ? true : false}>
</Modal>

And the error with isOpen set to true is :

ReferenceError: document is not defined
    at Modal.init (/home/user/project/node_modules/reactstrap/lib/Modal.js:272:23)

@lh0x00
Copy link
Contributor

lh0x00 commented Jul 11, 2019

hi @boutdemousse, version 8.0.0 is not included PR's fix this problem. Should you update to version 8.0.1 just released by @TheSharpieOne at ce2a9a0

@lh0x00
Copy link
Contributor

lh0x00 commented Jul 13, 2019

@TheSharpieOne should close this issue?

@Vadorequest
Copy link

Using Next.js, this issue crashes the whole app when loading a Tooltip that has isOpen={true} on the server.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants