The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Preview	Comments	Updated (UTC)
lexical	Ready	Preview	Comment	Sep 20, 2025 6:13am
lexical-playground	Ready	Preview	Comment	Sep 20, 2025 6:13am

Please do add tests if you're fixing bugs. Without a test there's nothing stopping a future regression.

Ah, will do.

For what it's worth I think this probably has other bugs related to splitting and re-using the cached selection. I think a more reliable strategy would be to do any slicing of text nodes first, then get the extracted nodes.

The caret APIs are better at dealing with this sort of thing, but nobody has refactored extract to use them.

Makes sense. Regardless this fix is useful for avoiding a custom patch when I next update my repo from upstream.

Adding a test should also ensure a future refactor doesn't regress and break me again.

I think manually updating cachedNodes is not a good fix, setting it to null when the selection changes is what's supposed to happen, it will just be recomputed the next time getNodes is called. It's never really supposed to be updated in-place, the logic to maintain that sort of cache is very error-prone and hard to maintain. If this repo had good readonly hygiene the return type of getNodes would be a readonly array so you wouldn't be able to mutate it without copying first since it is a shared cache.

It would make sense for extract() to either do the splitting first so it can return getNodes directly (since it will have the split nodes) or it should make a copy of the array at the start or at least before it starts mutating it.

The lastIndex thing seems like a good fix if the bug is observing lastNode twice at the end of the array.

It's not expected behavior for extract() to guarantee that getNodes() gets the same output, they use subtly different algorithms. The only expected side-effect is the TextNode splitting, it shouldn't otherwise be moving the anchor and focus points around, so only the changes directly caused by splitting of TextNodes should be observable from a subsequent call to getNodes().

_cachedNodes will be cleared by splitText when the selection is the editor's selection. The cache is completely incoherent if you are mutating a selection that isn't the editor's current selection, because the mutators only concern themselves with updating the current selection not every selection object that might exist.

Both of these methods are kinda bad, which is a big reason why the caret APIs exist. Any fixes to them should be really precise, and I don't think this strategy (trying to force things into the cache that won't be computed the same way when the cache is cleared) is the right one.

The only expected side-effect is the TextNode splitting, it shouldn't otherwise be moving the anchor and focus points around, so only the changes directly caused by splitting of TextNodes should be observable from a subsequent call to getNodes().

Unfortunately sometimes the changes aren't observable regardless, which is part of the issue I was hoping to solve. For example, if I remove the _cachedNodes changes and run my tests, you can see the selection will be stale (_ denotes selection):

pa_ragrap_h text
# selection.extract() -> new TextNode("ragrap")
# selection.getNodes() -> stale TextNode("paragraph text") that is no longer in document

It would make sense for extract() to either do the splitting first so it can return getNodes directly (since it will have the split nodes) or it should make a copy of the array at the start or at least before it starts mutating it.

So I'm not sure your suggestion would work since getNodes()'s value is stale. Making selectedNodes a copy of _cachedNodes rather than a reference does seem like the other reasonable alternative to updating _cachedNodes. I can make that update, though it doesn't solve the staleness. I guess I could update the anchor/focus here, or make splitText actually update it on the selection. Curious to hear what you'd recommend.

Overall, I'm just hoping to get the nicest behavior with minimal changes. Right now my repo will fail with a stale getNodes() since that is the method used in LexicalNode#isSelected().

I don't think you quite follow what I'm saying here. The _cachedNodes should be cleared. It isn't when the selection isn't the editor's current selection, because splitText doesn't know which selection it was called from. That should be fixed, with a single this.setCachedNodes(null) in this extract method (this can even go immediately after the getNodes call). The wrong solution is to mutate the result of getNodes and/or try and manually set _cachedNodes to anything but null except when computed by getNodes.

It would also make sense to make a copy of the result of getNodes() since it's mutated in-place and that would change previously observed values. Unlikely to happen, but might as well avoid another potential bug.

Nice, happy to see the tests passing with this style rather than modifying _cachedNodes. Thanks for finishing up and merging.