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

Wanted: Server-side rendering #1103

Closed
asood123 opened this issue Oct 13, 2016 · 62 comments
Closed

Wanted: Server-side rendering #1103

asood123 opened this issue Oct 13, 2016 · 62 comments

Comments

@asood123
Copy link

asood123 commented Oct 13, 2016

Hi! I work on OpenCollective - we help other open source projects (including react-boilerplate get funding.

We'd like to rebuild our frontend using React-boilerplate. But, Server-side rendering is a must for us - mostly for search crawlers and faster performance.

We are willing to pay someone to implement SSR in react-boilerplate and help the community out in the process. Would anyone be interested? You can reply on this issue.

Needless to say that whatever you build has to meet @mxstbr's approval and be merged into react-boilerplate to be considered complete.

Update: removed my email address. Prefer to discuss this publicly.

@sedubois
Copy link
Contributor

sedubois commented Oct 13, 2016

(NB just so that it is known, SSR seems to mix well with GraphQL. Relay, Apollo and Relate all seem to support it. E.g. the in-development Relax CMS created and uses Relate which has SSR, seems pretty neat. And I write this in brackets, because GraphQL clients are still on the experimental side 😉 see #1041 for more GraphQL discussion.)

@asood123 asood123 changed the title Wanted: Server-side rendering. Willing to pay. Wanted: Server-side rendering Oct 14, 2016
@sedubois
Copy link
Contributor

FYI other threads about SSR: #625, #174. The PR looks stale but could give inspiration I guess. Maybe @datapimp could comment?

@diegohaz
Copy link

I've implemented SSR on some React projects and I plan to do that on my boilerplate (https://github.com/diegohaz/arc). I'll see if I can do that in react-boilerplate and I send a PR. :)

@peter-mouland
Copy link
Contributor

if it helps, I have a project (much simpler than this, but still fully tested) which add SSR.

It shows how to add SSR using Koa or Express:

https://github.com/peter-mouland/react-lego

@sedubois
Copy link
Contributor

Looks like react-starter-kit provides SSR.

@sedubois
Copy link
Contributor

sedubois commented Oct 15, 2016

It also looks like hjs-webpack provides SSR (but really I never configured SSR so don't know what I'm talking about). And anyway looks like a nice way to simplify the webpack configuration, automatically comes with dev server, transpiling, HMR, minifying and publishing. My only concern is lack of activity since 2 months.

@peter-mouland
Copy link
Contributor

I guess Koa is the preferred server technology as it is more active? or are there others we should be looking into?

@steida
Copy link

steida commented Oct 15, 2016

That's how Este does server-side rendering with React Router 4.

https://github.com/este/este/blob/70409fe62e8b4fce48ec4d84eda155017069482a/src/server/frontend/render.js

It's not only about a server-side rendering but also server-side data fetching.
I think this 20 sloc component solves is universally for any data source.

https://github.com/este/este/blob/70409fe62e8b4fce48ec4d84eda155017069482a/src/server/frontend/ServerFetchProvider.js

@jotto
Copy link

jotto commented Oct 15, 2016

I'm self promoting here, but I just made a solution for this exact scenario, React checksums included: https://www.prerender.cloud/

@asood123
Copy link
Author

After receiving a few responses over email, I realized it's probably better to discuss it publicly.

So, if you are interested in helping, please respond here with:

  • how you plan to add ssr, any implementation details, snags, etc. Would rather resolve those ahead of time, then block PR from getting merged.
  • if you wish to be compensated, how much you expect to receive?

Fyi, @miksago.

Note that we plan to pay whoever builds this through react-boilerplate open collective. As in, we'd donate to opencollective.com/react-boilerplate (or help get the funds from others if it's beyond our budget). And then you can invoice that collective directly.

@sedubois
Copy link
Contributor

sedubois commented Oct 25, 2016

@asood123 FYI a very interesting article (retweeted by Reactjs) on whether SSR is actually needed for SEO (do make sure to read the ongoing comments below as well).

So although not having SSR might impact search rankings, it's at least inaccurate to say that it just isn't indexed at all, they might just not wait for async data. There's the "Fetch as Google" tool to check how your site is actually indexed. You could maybe give it a try?

As for the performance aspect, I wonder how impactful SSR is compared to the other benefits already in RBP (e.g code splitting)? @mxstbr do you have production-grade websites based on RBP to share, so that people can see with their own eyes how fast the rendering already is?

@jaysoo
Copy link

jaysoo commented Oct 25, 2016

@asood123 I might be wrong, but I'm not sure if there is a "best" way to do SSR for any given React+Redux app.

I've worked on some apps that use redux-saga to perform data tasks, and resolves using saga mechanisms. For other apps I've checked out a few libraries like redux-async-connect, and I've also wrote my own (react-transact -- excuse the lack of docs, but there are examples).

The main problems for SSR are:

  1. Mapping matched route components to data tasks.
  2. Guaranteeing that data tasks are complete before rendering HTML.
  3. Serializing application state atom into JSON + embed them on page.
  4. Deserializing application state atom from embedded JSON.

Luckily 3 and 4 are dead simple with Redux. However, I'd be hesitant to push for a single solution to do 1 and 2.

Not sure what @mxstbr thinks of this? I don't mind sharing some ideas, just don't know if any solution is generic enough to be part of the boilerplate.

In general, I'd avoid SSR if possible. It does add some performance gains (especially on mobile networks), and I'm sure SEO would improve, but it comes at the cost of added complexity.

@steida
Copy link

steida commented Oct 25, 2016

@jaysoo Sorry for spamming here, but this is already solved in Este:
Universal data fetching component helper. We use context for fetching registration.
https://github.com/este/este/blob/master/src/server/frontend/ServerFetchProvider.js

Waiting for the final render with timeout etc.
https://github.com/este/este/blob/master/src/server/frontend/render.js#L123

Example in one component. Sure we can use the same pattern in every other component.
https://github.com/este/este/blob/master/src/common/lib/redux-firebase/firebase.js#L35

Simple, elegant, battle tested, universal.

@jaysoo
Copy link

jaysoo commented Oct 25, 2016

@steida No worries about spamming. I still don't quite see how this addresses the issue universally though.

Counter example

When using redux-saga, you may have data tasks defined using generator functions.

// using redux-saga
import { call, put, take } from 'redux-saga/effects'

function* requestThing() {
  yield take('REQUEST_THING')
  const data = call(fetch, ...)
  yield put({ type: 'RESOLVED_THING', payload: data })
} 

This means components will only dispatch request actions, and the return type is void. The only way to access the promise is to get the task object and await on the task.done promise. But I'm unsure how to get access to these task objects from within components.

Note: It is possible to run a root saga only for the SSR case that will await all data tasks to complete before resolve its promise object. Just need a convention for what this looks like.

@steida
Copy link

steida commented Oct 25, 2016

You don't believe me it works? It works for server side rendering and React Native both. Run Este on your computer and check this:
https://github.com/este/este/blob/master/src/browser/users/OnlineUsers.js#L64

@jaysoo
Copy link

jaysoo commented Oct 26, 2016

@steida Not arguing it doesn't work for those cases. I've built several React apps with SSR myself, so I know the solution of provider+decorator+queue/manager works. I'm just bringing up another case that I don't think would work (without modification).

Say I have a component like this:

class MyPosts extends Component {
  static contextTypes = { store: React.PropTypes.object }

  componentWillMount() {
    this.context.store.dispatch({
      type: 'posts/REQUEST_POSTS'
    })
  }

  render () { ... }
}

Where the posts/REQUEST_POSTS action is picked up by a saga, which fetches the requested data, and then dispatches posts/LOADED_POSTS.

function* watchForRequestPosts() {
    while (true) {
      yield take('posts/REQUESTED_POSTS')
      const posts = yield call(fetch, '/posts')
      yield put({ type: 'posts/LOADED_POSTS', payload: posts })
    }
}

In this case, the MyPosts component, nor any HOC on top of it, can have access to know when posts are resolved.

Of course, there are many ways to get this to work with SSR, and I've done it before, it's just a matter of.

  1. Is it worth generalizing this for RBP.
  2. If so, which solution should be taken?

Since RPB recommends redux-saga, it is probably worth the consideration for SSR.

@jaysoo
Copy link

jaysoo commented Oct 26, 2016

Actually, didn't notice that RPB adds store.runSaga method to the enhanced redux store. This could be used to run a task and push the task.done promise for SSR case. The assumption here would be that no long-running saga is fetching data required for SSR.

@diegohaz
Copy link

@jaysoo @steida I've just implemented this on this project using redux-saga: https://github.com/diegohaz/arc/tree/universal-redux

Actions:

export const postList = {
  request: (limit, resolve, reject) => ({ type: POST_LIST_REQUEST, limit, resolve, reject }),
  success: (list) => ({ type: POST_LIST_SUCCESS, list }),
  failure: (error) => ({ type: POST_LIST_FAILURE, error })
}

Saga:

export function* listPosts (limit, resolve = fn, reject = fn) {
  try {
    const params = { _limit: limit }
    const { data } = yield call(api.get, '/posts', { params })
    resolve(data)
    yield put(postList.success(data))
  } catch (e) {
    reject(e)
    yield put(postList.failure(e))
  }
}

export function* watchPostListRequest () {
  while (true) {
    const { limit, resolve, reject } = yield take(POST_LIST_REQUEST)
    yield call(listPosts, limit, resolve, reject)
  }
}

export default function* () {
  yield fork(watchPostListRequest)
}

Container component:

class SamplePageContainer extends Component {
  static post ({ req, store }) {
    return Promise.all([
      this.get({ store }),
      store.dispatch(submit(config, req.body))
    ])
  }

  static get ({ store }) {
    return new Promise((resolve, reject) => {
      store.dispatch(postList.request(15, resolve, reject))
    })
  }

  render () {
    return <SamplePage />
  }
}

Server:

app.use((req, res, next) => {
  // ...
  match({ history, routes, location: req.url }, (error, redirectLocation, renderProps) => {
    // ...
    const fetchData = () => new Promise((resolve, reject) => {
      const method = req.method.toLowerCase()
      const { params, location, components } = renderProps
      let promises = []

      components.forEach((component) => {
        if (component) {
          while (component && !component[method]) {
            component = component.WrappedComponent
          }
          // try to call Component.method where method is the HTTP method name
          component &&
          component[method] &&
          promises.push(component[method]({ req, res, params, location, store }))
        }
      })

      Promise.all(promises).then(resolve).catch(reject)
    })
    // ...
    fetchData().then(() => {
      render(configureStore(store.getState(), memoryHistory))
    }).catch((err) => {
      console.log(err)
      res.status(500).end()
    })
  })
})

@jaysoo
Copy link

jaysoo commented Oct 26, 2016

@diegohaz That's pretty elegant, thanks for sharing! I guess generally if a request action is dispatched, there should be a resolve and reject handler that can be invoked by the corresponding saga.

@asood123
Copy link
Author

@jaysoo @diegohaz @steida. Am enjoying the discussion. Would one of you want to take on the task of adding support for SSR? :) Let us know how much you'd charge and we'll figure something out.

@diegohaz
Copy link

@asood123 I'll take a look at the RBP code today to see how long I can do this and give you an answer. 😊

@diegohaz
Copy link

Hi, @asood123, It will cost me about 3 working days. I will ask $750 for it.

@sedubois
Copy link
Contributor

FYI there's prep, a side-project of the Graphcool team, looks like an easy way to pre-render at build time (zero server/bundler configuration needed). See their example or their Learn Relay source code for usage. Don't know how much effort would be needed to integrate with more dynamic content and of course it would pre-render "old" data as it's at build time, but it might be an easy win especially for the more static parts and if deployments are frequent enough.

@jaysoo
Copy link

jaysoo commented Oct 27, 2016

I'll let @diegohaz take a stab at it. I will be on vacation soon for 3 weeks, and I have other work queued up at the moment. Happy to provide any support in terms of advice, etc.

@sedubois That could be interesting approach for static sites. I'd also look at https://zeit.co/blog/next for inspiration.

@asood123
Copy link
Author

asood123 commented Oct 28, 2016

@diegohaz that sounds pretty reasonable. When will you be able to start? And React-boilerplate will pay out the invoice when ssr is merged into master.

@diegohaz
Copy link

@asood123 I'll start a PR this monday. 😊

@GGAlanSmithee
Copy link
Contributor

when ssr is merged into master.

Not to interrupt your negotiations, but is this feature even wanted by the maintainers? I thought it was a conscious decision not to put ssr in the master branch?

@asood123
Copy link
Author

yes, we have been talking to @mxstbr. He'd like to get SSR support as well.

@diegohaz
Copy link

diegohaz commented Nov 9, 2016

@datapimp I've just read your article and will try to figure this out. I'll ask you for help if I have any doubts, thanks. 😊

@datapimp
Copy link
Collaborator

datapimp commented Nov 9, 2016

Great @diegohaz.

I believe in its current state, the DLL Bundle for this project gets compiled for libraryTarget: web.

In order to make SSR work, you will need to create the DLL Bundle for the libraryTarget: umd so that it can be required in both a browser environment and the node environment. I'm available if you need any help.

JS

@diegohaz
Copy link

I still have not managed to find the time to solve the problems I encountered and probably will not have time for this anytime soon.

@tomazy contacted me and It seems he has a working solution. Just posting here to ensure that there is no problem if he (or another person) takes this in my place.

@asood123
Copy link
Author

That's too bad to hear, @diegohaz. Thanks for the update though. No problem from our end if @tomazy picks it up.

@tomazy tomazy mentioned this issue Nov 15, 2016
8 tasks
@tomazy
Copy link
Contributor

tomazy commented Nov 16, 2016

I've created a new PR for this feature (#1236). There are still a few small things to be solved but in general it works. I'd appreciate if you guys could spend a minute on reviewing it to check if I don't make any design mistakes.

@isaachinman
Copy link

isaachinman commented Nov 16, 2016

@asood123 @tomazy Watching this closely and am very excited about the potential. Does anyone know if @mxstbr plans on merging this into master, or will it be a separate SSR branch? My preference would be the former, to ensure it's maintained properly.

@asood123
Copy link
Author

Last I talked to @mxstbr, this was going to merged into master...

@sedubois
Copy link
Contributor

I better understood the need for server-side rendering with this article: http://rauchg.com/2014/7-principles-of-rich-web-applications/#server-rendered-pages-are-not-optional

@tomazy
Copy link
Contributor

tomazy commented Nov 17, 2016

@asood123 Have you had a chance to look at #1236 ? I think it's pretty much done.

@asood123
Copy link
Author

asood123 commented Nov 17, 2016

@tomazy I looked over the PR. Looks fine to me - though I'm not that experienced. Better to have someone like @datapimp or @diegohaz look at it.

I'll ping a couple other people who might be able to review as well. And will play with the branch locally as well.

Excited to see this go live soon!

@WhatIfWeDigDeeper
Copy link

Any estimate on when this will get merged in? @mxstbr We wanted to start using react-boilerplate, but SSR is a must have for us. Thanks!

@mathieumg
Copy link

@WhatIfWeDigDeeper If you look at #1236 you can see there was activity a few hours ago.

@WhatIfWeDigDeeper
Copy link

Thanks for the update.

@mxstbr
Copy link
Member

mxstbr commented Dec 13, 2016

@mxstbr We wanted to start using react-boilerplate, but SSR is a must have for us. Thanks!

Please use the PR branch and let us know where it breaks so we can fix it before releasing it!

The one reason I haven't merged the PR yet is that I don't know if it works for everything. The only way to find that out is by having people use it! So please use the PR branch by @tomazy and let us know how it goes!

@ankitduseja
Copy link
Contributor

@mxstbr Its been a while since any activity here. Are you going to merge it?

@hadifarnoud
Copy link

is the SSR effort abandoned?

@justingreenberg
Copy link
Member

@hadifarnoud unfortunately there is currently no plan to land SSR support in react-boilerplate :( please see #1236 and #1776 for more information. specifically, please follow #1236 as there may be a maintained SSR fork that you can use... i am leaving this issue open for visibility until next release

@hadifarnoud
Copy link

if there is a working pull request, why not merge it since a lot of people seem to want this feature?

@ghost
Copy link

ghost commented Oct 30, 2017

@diegohaz Thanks for sharing the example. I have implemented something similar to that in React Universal. I am using static fetchData to call the server with the function at first through redux-saga

While I have everything working properly as I want, There seems to be a re-fetching in case of using the fetchData function inside componentDidMount or componentWillMount I can see that the server-side function is called successfully but for some reason yield put does not add the value to my store which is funny considering the rest is working

All up till the console.log is working with axios. Just yield put does not put the data inside store but when I call the same function inside ComponentDidMount again, the store works properly with the update.

Let me know if anything can be done about it to prevent re-fetching!

Here is my saga.js

import { call, put, takeEvery, all, select } from 'redux-saga/effects';
import axios from 'axios';
import jwt from 'jsonwebtoken';
import { port, host } from '../appConfig';

export const getHomeState = state => state.home;

export function* fetchData() {
  const homeState = yield select(getHomeState);
  console.log(JSON.stringify(homeState));
  try {
    const token = jwt.sign({ exp: Math.floor(Date.now() / 1000) + (60 * 60) }, 'whatever');
    let response = null;
    if (port) {
      const reqURL = `http://${host}:${port}/api/data`;
      response = yield call(axios.get, reqURL, { headers: { authorization: token, 'Content-Type': 'application/json' } });
    } else {
      const reqURL = '/api/data';
      response = yield call(axios.get, reqURL, { headers: { authorization: token, 'Content-Type': 'application/json' } });
    }
    console.log(response.data.msg);
    yield put({ type: 'FETCHED_JWT', data: response.data.msg });
  } catch (err) {
    console.log(err);
    console.log('saga API data did not work. Retry fetching it');
  }
}

export function* watchFetchData() {
  yield takeEvery('FETCH_DATA', fetchData);
}
export default function* rootSaga() {
  yield all([
    watchFetchData(),
  ]);
}

Thanks

@smspillaz
Copy link

Is there any interest in taking another shot at this? I recently implemented SSR in my own fork of react-boilerplate (writeup of what had to change here). If the maintainers are interested, I'm happy to send my changes upstream in a pull request, but I don't want to do that if there is a pre-existing plan for this that I'd be stomping on.

@hadifarnoud
Copy link

@smspillaz
I don't think there is any SSR plan unfortunately. there is a pull request here #1236

It was decided not to include SSR. we had the same issue as @asood123 and used this fork (had problems along the way but it works). It would be fantastic if @mxstbr helps the effort and allow that fork or any other implementation to merge. This is very important to many.

btw, I'm interested to learn what is the difference between your fork and @tomazy.

@peter-mouland
Copy link
Contributor

remembering back to the original conversation - i think the decision was to keep this project as a client-side only boilerplate. The hope being that it would help focus future changes and keep it simpler. You might come up against the same push-back that happened last time.

I think if your PR enables client-side only and a SSR option, then you're on to a winner, especially if the changes to either are minimal.

@gretzky
Copy link
Member

gretzky commented Mar 2, 2018

I agree heavily with @peter-mouland, couldn't have said it better. Having an SSR option in addition to client side only would be ideal.

Adding in my two cents: I feel that SSR should be a decision based on project scopes and doesn't belong in a boilerplate. SSR can take a big toll on performance if you don't have a well thought-out project scope for both front and back end, and those alone open up large cans of worms that wouldn't be fair to put on the shoulders of a boilerplate.

If anyone wants to restart the ball rolling with adding an SSR option, please feel free, as long as it is truly an option. See #2129 for a short list of what I'm currently working on, it may have a place in there as well.

@gretzky gretzky closed this as completed Mar 2, 2018
@kunal-arora
Copy link

@smspillaz
Thanks for putting in the effort. I will try and use your branch in my project. Will let you know if i run into anything.

@lock
Copy link

lock bot commented May 28, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators May 28, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests