facebook / react

The library for web and native user interfaces.
https://react.dev
MIT License
228.71k stars 46.81k forks source link

document is not defined on server #3620

Closed bishtawi closed 7 years ago

bishtawi commented 9 years ago

I saw there was a bug for this a year ago (facebook/react#1866) and it was fixed. I am encountering it now on 0.13.1.

I am running react on the server to generate html. On one of my parent components I have a variable whose value cannot be determined until it reaches some child component. So I need to implement some way for a child component to pass back data to its parent. I read https://facebook.github.io/react/tips/communicate-between-components.html and I implemented it:

Parent component passes callback function as a prop to child component. Child component calls callback in componentWillMount function (also tested calling callback in getInitialState). Callback calls setState to update state variable in parent.

This does not work serverside as an exception is thrown about document not being defined.

tmbtech commented 9 years ago

There's no DOM when SSR, you will have to add some kind of abstraction to test if the DOM is present.

https://gist.github.com/tmbtech/92522d315d50ea5281d1

bishtawi commented 9 years ago

So there is no way to pass data up to a parent component serverside? That state variable in the parent component needs to be set in order for the page to be generated properly. Hence the reason why I need it to happen serverside and not just on the client.

jimfb commented 9 years ago

Parents and children can still communicate in the normal way (callbacks, but you need to do it in the right places, because some lifecycle methods (including componentDidMount) won't be called on the server side), but the fact that you're relying on a child for information seems to imply you need to know something about the child's layout/size/etc. The blocking issue for you is going to be that... if you're relying on the browser to figure out the component's layout/size, that information won't be available to you on the server side (because you don't have the browser).

Or is the exception being thrown from within React? If it's thrown from within React, that sounds like it might be a bug. Can you provide a piece of example code that crashes?

bishtawi commented 9 years ago

Well, my use case is creating breadcrumbs. I am using react-router and in my layout component I am rendering the breadcrumbs. But I dont know what breadcrumbs are until the page components (child component of the layout component) is created.

bishtawi commented 9 years ago

I already deleted most of the code and implemented a workaround, but my code originally looked something like this:

react-router:

<Route name="app" path="/" handler={ require('./components/layout') }>
  <DefaultRoute handler={require('./components/home')}/>
  <Route name='About Us' path="/about-us" handler={require('./components/about')}/>
  /* more routes */
</Route>

layout:

getInitialState: function() {
  return {breadcrumbs: []};
},
breadCrumbCallback: function(bc) {
  this.setState({breadcrumbs: bc});
},
render: function() {
    if (this.state.breadcrumbs.length) {
      var breadcrumbs = /* create breadcrumbs html from breadcrumbs state variable*/
    }
    return (
      <div>
        {breadcrumbs}
        <RouteHandler breadCrumbCallback={this.breadCrumbCallback}/>
      </div>
    )
}

Then in each of my page components (home, about, etc), I call breadCrumbCallback:

componentWillMount: function() {
  this.props.breadCrumbCallback(/*Page specific breadcrumbs array*/)
}

Is there something I am doing wrong?

tmbtech commented 9 years ago

I think you can use this.context.router to figure out the breadcrumbs in the parent container.

https://github.com/rackt/react-router/blob/master/docs/api/RouterContext.md https://github.com/rackt/react-router/issues/404 -- react-router breadcrumbs

bishtawi commented 9 years ago

Yeah, thats actually the workaround I am using right now. But, correct me if I am wrong, this is still a bug in react, no?

zpao commented 9 years ago

You only have a single render pass on the server so abstractly this pattern doesn't really make sense on the server. You're parent has already rendered so effectively you're trying to kick off a 2nd render when you call a function that sets state in the parent. It does seem like we shouldn't throw in such a bad way here though. I bet we can blame @sebmarkbage for the changes he made to the state queue...

jimfb commented 9 years ago

@bishtawi I'm not convinced it's a bug in React. For SSR, I can think of a bunch of reasons to want to enforce single-pass rendering (if nothing else, it keeps open the possibility of streaming the output to the client). I can't think of any (good) reason to have a child pass that kind of information back up to the parent (your use case sounds like an antipattern to me). So maybe the bug is that the error message isn't the best, but the behavior is probably correct.

Going to close the issue, feel free to continue the discussion here.

sebmarkbage commented 9 years ago

We've determined that callbacks up to parents from all life cycle methods is typically an anti-pattern. It actually opens up a lot of strange timing issues. (For example ref resolution order changed in 0.13 so refs of a parent is not resolved when a child's componentDidMount is called.)

@jsfb, we can't proclaim that it's an anti-pattern without providing an alternative solution. We should also fix the error message if we don't want to support this.

jimfb commented 9 years ago

@sebmarkbage I thought @tmbtech did provide an alternate solution, no?

Until we start handling layout internally (instead of deferring to the browser), my intuition is that we don't want to support this. Fixing the error message makes sense, but should maybe be a new/different issue?

zpao commented 9 years ago

The alternative solution presented makes use of something specific to react-router.

jimfb commented 9 years ago

Well, the general solution is to have the parent do whatever calculation the child is doing (possibly by calling into a shared library). Outside of layout, the parent should have all the information that the child has (since the child presumably got it from the parent). The react-router, in this case, is the shared code/data, but that same solution is a general solution.

sebmarkbage commented 9 years ago

One neat way to do this is to have a static method on the child so you can call. ChildComponent.getBreadcrumb(this.props) before rendering it. The ChildComponent can forward the call deeper if it needs to.

Neither of these solutions solve the case when the parent and child are vastly detached from each other and passing info through the intermediate abstractions is prohibitive. It is the exact same problem as layout. That's what prerender was suppose to solve. https://github.com/reactjs/react-future/blob/master/04%20-%20Layout/02%20-%20Layout%20Components.js#L15

wied03 commented 9 years ago

I'm having the same issue with 0.13.3 and I'm not passing callbacks around. I'm just trying to setState using an externally fetched value from within componentWillMount and I'm encountering the issue

jimfb commented 9 years ago

@wied03 Is there a reason you are using componentWillMount instead of the constructor or getInitialState? Also, can you provide a jsfiddle that demonstrates the issue?

sebmarkbage commented 9 years ago

I noticed similar issues in workers. We should fix these for 0.14.

wied03 commented 9 years ago

@jimfb I thought it was failing in a different place than it was. As it turns out, I'm trying to do the same thing the original poster is. Sorry for the noise. As to why not initial_state or a constructor, it's a simpler programming model this way. I have cascading dropdowns and can have the same code, which makes piecemeal calls to setState, deal with an initial preselections and also react to valueLink requestChange events. Using initial state requires that I combine it all into piece of code and pass the "candidate" state around.

sophiebits commented 9 years ago

@sebmarkbage Can you be more specific?

sebmarkbage commented 9 years ago

@spicyj I found it! https://github.com/facebook/react/blob/52752446760dee0bc7232b4146f5a309ac57f065/src/shared/vendor/core/dom/getActiveElement.js#L23

sebmarkbage commented 9 years ago

The stack is:

getActiveElement ReactInputSelection.getSelectionInformation Mixin.initializeAll Mixin.perform batchedMountComponentIntoNode

This should not happen on the server though

ericclemmons commented 9 years ago

Someone pointed me to this thread, so I'd like to chime in on a use-case...

I've been doing a rewrite of react-resolver to avoid several context issues in v0.13, and it is heavily dependent on setState on the server:

  1. Desired root element is rendered normally via React.renderWhatever to kick off the lifecycle.
  2. Containers wrap lazy-loaded components & fetch their data via Promises on componentWillMount.
  3. Data is returned, and setState is called within the Container after Promises resolve.
  4. The wrapped Component renders, kicking off steps 2-4 all over again.
  5. Once the tree completes, the Resolver aggregates all lazy-loaded data.
  6. A final React.renderWhatever is called, but this time the data is already available & the full tree renders synchronously.

The alternative would be to constantly re-render the full tree from the root until no more promises appear, but that seems poor architecturally, as then we have 2 different algorithms for both server & client rather than a "universal" (can't say "isomorphic" without @mjackson knowing about it! :D) one, since setState is neutered on the server.

andreimc commented 9 years ago

Any news on this issue how can I work around it ?

ericclemmons commented 9 years ago

@andreimc In both using React v0.13 & React v0.14 (beta) on the server, I've found it's wise to move any setState logic within componentWillMount to constructor.

Otherwise I've seen both:

andreimc commented 9 years ago

thanks @ericclemmons

mik01aj commented 9 years ago

It was bugging me as well, but from what I understand, the commit that fixed this was merged to master, and will be released in v0.14, and I can already use it by installing version 0.14.0-beta3 from npm. At least, I tried the update and it helped. I guess this ticket could be closed then.

ericclemmons commented 9 years ago

I am confuse. The commit you linked isn't for React proper. It's kinda bad for user libs to have to hack around setState in componentWillMount. IMO, either there should be deprecation warnings for users to move their logic from componentWillMount to constructor, or adjust setState to work as expected.

sophiebits commented 9 years ago

@ericclemmons Yes, this is still a React issue.

mik01aj commented 9 years ago

Ah, @ericclemmons, you're right, I didn't notice that! Now I'm confused too. I upgraded React to 0.14.0-beta3 because I saw this comment and this seemed to fix the problem for me.

sophiebits commented 9 years ago

Hm, it's possible 40b7c19a890b04e5d7bd1273e4f90bb78769ae5b fixed it.

ericclemmons commented 9 years ago

Good find! Looks like that'll make it into the next release...

thinkkevin commented 9 years ago

I installed the 0.14-rc1, but still got this issue.

::ffff:127.0.0.1 - - [29/Sep/2015:05:58:09 +0000] "GET /v1/article/236 HTTP/1.1" 200 - "-" "-"
/Users/qinguo/allcodes/gitebay/NewX/go/node_modules/react/lib/getActiveElement.js:23
    return document.body;
           ^

ReferenceError: document is not defined
    at getActiveElement (/Users/qinguo/allcodes/gitebay/NewX/go/node_modules/react/lib/getActiveElement.js:23:12)
    at ReactReconcileTransaction.ReactInputSelection.getSelectionInformation (/Users/qinguo/allcodes/gitebay/NewX/go/node_modules/react/lib/ReactInputSelection.js:40:23)
    at ReactReconcileTransaction.Mixin.initializeAll (/Users/qinguo/allcodes/gitebay/NewX/go/node_modules/react/lib/Transaction.js:168:30)
    at ReactReconcileTransaction.Mixin.perform (/Users/qinguo/allcodes/gitebay/NewX/go/node_modules/react/lib/Transaction.js:133:12)
    at ReactUpdatesFlushTransaction.Mixin.perform (/Users/qinguo/allcodes/gitebay/NewX/go/node_modules/react/lib/Transaction.js:134:20)
    at ReactUpdatesFlushTransaction.assign.perform (/Users/qinguo/allcodes/gitebay/NewX/go/node_modules/react/lib/ReactUpdates.js:95:38)
    at Object.flushBatchedUpdates (/Users/qinguo/allcodes/gitebay/NewX/go/node_modules/react/lib/ReactUpdates.js:175:19)
    at Object.wrapper [as flushBatchedUpdates] (/Users/qinguo/allcodes/gitebay/NewX/go/node_modules/react/lib/ReactPerf.js:70:21)
    at ReactDefaultBatchingStrategyTransaction.Mixin.closeAll (/Users/qinguo/allcodes/gitebay/NewX/go/node_modules/react/lib/Transaction.js:207:25)
    at ReactDefaultBatchingStrategyTransaction.Mixin.perform (/Users/qinguo/allcodes/gitebay/NewX/go/node_modules/react/lib/Transaction.js:148:16)
    at Object.ReactDefaultBatchingStrategy.batchedUpdates (/Users/qinguo/allcodes/gitebay/NewX/go/node_modules/react/lib/ReactDefaultBatchingStrategy.js:66:19)
    at Object.enqueueUpdate (/Users/qinguo/allcodes/gitebay/NewX/go/node_modules/react/lib/ReactUpdates.js:215:22)
    at enqueueUpdate (/Users/qinguo/allcodes/gitebay/NewX/go/node_modules/react/lib/ReactUpdateQueue.js:30:18)
    at Object.ReactUpdateQueue.enqueueSetState (/Users/qinguo/allcodes/gitebay/NewX/go/node_modules/react/lib/ReactUpdateQueue.js:208:5)
    at Article.ReactComponent.setState (/Users/qinguo/allcodes/gitebay/NewX/go/node_modules/react/lib/ReactComponent.js:69:20)
    at Article.<anonymous> (/Users/qinguo/allcodes/gitebay/NewX/go/lib/shared/mixin/request.js:35:14)
    at Request.self.callback (/Users/qinguo/allcodes/gitebay/NewX/go/node_modules/request/request.js:198:22)
    at emitTwo (events.js:87:13)
    at Request.emit (events.js:172:7)
    at Request.<anonymous> (/Users/qinguo/allcodes/gitebay/NewX/go/node_modules/request/request.js:1073:14)
    at emitOne (events.js:82:20)
    at Request.emit (events.js:169:7)
    at IncomingMessage.<anonymous> (/Users/qinguo/allcodes/gitebay/NewX/go/node_modules/request/request.js:1019:12)
    at emitNone (events.js:72:20)
    at IncomingMessage.emit (events.js:166:7)
    at endReadableNT (_stream_readable.js:893:12)
    at doNTCallback2 (node.js:429:9)
    at process._tickCallback (node.js:343:17)
Rleahy22 commented 9 years ago

Using 0.14 still seeing this issue. I'm using document.querySelectorAll because I need to pass dom nodes as part of props, but I get:

ReferenceError: Error with partial rendering: Error with partial rendering: document is not defined

borm commented 8 years ago

How fix it?

ericclemmons commented 8 years ago

Personally, I haven't seen this error in ages. So if anyone on this thread is still having an issue, please provide an example repo so we can actually test it. Otherwise, we're guessing as to what 99% of what your app is doing.

dreambo8563 commented 8 years ago

@ericclemmons , I got one.. I am using react start kit. I import react echart as an component and ensure.require the component. then .... got this error on server side. my repo https://github.com/dreambo8563/RSK/blob/master/src/routes/content/index.js

will see the error:

ReferenceError: document is not defined
    at detect (D:\Projects\RSK-master\node_modules\.3.1.3@zrender\lib\core\env.js:104:31)
    at Object.<anonymous> (D:\Projects\RSK-master\node_modules\.3.1.3@zrender\lib\core\env.js:21:15)
    at Module._compile (module.js:556:32)
    at Object.Module._extensions..js (module.js:565:10)
    at Module.load (module.js:473:32)
    at tryModuleLoad (module.js:432:12)
    at Function.Module._load (module.js:424:3)
    at Module.require (module.js:483:17)
    at require (internal/module.js:20:19)
    at Object.<anonymous> (D:\Projects\RSK-master\node_modules\.3.2.3@echarts\lib\echarts.js:30:15)
    at Module._compile (module.js:556:32)
    at Object.Module._extensions..js (module.js:565:10)
    at Module.load (module.js:473:32)
    at tryModuleLoad (module.js:432:12)
    at Function.Module._load (module.js:424:3)
    at Module.require (module.js:483:17)

add more info: if I just import the component at the top of the router, no error there

aweary commented 7 years ago

It's been over a year and there have been no reports of this issue occurring anymore. There is also no clear example that we could use to reproduce this issue in the latest versions of React. I'm going to close this out under the assumption that https://github.com/facebook/react/commit/40b7c19a890b04e5d7bd1273e4f90bb78769ae5b or some other commit from that period resolved this issue.

If you're still seeing this issue, please provide a small repro that we can verify. Thanks!

kvedantmahajan commented 6 years ago

I am facing this issue in React 16. Seemingly cannot find any solution when rendered from server. Without server render it used to work fine.

Sharing my two snippets - server/index.js and src/index.js (client )

import express from 'express';
import serverRenderer from './middleware/renderer';

const PORT = 3001;
const path = require('path');
const app = express();
const router = express.Router();
import Loadable from 'react-loadable';

router.use('^/$', serverRenderer);
app.use('/static', express.static(path.join(__dirname, 'assets')));
router.use(express.static(
    path.resolve(__dirname, '..', 'build'),
    { maxAge: '30d' },
));
router.use('*', serverRenderer);
app.use(router);
Loadable.preloadAll().then(() => {
    app.listen(PORT, (error) => {
        if (error) {
            return console.log('something bad happened', error);
        }

        console.log("listening on " + PORT + "...");
    });
});

renderer.js

import React from 'react'
import ReactDOMServer from 'react-dom/server';
import { Provider } from 'react-redux';
import { ConnectedRouter } from 'react-router-redux';
import store from '../../src/store';
import {createMemoryHistory } from 'history';
import { StaticRouter } from 'react-router'
// import App from '../../src/containers/app';
import { ServerStyleSheet } from 'styled-components'

// import our main App component
import App from '../../src/index';
const path = require('path');
const fs = require('fs');
const history = createMemoryHistory({
  initialEntries: ['/', '/next', '/last'],
  initialIndex: 0
});

export default (req, res, next) => {
    const filePath = path.resolve(__dirname, '..', '..', 'build', 'index.html');
    fs.readFile(filePath, 'utf8', (err, htmlData) => {
        if (err) {
            console.error('err', err);
            return res.status(404).end()
        }
        const sheet = new ServerStyleSheet();
        const body = ReactDOMServer.renderToString(
            sheet.collectStyles(<Provider store={store}>
              <ConnectedRouter history={history}>
                <App />
              </ConnectedRouter>
            </Provider>)
        );
        const styleTags = sheet.getStyleTags() // or sheet.getStyleElement()
        var htmlData = htmlData.replace(
                '</head>',
                `${styleTags}</head>`
            );
        var htmlData = htmlData.replace(
                '<div id="root"></div>',
                `<div id="root">${body}</div>`
            );
        return res.send(htmlData);
    });
}

This above works fine if I use containers/app.js (with no document inside) instead of src/index.js.

src/index.js

import ReactDOM from 'react-dom';
import './index.css';
import store, { history } from './store';
import App from './containers/app';

const target = document.querySelector('#root');

ReactDOM.hydrate(
      <App />,
      target
);

Using the above file won't work from server and throws the same error

gaearon commented 6 years ago

You’re writing in a very old issue that has been fixed. Whatever problem you’re having with React 16 is unrelated. Please file a new issue with a minimal reproducing example for your problem. Thanks!