react-codemirror icon indicating copy to clipboard operation
react-codemirror copied to clipboard

Why is Codemirror#componentWillReceiveProps async?

Open n1k0 opened this issue 8 years ago • 7 comments

There might be a very good reason I'm not aware of for debouncing componentWillReceiveProps by default, but it created a lot of headache when it comes to state synchronization (refs https://github.com/mozilla-services/react-jsonschema-form/pull/175).

What's the reasoning behind scheduling it to the next tick, perfs? Could it be conceivable to make this an optional behavior?

n1k0 avatar Apr 28 '16 07:04 n1k0

The debouncing causes me some trouble as well. It appears that the debounce applies to all instances of the component on a page simultaneously, so if they all try to re-render at once, only the last instance will actually show a change.

This was the issue I described in #46, which I closed because I found a way to work around it. However, this will still be a problem for instances like drag & drop sorting, where there is more than one instance that legitimately needs to re-render at the same time.

Here's a link to a video demonstrating the issue.

https://youtu.be/-mbHy17bxZw

sslotsky avatar May 02 '16 05:05 sslotsky

I submitted #49 to make the debounce specific to the instance, rather than all instances.

This resolves the issue shown in the video linked above, as can be seen in this video.

https://drive.google.com/open?id=0Bzbw-6Q_sVTyblQzMldidXdIdUU

sslotsky avatar May 04 '16 03:05 sslotsky

Thanks for the pull request, @sslotsky. It fixes a similar problem I was having where a first instance of CodeMirror fails to sync its options to state when there's a second instance on the page.

attaboy avatar May 30 '16 21:05 attaboy

I submitted the PR which added the debouncing—it could have been misguided. I was having a nightmare with state synchronization, as described in #5. I wrote #35 to fix my problems. I didn't consider what would happen with multiple instances.

Maybe someone else can dig into the state sync problems and come up with a better solution.

alexdmiller avatar Jun 04 '16 01:06 alexdmiller

Hey @alexdmiller what was the reason to debounce?

If there is a good reason, one way to fix the bug is to create the debounce function in the componentDidMount function, I can give it a shot. But before that, could you elaborate the reason for the debounce?

Chun-Yang avatar Sep 06 '16 19:09 Chun-Yang

This seems really tricky to work around. For example, in my component I have a componentDidUpdate where I'm calling doc.markText(), but this assumes that the underlying CodeMirror instance has the value that was passed into the React-CodeMirror instance during the last render. This is problematic though, because React-CodeMirror doesn't pass the value to the underlying CodeMirror instance until its debounce is complete, which is after my componentDidUpdate execution.

Aaronius avatar May 10 '17 19:05 Aaronius

This appears to be a duplicate of #106. I ran into the same issue and created react-codemirror2 because I was dying over the debouncing issue (and couldn't sleep at night with the lodash dependency). My wrapper component is very barebones, surely needs some more work, but works well for my simple usage. I'll be maintaining it moving forward so any struggles going on here I'll happily work out in the alternative package. Please let me know if anyone takes the time to check it out and finds it useful - thanks.

scniro avatar Jun 01 '17 18:06 scniro