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

renderToNodeStream support #13

Closed
antonybudianto opened this issue Oct 24, 2017 · 7 comments · Fixed by #18
Closed

renderToNodeStream support #13

antonybudianto opened this issue Oct 24, 2017 · 7 comments · Fixed by #18

Comments

@antonybudianto
Copy link

Hi,

Do you have plan for renderToNodeStream support?

Thanks

@tizmagik
Copy link
Owner

Haven't considered that use case yet, but presumably instead of pushing to an Array we could use an Observable. Do you have any thoughts on this?

@antonybudianto
Copy link
Author

Sorry I'd no idea too @tizmagik. But observable is good idea though.

@tizmagik
Copy link
Owner

Actually hmm, now that I think about it, it probably doesn’t make sense for this library to actually do anything when rendering to node stream on the server since once the head is sent over the wire, it can’t change. The good news though is that on the client the head should be updated correctly with any head tags rendered deeper in the tree after minting.

Does that make sense? What do you think?

@antonybudianto
Copy link
Author

antonybudianto commented Oct 30, 2017

yes, I also think it's impossible, and the same issue is still open on the react-helmet side.
My plan is to integrate this lib into https://github.com/antonybudianto/cra-universal, but looks like I must prepare both renderToString and renderToNodeStream versions where only renderToString can support document head SSR

Also there's possible solution in nfl/react-helmet#296, but still no idea how to implement it

@tizmagik
Copy link
Owner

Well the good news is that this lib should work fine whether you renderToString or renderToNodeStream. The only issue is that the SSR head tags coming in from the server when rendering to node stream won't be immediately the right values, but will eventually be correct once the client mounts. In either case the code would look the same and you shouldn't need to do much conditional logic, if at all, to handle the different rendering modes.

That cra-universal project looks really cool!! I'm glad you've considered this lib for it. Let me know if I can be of any assistance. Good luck!

@fabiulous
Copy link

fabiulous commented Nov 5, 2017

import React from "react"
import { Provider } from "react-redux"
import { renderToNodeStream } from "react-dom/server"
import { StaticRouter } from "react-router-dom"
import { getLoadableState } from "loadable-components/server"
import Helmet from "react-helmet"
import I18n from "redux-i18n"

export default async (req, res) => {
  const store = configureStore({
    i18nState: {
      lang: "en",
      translations: {},
    },
  })
  const context = {}

  const appWithRouter = (
    <Provider store={store}>
      <I18n translations={{}} fallbackLang="en" useReducer>
        <StaticRouter location={req.url} context={context}>
          <App />
        </StaticRouter>
      </I18n>
    </Provider>
  )

  if (context.url) {
    res.redirect(context.url)
    return
  }

  let loadableState = {}

  store.runSaga(sagas).done.then(() => {
    const headAssets = Helmet.rewind()
    res.status(200).write(renderHeader(headAssets))

    const initialState = store.getState()

    const htmlSteam = renderToNodeStream(appWithRouter)
    htmlSteam.pipe(res, { end: false })
    htmlSteam.on("end", () => {
      res.write(renderFooter(loadableState, initialState))
      return res.send()
    })
  })

  // Trigger sagas for component to run
  // https://github.com/yelouafi/redux-saga/issues/255#issuecomment-210275959
  loadableState = await getLoadableState(appWithRouter)
  // Dispatch a close event so sagas stop listening after they're resolved
  store.close()
}

loadable-components has allowed me to use renderToNodeStream

I got everything to work thanks to https://github.com/alexisjanvier/universal-react
and https://marmelab.com/blog/2017/10/17/code-splitting.html

Edit: Forget this. I noticed before going to prod that meta tags were inconsistent and had to change back to renderToString

@tizmagik
Copy link
Owner

Edit: Forget this. I noticed before going to prod that meta tags were inconsistent and had to change back to renderToString

Thanks for looking into this @shizpi -- they were inconsistent in what way?

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

Successfully merging a pull request may close this issue.

3 participants