Change the render approach so that it does not need ReactDomServer #32
Conversation
|
Hi @CurtisHumphrey, thanks for you PR. I have some questions:
Do you have some numbers to back this up? Also, please see inline comment(s) for other questions/comments. At this point though I'm not keen on rewriting the module internals and potentially compromising stability, unless there is a proven issue that needs addressing. |
| <div className={className} data-src={path} style={style} /> | ||
| </div> | ||
| const wrapper = document.createElement('div'); | ||
| ReactDOM.render( |
| evalScripts, | ||
| each, | ||
| }); | ||
| this.container.appendChild(wrapper.firstChild); |
There was a problem hiding this comment.
Any reason why the order of this has been flipped compared to the original version? I vaguely remember the original order was deliberate too but I'll have to check the issues again to be sure.
|
Closing due to no activity... |
|
I saw this too and thought it might've been a massive issue, but including it in my project adds only ~13 KB pre-gzip which I think is somewhat reasonable. This is probably due to dead code removal. |
|
Thanks for adding that info @matthewoates 👍 |
ReactDomServer is not typically included on a website and so it usage increase bloat. This approach does the same thing and uses dependence present namely ReactDom.
It does output style slightly differently so I had to also update the fixtures (added a space and a ";")