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 should be distinct from client-rendering #1979

Closed
syranide opened this issue Aug 2, 2014 · 22 comments
Closed

Server-rendering should be distinct from client-rendering #1979

syranide opened this issue Aug 2, 2014 · 22 comments

Comments

@syranide
Copy link
Contributor

syranide commented Aug 2, 2014

Kind of related to #1978 (concerns polyfills)

It seems to me that we should really treat server-rendering as distinct from client-rendering, they have different requirements and priorities.

  1. Server-rendering should be able to generate optimal markup for SEO, markup must also be fully cross-browser compatible and small markup is preferable.
  2. Client-rendering sees SEO as largely irrelevant, full cross-browser compatibility is also irrelevant and rendering performance is of greater importance.

So I propose that we treat server-rendering as uniquely distinct from client-rendering, e.g. as a separate flagReact.serverSide = true (which would default to true if document is not available). It's even imaginable that we could extend this with e.g. React.targetClient = 'SEO' / 'legacyClient' / 'everGreenClient', which would optionally allow the markup to be tailored for a specific purpose.

So when reusing server-rendered markup client-side:

  1. The client should do an initial server-rendering pass being passed the exact same props (and time) as during the server-side rendering, but instances are actually allocated on the client.
  2. Immediately afterwards a client-rendering pass is done with any props (and current time) you like, instances/markup is efficiently reused and should not add significant costs as little DOM manipulation should be involved.

I'm not really affected by this, but for those that do use server-rendering; this seems like the correct way of approaching it (as opposed to just rendering everything exactly the same). However, I'm working on img- and picture-element polyfills, but without this they cannot be made to generate sensible server-side markup.

PS. I realize this can be done today using a global + mixin, but without a standardized solution, it's unlikely that third-party (and polyfill) components could ever participate in this. But that doesn't mean that it needs to be shipped with React even (none of this requires core support I believe), as long as it's standardized.

@sophiebits
Copy link
Collaborator

It's unclear to me what you're proposing. We already have ReactServerRenderingTransaction internally which makes a few things behave differently when doing server rendering. What else were you thinking might differ?

@syranide
Copy link
Contributor Author

syranide commented Aug 4, 2014

The way server-rendering is handled internally does not allow for different nodes to be generated, only different attributes. What I'm proposing is exposing an API for being able to have server-rendered markup be (potentially) entirely different to the client-rendered markup. For the purpose of better SEO, UX and speed.

What I'm suggesting is the general idea of this:

render: function() {
  if (this.isServerSide()) {
    return <div><Button ... /><Content hidden ... /><script ... /></div>;
  } else {
    return <div><Button ... />{this.state.expanded ? <Content /> : null}</div>;
  }
}

I.e that you can generate completely different markup server-side without affecting client-side performance, the script-block above could theoretically allow the button to show/hide the Content-component until the page has finished loading, the same approach could also be used for SEO-purposes or for limiting the size of the generated server-side markup.

Personally, my img- and picture-element srcSet polyfills require this for them to at all work after server-rendering. As rendering img and picture as-is without clearing srcSet on browsers with no/partial support will break.

So to me it seems that exposing e.g. this.serverSide() AND introducing new behavior for reusing server-rendered markup is the correct solution. E.g something like React.renderComponentAndReuseMarkup(serverDescr, clientDescr, targetNode), when reusing server-rendered markup, the client would first render normally using this.serverSide() === true and serverDescr, immediately afterwards it would render again with this.serverSide() === false and clientDescr instead. DOM generated by the server has been reused (as much as possible), while also enabling server/client to generate tailored markup.

@syranide
Copy link
Contributor Author

syranide commented Feb 5, 2015

I'm assuming contexts would be the right and super easy solution to this, it would just need to be standardized to really make work and be usable throughout.

@quantizor
Copy link
Contributor

It would be cool if there was a built-in way to parse this server-side code out of the JSX client-targeted bundle at bundling time. I know there's a variety of tools for the job, but something officially-recommended would aid the isomorphic app story.

@syranide
Copy link
Contributor Author

syranide commented Feb 6, 2015

@yaycmyk "parse this server-side code out of the JSX client-targeted bundle at bundling time" huh?

@jimfb
Copy link
Contributor

jimfb commented Feb 6, 2015

Don't we already re-use existing markup for isomorphic apps? Isn't that the point of the the checksum?

I'm not entirely sure I understand the motivation for having components render different markup if they're on the server side. You're just adding more branches/codepaths that need to be tested/maintained (the point of isomorphic rendering is to avoid exactly that, otherwise, just implement your SEO version separately). Also, sites are penalized if the content they display to users deviates from what they show Google, so I'm not entirely convinced this is a good SEO story in the first place.

My intuition is that: We don't recommend this, but we aren't going to stop you. If you're going to do this, you can use a global. This is probably not behavior that third party components should be participating in.

Is there a specific example usecase that isn't easily handled with a implementation-specifc global?

@syranide
Copy link
Contributor Author

syranide commented Feb 6, 2015

@JSFB You can't do feature detection server-side, hence most complex inline styles and polyfills are impossible to do server-side, there's also no way to generate placeholder content and apply the styles/polyfills once the client-side mounts as there is no transition taking place. I worked on a Picture and ImgSet polyfill which works great, but it's impossible to make them work at all when server-rendering, it simply cannot be done at current.

@jimfb
Copy link
Contributor

jimfb commented Feb 7, 2015

as there is no transition taking place

Perhaps I'm miss-understanding, but couldn't you do a forceUpdate when the page loads to update the placeholder content? Perhaps triggered by onMount, which I think doesn't get called server-side.

I'm still feeling like a global+useragent is sufficient for the vast majority of cases. It would be useful to have a specific example of a piece of markup that is both useful/desirable and unworkable server-side. I'm not saying such examples don't exist, but I looking for a persuasive example.

@syranide
Copy link
Contributor Author

syranide commented Feb 7, 2015

Perhaps I'm miss-understanding, but couldn't you do a forceUpdate when the page loads to update the placeholder content? Perhaps triggered by onMount, which I think doesn't get called server-side.

You're still lacking a way to find out if you are server- or client-rendering. Hmm, yes forceUpdate should actually work and be batched, it could be preferable... something based on context would be more seamless and Reacty, but that isn't a necessity I'd say.

I guess the neat thing about the forceUpdate approach is that it could (technically) just be exported globally via React.isServerRendering() and that is all that would be needed.

It would be useful to have a specific example of a piece of markup that is both useful/desirable and unworkable server-side.

Anything that requires feature detection; i.e. vendor styles and polyfills by and large. User agent sniffing is generally a big no-no and very fragile for feature detection... but it can work and could be the ultimate approach in a way, but you would still need a backup forceUpdate when it makes a mistake.

tl;dr forceUpdate should work great, but there still needs to be a way to detect server-rendering.

@quantizor
Copy link
Contributor

@syranide You wouldn't want to ship server-targeted markup to the client, would you? I was just suggesting it would nice to have an officially-supported, holistic approach to the development of isomorphic apps using React.

@syranide
Copy link
Contributor Author

@JSFB Oh right, lazy images/lists/etc require something like this too to be server-renderable.

@kryptt
Copy link

kryptt commented Apr 22, 2015

@syranide what if this can be re-stated as: "a more clearly defined transition between server rendering and client render should be made available"?

@JSFB To me, this means, since components are state-full, we want a well defined hand-off where a component (and its state) can be evidenced as leaving the server (some server-side events for serialization) and then coming into the client (some client-side events for deserialization).

I think environment and feature detection can then be identified / diagnosed from the context as / when necessary, but handling how server -> client transitions affect components needs to be somehow surfaced:
something like: componentWillSerialize for a server side rendering,
and either stop calling componentWillMount on the server side or add the corresponding componentwillDeserialze for the initial (called only once for pre-rendered components) client rendering..

@syranide
Copy link
Contributor Author

@kryptt Yeah I think so. Really it all just boils down to being able to determine client/server somehow and there being an update when the transition happens, i.e. (I assume) a value in context would be sufficient.

@dantman
Copy link
Contributor

dantman commented Jun 21, 2015

I'm in favour of this idea.

I believe this would have the side effect of fixing one of my issues. ie: These warnings when I conditionally use different content on the client and server (eg: a static link to home on the server and a JS navigation on the client) based on a require('is-browser') test.

Warning: React attempted to reuse markup in a container but the checksum was invalid. This generally means that you are using server rendering and the markup generated on the server was not what the client was expecting. React injected new markup to compensate which works but you have lost many of the benefits of server rendering. Instead, figure out why the markup being generated is different on the client or server:
 (client) 6hrsg74.3.0.1.0.0"><button id="mf6tgk6tr
 (server) 6hrsg74.3.0.1.0.0"><a id="p2yg9ecthg5y7g

I probably won't use it much, but I also like the idea of being able to control the serialization and deserialization of components on the client.

@sophiebits
Copy link
Collaborator

You should be able to do this with context fairly easily in 0.14.

@dantman
Copy link
Contributor

dantman commented Sep 5, 2015

One issue related to isomorphic apps, context, and setState in component{Will,Did}Mount comes to mind. Or rather, one mess.

  • It's said that it's good practice to try and call setState in componentWillMount instead of componentDidMount if it's immediate. As this avoids triggering a re-render where not necessary.
  • Using the double-render context pattern (render once as server then once as client on the client) that re-render from setState really isn't a problem. But it is if that same component is rendered client-only (eg: if you're using pushState so subsequent requests don't even hit the server and the next page visit only does client rendering).
  • It's reasonable to do client-only stuff in these two methods. For example doing some initialization based on browser features or state. Immediate stuff in Will and timeouts, ajax, dom in Did.
  • Using that pattern both componentWillMount and componentDidMount will run only once, when the component's context says it's being rendered with the server as the target.

So

  • The client-only componentDidMount runs when a component thinks its on the server.
  • componentWillMount cannot do client only stuff on first load, because it's first run with a server target (when it's actions may trigger checksum errors) and doesn't know when the target switches to the client.
  • Even if we defer componentDidMount until the browser target, componentDidMount is still an issue because it's needed to avoid double-render when only rendered on the client

ianobermiller pushed a commit to FormidableLabs/radium that referenced this issue Jan 27, 2016
My use case for this, is to prefix for all user agent on the server side
and only for the browser agent on the client side. As pointed out here facebook/react#1979
I think that it's a very good trade-off.
This allow me to remove the user agent from the memoization I use on the `renderToString` method.

The test fails because of robinweser/inline-style-prefixer#58.
I'm gonna address it.

Fix #546.
@taseenb
Copy link

taseenb commented May 15, 2017

It is not clear to me how this problem was solved (if it was). I still don't see how to bundle different packages for server/client and render something different without the checksum warning. What is the warning about btw? What does it actually mean for React?

I could not find any real solution. Hopefully i'm wrong, because it would mean that it is impossible to use React for isomorphic apps in so many scenarios. Well, at least all my scenarios: involving canvas, svg, video, animations... a lot of code and related modules that aren't needed on the server - and in some cases break the code with "window is not defined".

I tried to do something like this:
{ this.props.isBrowser ? <WebGL/> : null }

But of course that didn't work. Any idea on how to use React to precisely define 2 different bundles/markups in an isomorphic way without incurring in the warning?

@dantman
Copy link
Contributor

dantman commented May 15, 2017

I tried to do something like this:
{ this.props.isBrowser ? <WebGL/> : null }

But of course that didn't work. Any idea on how to use React to precisely define 2 different bundles/markups in an isomorphic way without incurring in the warning?

The browser version is usually bundled separately. Normally you'd just outside of react use a technique like that used in the is-browser package. Basically you just have a module that returns false (so it always returns false on the server) and tell your packager (browserify/webpack/etc...) to replace that module with one that returns true (so it always returns true when bundled and shipped to the browser).

@taseenb
Copy link

taseenb commented May 15, 2017

Thanks @dantman
I still couldn't understand how to deal with Webpack/React to get server + client bundles the way you say, but i'll keep trying. I also found that the simplest way to avoid requiring libraries that could break the server is leave them outside of the bundle, with a normal script tag (which is sometimes what you actually want if you use public CDN). It's still not ideal in many scenarios, but neither the server nor React complain.

If someone had an example of is-browser used with Webpack+React would be great :)

@gaearon
Copy link
Collaborator

gaearon commented Oct 1, 2017

I just wanted to add that the most common way to have different client/server markup is to set a flag in componentDidMount for the components that need to differ:

componentDidMount() {
  this.setState({ hasMounted: true });
}

Then you can use this.state.hasMounted for client-only elements in render.

@gaearon
Copy link
Collaborator

gaearon commented Jan 6, 2018

I outlined a possible solution that works today above.

I’ll close this issue but if you feel strongly this needs a separate API or a change in behavior please file an RFC and describe how you think it should work: https://github.com/reactjs/rfcs

Thanks!

@gaearon gaearon closed this as completed Jan 6, 2018
@ignatiusreza
Copy link

We managed to solve it by using es2015 import() via webpack.. basically, the main idea is to load non-critical components only on the client side lazily.. we do this by having a decorator which looks like

const asyncLoader = ({ loader }) => {
  class AsyncLoader extends Component {
    constructor() {
      super();

      this.state = { };
    }

    componentDidMount() {
      loader().then((bundle) => {
        const AsyncComponent = (bundle && bundle.default) ? bundle.default : bundle;

        this.setState({ AsyncComponent });
      });
    }

    render() {
      const { AsyncComponent } = this.state;

      return AsyncComponent && (<AsyncComponent {...this.props} />);
    }
  }

  return AsyncLoader;
};

and then use it to make any component async by doing

# path/to/component/index.jsx
const AnyComponent = () => <div />;

export default AnyComponent

# path/to/component/async.js
export default asyncLoader({
  loader: () => import('./index'),
});

which then can be imported by:

import AnyComponent from 'path/to/component/async'

the key here is, not all components need to be rendered in its full glory on SSR, SEO mostly only care about simple stuff like meta tags and text.. and users are generally okay with seeing above the fold fast feedback, even though they're incomplete.. hence, components which requires polyfill or big dependencies should be delayed and be loaded async (which would also help in improving the initial bundle size - faster page load)..

MattDennyir pushed a commit to MattDennyir/radium that referenced this issue Oct 19, 2022
My use case for this, is to prefix for all user agent on the server side
and only for the browser agent on the client side. As pointed out here facebook/react#1979
I think that it's a very good trade-off.
This allow me to remove the user agent from the memoization I use on the `renderToString` method.

The test fails because of robinweser/inline-style-prefixer#58.
I'm gonna address it.

Fix FormidableLabs/radium#546.
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