Set shouldComponentUpdate to false for codemirror binding

[jimpick]

Reduces the amount of react re-rendering.

Here are some videos to show the impact:

Before: https://gateway.ipfs.io/ipfs/QmV5UdKMTUbmVuz3nLrUSHLidroy8DaBZWZZ5EuB1wUZfH/react-before.mp4

After: https://gateway.ipfs.io/ipfs/QmQJddeAsL2u8f3pUFwBTVoPTD82CvHwP3LvuB5YiiDEE7/react-after.mp4

[victorb]

Setting shouldComponentUpdate straight to false without any logic, feels like a dangerous pattern. That means that any changes to state OR props will not actually be reflected in the component after the first mount.

I think a better route would be to hunt down exactly which state/prop change causes an unnecessary render and then update shouldComponentUpdate to ignore that specific state/prop instead.

[jimpick]

Codemirror is designed to be used as a stand-alone library, so it does take care of all it’s rendering needs independently of react. It would be good to make sure any state/prop changes are relayed to the wrapped codemirror instance.

I’m wondering if the codemirror/react binding to peer-star-app could be broken out into a separate package with tests?

[pgte]

Yeah, codemirror and react don’t play along. This should free some rendering cycles! On 2 Dec 2018, 03:55 +0000, Jim Pick notifications@github.com, wrote:

Codemirror is designed to be used as a stand-alone library, so it does take care of all it’s rendering needs independently of react. It would be good to make sure any state/prop changes are relayed to the wrapped codemirror instance. I’m wondering if the codemirror/react binding to peer-star-app could be broken out into a separate package with tests? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

[jimpick]

I created a new issue #249 to track ideas for breaking out the codemirror <-> peer-star-app component and peer-star-app binding into a separate package. I think it could be improved quite a bit, and would benefit from having it's own separate test suite.

[jimpick]

I did some more testing, and I didn't see any problems introduced by this change, so I'm going to merge it.