[lexical] temporarily set editor to not writable when calling getCachedTypeToNodeMap

[potatowagon]

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

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
lexical	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jan 7, 2025 4:30pm
lexical-playground	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jan 7, 2025 4:30pm

[github-actions]

size-limit report 📦

Path	Size
lexical - cjs	31.39 KB (0%)
lexical - esm	31.19 KB (0%)
@lexical/rich-text - cjs	40.47 KB (0%)
@lexical/rich-text - esm	33.12 KB (0%)
@lexical/plain-text - cjs	39.04 KB (0%)
@lexical/plain-text - esm	30.37 KB (0%)
@lexical/react - cjs	42.3 KB (0%)
@lexical/react - esm	34.42 KB (0%)

[etrepum]

This doesn’t seem like the right thing to do, and would cause extra events to fire, what are the preconditions for this problem?

[potatowagon]

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 editor.registerNodeTransform being called from a useEffect in workplace. that cause workplace post composer to break. i fixed it on the intergration layer, but not sure where else could break as a result of getCachedTypeToNodeMap being called with a writable editor state

[potatowagon]

and would cause extra events to fire

this looks correct based on the unit tests failures

[etrepum]

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

[etrepum]

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).

[etrepum]

I think the root cause is probably setRootElement(null) which probably just shouldn't be doing this

        // 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;

[ivailop7]

With Bob on this one, this doesn't seem like the right approach to solve this problem.

[etrepum]

#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 _editorState

[potatowagon]

#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 _editorState

replied on discord to answer the how, closing this PR since its not the right approach