tizmagik / react-head

⛑ SSR-ready Document Head tag management for React 16+
MIT License
323 stars 34 forks source link

renderToNodeStream support #13

Closed antonybudianto closed 6 years ago

antonybudianto commented 7 years ago

Hi,

Do you have plan for renderToNodeStream support?

Thanks

tizmagik commented 7 years ago

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 commented 7 years ago

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

tizmagik commented 7 years ago

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 commented 7 years ago

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 https://github.com/nfl/react-helmet/pull/296, but still no idea how to implement it

tizmagik commented 7 years ago

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 commented 7 years ago
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 commented 6 years ago

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?