[lexical] temporarily set editor to not writable when calling getCachedTypeToNodeMap#7022
[lexical] temporarily set editor to not writable when calling getCachedTypeToNodeMap#7022potatowagon wants to merge 4 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
size-limit report 📦
|
|
This doesn’t seem like the right thing to do, and would cause extra events to fire, what are the preconditions for this problem? |
im not sure what other side effects this would introduce, trying it out in a draft PR to experiment, so happy to close if this PR will cause nasty side effects. this is a followup to the breaking change in #6894. one trigger is a case of |
this looks correct based on the unit tests failures |
|
This should only be a problem if getEditorState returns a writable state, it’s never supposed to, because that is the state that was most recently reconciled. There is a workaround in there to handle an initial empty EditorState which is not frozen, but maybe there is also a case where your team is creating a non-empty EditorState that is not frozen but still returned from getEditorState somehow? I’m not currently at a computer to look into this but if you can come up with a repro I’m sure we can fix this cleanly |
|
In this scenario what I would do, if I couldn't diagnose and fix the root cause of editor.getEditorState() returning a non-empty writable EditorState under some condition, is expose computeTypeToNodeMap and call that instead when this issue would trigger. You really don't want to populate this cache with a writable EditorState because there's no way to invalidate it if the EditorState changes (because it's not supposed to). |
|
I think the root cause is probably // If content editable is unmounted we'll reset editor state back to original
// (or pending) editor state since there will be no reconciliation
this._editorState = pendingEditorState;
this._pendingEditorState = null;
this._window = null; |
|
With Bob on this one, this doesn't seem like the right approach to solve this problem. |
|
#7023 is a cleaner approach, but I can't be entirely sure this is exactly how they are getting the editor to have a writable |
replied on discord to answer the how, closing this PR since its not the right approach |
Description
Describe the changes in this pull request
Closes #
Test plan
Before
Insert relevant screenshots/recordings/automated-tests
After
Insert relevant screenshots/recordings/automated-tests