[lexical][lexical-overflow] Refactor: simplified removeText and insertText rewrite (part 1)

[GermanJablo]

This is something I've been wanting to do since I did the insertNodes rewrite. These are also methods with long and very complex implementations.

Currently, removeText depends on insertText, since removeText is defined as inserting an empty string (insertText("")). This PR seeks to reverse the dependency of these methods, making insertText dependent on removeText.

This can be achieved in 2 steps:

1. Rewrite removeText without making it dependent on insertText.
2. Rewrite insertText with the new removeText.

So far in this PR, I have managed to complete step 1 and 95% of step 2. I decided to leave step 2 for a later PR because there are about 4 tests that fail and, after having dedicated so much time to it, I want to do a pause on this.

[vercel[bot]]

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	Sep 4, 2024 11:11pm
lexical-playground	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Sep 4, 2024 11:11pm

[github-actions[bot]]

size-limit report 📦

Path	Size
lexical - cjs	29.38 KB (0%)
lexical - esm	29.22 KB (0%)
@lexical/rich-text - cjs	37.87 KB (0%)
@lexical/rich-text - esm	31.08 KB (0%)
@lexical/plain-text - cjs	36.45 KB (0%)
@lexical/plain-text - esm	28.44 KB (0%)
@lexical/react - cjs	39.64 KB (0%)
@lexical/react - esm	32.52 KB (0%)

[GermanJablo]

Yes, to be continued in another PR

[etrepum]

I think some of the E2E tests are failing, I was hoping that it was just flakiness but it has consistently failed since I turned on extended tests. #6488 is passing (after a flaky test failure) which is based on the same main commit.

[potatowagon]

I think some of the E2E tests are failing, I was hoping that it was just flakiness but it has consistently failed since I turned on extended tests. #6488 is passing (after a flaky test failure) which is based on the same main commit.

yea quite abit of e2e plaintext tests failures that are not known to be flaky: https://github.com/facebook/lexical/actions/runs/10251936357/job/28361149775?pr=6456

[GermanJablo]

done @etrepum

[ivailop7]

Let's do 0.17.1 first and then do this one as a 0.18 with the defaults change.

[adrianmxb]

I am a bit late to the party but the changes here break removal of TextNodes in token mode. @ivailop7 @GermanJablo

One example

[etrepum]

Do you mind creating a new issue so that this isn't lost?

[GermanJablo]

Isn't that supposedly the correct behavior, or am I missing something? Are you sure it worked differently before? There were a lot of those tests I had to pass.

[adrianmxb]

Chose an unfortunate example here, the mention node created in the GIF is in token mode to demo the issue.

Anyway, our users can insert placeholder nodes, which are basically a custom text node in token mode.

According to the docs:

mode token - acts as immutable node, can't change its content and is deleted all at once

Before these changes, token nodes behaved as described in the documentation.

For segmented mode the behaviour is correct, I updated my initial comment, sorry for that one.