A bug
The editor crashes if we connect the onChange function with redux action and our typing speed is just nominally fast, and wait for the state update. Given below in the codesandbox link just try to type nominally fast and the editor will crash.
Codesandbox reproducing the issue
Slate: 0.57.1
Browser: Chrome / Safari / Firefox / Edge
OS: Mac / Windows / Linux / iOS / Android
The editor should not crash on connecting with redux and waiting for the state to update.
This is a reopen of #3332 as currently we have to bypass the redux lifecycle. And the editor still crashes if we directly connect it with redux.
I am facing the same issue. I used debounce to solve this problem.
import debounce from "lodash/debounce";
<YourSlateEditor value={value} onChange={debounce(value=>{
onChange(value);
}, 500)} />
This works for me. As a React developer, we like to put everything into the single source to manage state. However, if you need to update a big big big state like our SlateEditor, you have to bypass it by storing the latest value and skip unnessary updates to Redux store. For instance, if a user keeps typing 12345678999999999 in SlateEditor. The SlateEditor keeps updating the value like "1", "12", "123", "1234". Our Redux store will be updated too frequently. It's costy becaues the state of SlateEditor is really huge. It's a JSON format, not just a pure text. What we can do is to use debounce. The logic is our Redux store skips "1", "12", "123" until we found user have finished their input, and then store a complete string "12345678999999999" into our Redux store.
We experienced the same issue with our GraphQL cache (data store).
I think the reproducing example is slightly misleading. I believe the core of the issue is calling onChange asynchronously, and not necessarily taking a lot of time. As long as your redux update remains synchronous, I believe it should work fine.
Here's a fork to demonstrate: https://codesandbox.io/s/frosty-butterfly-qzpec
Instead of simulating lagginess with a timeout, I've written a dumb recursive function that actually likely takes even more time that 5ms and while it is incredibly janky, it does not crash.
Are you guys perhaps doing something asynchronous in your redux? :thinking:
Hi, @JirkaVebr , I didn't use any asynchronous call in my Redux.
@YacheLee Could you please make a synchronous, reproducing code sandbox then? You might not be using any asynchronous calls yourself. Instead, it might be caused by some middleware, redux-thunk or something similar. My point is, referring to my previous code sandbox, that even if you ramp up the parameter of the artificially slow function to the point that it becomes so incredibly slow that it's downright unusable, you still don't get any exceptions because it stays synchronous. I don't think your redux updates are actually that slow but rather asynchronous for some reason.
Hi, @JirkaVebr, Here is the example of using Redux with SlateEditor(reproduce the bug)
https://codesandbox.io/s/react-example-ny1ok
@YacheLee Interesting! It appears that I stand corrected and there is indeed a way to reproduce this. I'll try to investigate further.
Thanks for the sandbox, that is very helpful. While I've never run into this myself, you've just demonstrated that I feasibly could, and so I'll try to figure this out and perhaps even come up with a fix, if I can. Thanks!
@JirkaVebr I'm not able to reproduce the bug in @JirkaVebr's last sandbox. Can you share the steps to reproduce?
@pzhine That's was the point. My sandbox was meant as a counterexample in which it isn't possible to reproduce the issue (or at least I don't know how to reproduce it there). YacheLee then probably edited his sandbox because I can't reproduce it anymore either. I do, however, have my own fork of his version from yesterday where I briefly experimented with the issue: https://codesandbox.io/s/slateeditor-with-reduxbug-z0kiq . Just place your caret at the very end of the huge string and start typing as fast as you can. You'll likely get an exception within a couple of characters.
Sorry, I should fork my sandbox. You can try again. The bug is there. https://codesandbox.io/s/react-example-ny1ok
Hmm, @YacheLee I still don't get an error. It responds very slowly and jumps the scroll around if I add text to the end of the very long line, but it doesn't crash.
@JirkaVebr example was crashing on the first keypress because they re-implemented Redux using useSelector instead connect. However, it was missing useDispatch, so actions were not making it to the reducer. Once I added that, it behaved like @YacheLee's sandbox, i.e. not crashing...
I'd recommend that in the interest of keeping the onChange loop as tight as possible that the broadcast to redux be treated as a one-way send.
Yes, I understand the desire to keep state consistently in redux if possible, but unless you have a specific reason to have slate read from your redux store, it seems more performant to not subscribe to it.
I'd recommend that in the interest of keeping the onChange loop as tight as possible that the broadcast to redux be treated as a one-way send.
Yes, I understand the desire to keep state consistently in redux if possible, but unless you have a specific reason to have slate read from your redux store, it seems more performant to not subscribe to it.
@CameronAckermanSEL I did implemented in a similar way, but had to use flags to indicate that the new change is coming in from a different place so update the local state in synch with the new redux state. I am thinking same thing I'll have to follow when I start working my way up to a collaborative editor
@CameronAckermanSEL @JirkaVebr Since updating the redux store too frequently seems like a bad design decision with Slate, then should I close this issue? I opened this issue mainly on the lines that it may cause lot of issues with building collaborative editor, since we are not subscribing to the redux store
As I said: we had the same issue with Apollo Client. And we did exactly the same: we kept the state close to the editor. However, I suppose people will run into that issue again and again since it's pretty common to hold you state in a global data store like Redux. Maybe we could add a hint in the docs at least? 馃
@alfechner I agree, it would be better if we can add a hint in the docs, because I spent quite a lot of time in making it work and then bypassing it altogether.
I have the same issue. as soon as SlateEditor is connected to store it blocks the thread. I used debounce, it dispatch the editor to the store but then editor turned to be read only, i could not write anything to the screen. So as far as I learn, slate editor cannot connect to the redux store?
@yilmazbingo did you read the advice in the thread and make dispatch to the redux store a one way operation? Or are you also consuming the value updates asynchronously from the redux store? Doing the latter is going to cause a delay in slate receiving value updates and is going to make it not work correctly.
I'm seeing the error thrown only in older versions of chrome (ie version 75), if I try it with the current version (83) I cannot reproduce the error.
I checked with the following: https://codesandbox.io/s/react-example-ny1ok
Most helpful comment
I am facing the same issue. I used debounce to solve this problem.
This works for me. As a React developer, we like to put everything into the single source to manage state. However, if you need to update a big big big state like our SlateEditor, you have to bypass it by storing the latest value and skip unnessary updates to Redux store. For instance, if a user keeps typing 12345678999999999 in SlateEditor. The SlateEditor keeps updating the value like "1", "12", "123", "1234". Our Redux store will be updated too frequently. It's costy becaues the state of SlateEditor is really huge. It's a JSON format, not just a pure text. What we can do is to use debounce. The logic is our Redux store skips "1", "12", "123" until we found user have finished their input, and then store a complete string "12345678999999999" into our Redux store.