search

airbnb / hypernova-react

React bindings for Hypernova.
https://github.com/airbnb/hypernova
MIT License
248 stars 43 forks source link

Setup hypernova-react for streaming response #28

Closed wingleung closed 6 years ago

wingleung commented 6 years ago

In React 16 2 new streaming methods were introduced:

Should these functionalities be added to hypernova-react?

ljharb commented 6 years ago

Can they be polyfilled for older versions of React?

wingleung commented 6 years ago

That's core react functionality that will need to be polyfilled, possibily adding another lib for all the streaming logic.

Could we maybe fallback to renderReact and renderReactStatic?

ljharb commented 6 years ago

Right, I think it'd be ok to provide a stream for the result even if it wasn't "streaming"

goatslacker commented 6 years ago

We can expose both APIs and if their version of ReactDOMServer doesn't have a renderToNodeStream method then it'll throw.

ljharb commented 6 years ago

That's certainly one approach; it'd be better if we could wrap it in a stream in that case, instead.

wingleung commented 6 years ago

I agree with exposing both apis and falling back on a wrapped stream in renderTo(Static)NodeStream if React < 16.

However I just tried the stream functionality in my project and it seems hypernova always sends back a string which makes stream support impossible without a change in the hypernova-node code.

See code in https://github.com/airbnb/hypernova-node/issues/29

ljharb commented 6 years ago

You can make a stream around a single string just fine; it won't have any performance benefits but the API will be the same.

wingleung commented 6 years ago

closing as this is not relevant to me anymore, we solved the streaming by just using Varnish streaming capabilities with ESI more info -> https://medium.com/vrt-digital-studio/server-side-rendering-react-components-as-a-service-9d6b887cb02a