React18 #793
React18 #793
Conversation
| @@ -294,7 +294,6 @@ class ReactQuill extends React.Component<ReactQuillProps, ReactQuillState> { | |||
| destroyEditor(): void { | |||
| if (!this.editor) return; | |||
| this.unhookEditor(this.editor); | |||
| delete this.editor; | |||
There was a problem hiding this comment.
Are there any negative consequences to doing this that you can tell? Do we leak memory or something?
There was a problem hiding this comment.
Hmm,
I did a little experiment there:
I'm not deleting this on unmount, yet GC seems to be picking it up:
It's a pity Quill doesn't have proper clean up method, I guess the right way would be to call it on unmount.
https://reactjs.org/docs/strict-mode.html#ensuring-reusable-state
reactwg/react-18#18
There was a problem hiding this comment.
What do you think @alexkrolick ?
There was a problem hiding this comment.
It's a pity Quill doesn't have proper clean up method, I guess the right way would be to call it on unmount.
Are you suggesting a change here?
There was a problem hiding this comment.
@g12i, were you suggesting a change?
There was a problem hiding this comment.
@AndreaEsposit I was suggesting that maybe it would better from React's strict mode point of view, if the Quill had destroy() method. It doesn't. I think this PR should still be merged as is.
|
Any updates on this? |
|
I'm also waiting for this to be merged. |
|
Same |
|
Ok, thanks for the fix. May take a bit to release to NPM. |
@alexkrolick will there be a new beta release number? currently on npm there is version |
|
@mtraj good question. I assume this will come alongside the next beta release (beta.5), right? @alexkrolick |
|
When will this be available through NPM? Still can't install on react 18. Its stopping us from deploying to our host! |
Fixes #784
Covers #781