React: Consider renaming renderToStream to renderToNodeStream

Created on 4 Aug 2017  路  13Comments  路  Source: facebook/react

With the checkin of #10362, the code to render to a stream now looks like:

import { renderToStream } from 'react-dom/server'

renderToStream(<div>some text</div>).pipe(response);

This is great! I am, however, a little concerned about taking the word "node" out of the mix, because I'm thinking that we may eventually want to render to browser streams, which are a really neat feature that can be used in concert with "service-worker-side rendering". I would propose renaming renderToStream to renderToNodeStream and renderToStaticStream to renderToStaticNodeStream. This would give room for renderToBrowserStream in the future.

If I make a PR on this, would it have a chance of being accepted?

cc @gaearon & @sebmarkbage

Most helpful comment

The thing I like about "node stream" naming is now people know to google "node streams" to find documentation about it. It wasn't obvious before what kind of API the stream provides.

All 13 comments

I think we can do it.

maybe another option can be to export renderToStaticStream, renderToStream directly in react-dom -> import { renderToStream, renderToStaticStream } from 'react-dom' when browser streams are ready, and leaving the server part (node streams) in react-dom/server -> import { renderToStream, renderToStaticStream } from 'react-dom/server'

The thing I like about "node stream" naming is now people know to google "node streams" to find documentation about it. It wasn't obvious before what kind of API the stream provides.

good point, makes sense, i think explicitness wins in this case 馃槃

I think we can do it.

To be clear, do you mean you can accept such a PR, or you can do the name change yourselves? I'm fine either way, just want to understand if I should work on it or not. ;)

We'll take a PR :-)

Both environments have claimed the word Stream which is mostly fine because they're not expected to mix. So you just use one notion of the opaque Stream thing in one and get a different in the other.

One is more likely to leak into other environments though. Such as Electron or React Native apps. Which one are they going to favor? If it spreads then Node Stream is a bit of misnomer. (it's not "Browser Promise".) If it's considered legacy then it might make sense though.

Both environments have claimed the word Stream which is mostly fine because they're not expected to mix. So you just use one notion of the opaque Stream thing in one and get a different in the other.

I'm not sure what you mean. Are you suggesting that the same method would return two different types of objects depending on its environment? That seems counterintuitive to me, and sample code using the method would be very confusing, as it would only work in the right environment.

Such as Electron or React Native apps. Which one are they going to favor? If it spreads then Node Stream is a bit of misnomer.

I mean, it still came from Node. But Electron is an interesting example, as I believe it includes both Chromium and Node APIs, so both web streams and Node streams are available there, I think.

I think it's acceptable to risk a minor misnomer in this case. The target audience will understand that node streams refer to the stream API implemented and popularized by node, even if they're being utilized in other environments.

What do you think of renderToStreamNode instead? renderToNodeStream makes me wonder "what's a node stream? what else would be in the stream if not DOM node markup?"

renderToStreamNode makes me wonder if there's some kind of DOM node that supports streams 馃槃 I think there's potential for confusion, either way, so maybe it's worth asking the community what they think is clearer?

What do you think of renderToStreamNode instead?

I worry that it seems like "Stream" is an adjective modifying the noun "Node", and that the method would be expected to return a node. Sigh. We don't have enough words.

Fixed in #10425. Thanks!

Was this page helpful?
0 / 5 - 0 ratings