Repository navigation
Conversation
…s text node's text, so transforms between two formatted nodes settle ## Description `registerLexicalTextEntity`'s text node transform found the first match in a node in that node's own text, while the previous text node's transform checks a match at its end against the following node's text too. Take plain text between two text nodes of another format (bold "x", "dogcat", bold "y") and type a space after "dog": - the transform of "dog cat" made "dog" an entity, since "dog" matches at the start of its own text; - the transform of "x" then undid it, since "xdog" has no "dog"; - the two nodes merged again, and so on, until Lexical threw "One or more transforms are endlessly triggering additional transforms" and the update was lost. The first match in a node is now found after the text of a plain previous sibling, as that sibling's transform checks it, and so is the check against the next sibling's text that follows it. Later matches in the node are found as before. Closes #9637 ## Test plan `registerLexicalTextEntityContext.test.ts` (new, 6 tests): - the issue's two cases, now with no entity; - three controls where the match holds next to the text before it, with "dog" an entity; - 2,000 random texts of keywords and other pieces between two bold nodes, a character typed anywhere in the middle one. ### Before On main (17154c2): ``` $ pnpm exec vitest run --project unit packages/lexical-text/src/__tests__/unit/registerLexicalTextEntityContext.test.ts × bold "x" | "dogcat" | bold "y", a space after "dog": no entity × bold "x" | "dogcat" | bold "dog", an emoji after "dog": no entity × random texts between formatted neighbours settle Error: One or more transforms are endlessly triggering additional transforms. May have encountered infinite recursion caused by transforms that have their preconditions too lose and/or conflict with each other. Tests 3 failed | 3 passed (6) ``` ### After ``` $ pnpm exec vitest run --project unit packages/lexical-text/src/__tests__/unit/registerLexicalTextEntityContext.test.ts Tests 6 passed (6) $ pnpm exec vitest run --project unit --project scripts-unit Tests 27691 passed | 204 expected fail | 135 skipped (28030) $ pnpm run ci-check Found 0 errors ``` The issue's repro passes 4 of 4 (main: 2 of 4). The other `@lexical/text`, hashtag, React and playground unit tests (577) pass.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
zurfyx
requested review from
acywatson,
etrepum,
ivailop7 and
potatowagon
as code owners
October 31, 2026 03:23
meta-cla
Bot
added
the
CLA Signed
|
AI review status for this pull request.
|
lexical
@lexical/a11y
@lexical/clipboard
@lexical/code
@lexical/code-core
@lexical/code-prism
@lexical/code-shiki
@lexical/compiler
@lexical/devtools-core
@lexical/dragon
@lexical/eslint-plugin
@lexical/extension
@lexical/fast-check
@lexical/file
@lexical/hashtag
@lexical/headless
@lexical/history
@lexical/html
@lexical/internal
@lexical/link
@lexical/list
@lexical/mark
@lexical/markdown
@lexical/mdast
@lexical/offset
@lexical/overflow
@lexical/plain-text
@lexical/react
@lexical/rich-text
@lexical/selection
@lexical/table
@lexical/tailwind
@lexical/text
@lexical/utils
@lexical/y
@lexical/yjs
commit: |
…s text ## Description Review of the previous commit: with a matcher whose match needs the text before it (a lookbehind, `/(?<=@)dog/`), the first match in "dog" after a bold "@" was found as "dog" and made an entity. The entity's own transform then checks it on its own text, where there is no "@", and undid it, so the transforms looped again (main never makes it an entity, and settles). A first match found after the previous sibling's text must now also be a match of its own text. If it is not, it is left as text, and the rest of the node is matched as before. Where a match is split out after such skipped text, it is split at its offset in the node (a match at the start of the rest is not at the start of the node). ## Test plan `registerLexicalTextEntityContext.test.ts` adds: - a lookbehind after a bold "@": no entity, and the update goes through; - a lookbehind or a word: the word is still an entity; - a word boundary after a bold letter (no entity) and after a bold space (an entity). ### Before On d8f3d34, the review's repro: ``` $ pnpm exec vitest run --project unit review-9655-Lookbehind.test.ts Error: One or more transforms are endlessly triggering additional transforms. Tests 2 failed (2) ``` ### After ``` $ pnpm exec vitest run --project unit packages/lexical-text/src/__tests__/unit/registerLexicalTextEntityContext.test.ts Tests 10 passed (10) $ pnpm exec vitest run --project unit --project scripts-unit Tests 27695 passed | 204 expected fail | 135 skipped (28034) $ pnpm run ci-check Found 0 errors ``` - The review's repro: 2 of 2 pass. - On main, the 4 new cells pass and the 3 earlier ones fail, as before.
…t createNode gives it ## Description Review of the previous commit: a first match found after the previous sibling's text was kept only if its source text matched on its own. An entity node is checked on its own text, and that is the text `createNode` gives it, which may differ (a `createNode` that lower-cases). With `/dog|(?<=@)DOG/` and a lower-casing `createNode`, "@Dog" after a bold "x" made no entity, where main makes "dog". The skip now uses the predicate the entity's own transform uses (`$reverseNodeTransform`: a match of the whole text the entity holds). If the source text does not match on its own, the match stays an entity when the text `createNode` gives it does. That text comes from a detached node, which the update's garbage collection removes. A source text that matches on its own is taken as is, as matches found without the previous sibling's text always were. ## Test plan `registerLexicalTextEntityContext.test.ts` adds: a lookbehind match that a lower-casing `createNode` makes a match of its own (an entity, as on main). ### Before On d51d2a6: ``` $ pnpm exec vitest run --project unit packages/lexical-text/src/__tests__/unit/registerLexicalTextEntityContext.test.ts × a lookbehind match that createNode makes a match of its own: an entity AssertionError: expected { entities: [], text: 'x@DOG!' } to deeply equal { entities: [ 'dog' ], text: 'x@dog!' } ``` ### After ``` $ pnpm exec vitest run --project unit packages/lexical-text/src/__tests__/unit/registerLexicalTextEntityContext.test.ts Tests 11 passed (11) $ pnpm exec vitest run --project unit --project scripts-unit Tests 27696 passed | 204 expected fail | 135 skipped (28035) $ pnpm run ci-check Found 0 errors ``` - The review's repro passes 2 of 2, and its lookbehind repro still does. - On main, the new cell passes (main makes the entity) and the 3 earlier cells fail, as before.
zurfyx
marked this pull request as draft
October 31, 2026 03:54
![meta-code-review[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/5334748?s=60&v=4){kind=small}
There was a problem hiding this comment.
Thanks, this looks good. The middle node's transform now finds its first match after the previous plain sibling's text. That makes it agree with the check the sibling's own transform already makes against its next sibling, which is what breaks the loop in #9637, and the new tests cover both cases from the issue. The suggestions below are optional and non-blocking.
Optional suggestions
- Context after a skipped lookbehind match (
packages/lexical-text/src/registerLexicalTextEntity.ts, lines 178-180). After the skip, the rest of the node is matched on its own, without the skipped characters in front of it. Take a matcher like/(?<=@)dog|\bcat\b/and a bold@followed by plaindogcat:mainleavesdogcatas text, while this branch turnscatinto an entity in the middle of the word. Lookbehind matchers already loop onmainwhen@dogcatsits in a single node, so this is an edge case. Still, the comment at lines 176-177 ("the rest of this node is matched as it was") overstates it. You could either note in theregisterLexicalTextEntityJSDoc thatgetMatchshould not depend on text before the match (an entity is re-checked on its own text in$reverseNodeTransform), or add a test that pins this case. createNodeas a probe (lines 95-100).$staysEntitycan now call the caller'screateNodeon a detached text node just to see what text it produces. The JSDoc forcreateNode(line 39) only describes creating the replacement node. A sentence saying it may also be called on a detached node, so it should not have side effects or rely on the node's parent, would help anyone passing a non-trivialcreateNode.- Repeated
getMatch(line 169). On the first iteration,matchAfter(before, remainingText)evaluatesgetMatch(previousText + text), the same stringprevMatchwas computed from at line 124. ReusingprevMatchthere saves one scan per transform.
The changeset follows CONTRIBUTING.md: a [fix] entry for @lexical/text with the Verify: paragraph a behavior-changing fix needs.
| // made an entity and undone again: it is left as text, and the | ||
| // rest of this node is matched as it was. | ||
| prevMatchLengthToSkip += match.end; | ||
| text = remainingText.slice(match.end); |
There was a problem hiding this comment.
Optional: after this skip, the next iteration matches the rest of the node on its own, without the skipped characters in front of it. With /(?<=@)dog|\bcat\b/ and a bold @ followed by plain dogcat, main leaves dogcat as text, while this branch makes cat an entity in the middle of the word. Lookbehind matchers already loop on main for @dogcat in one node, so this is an edge case. Consider documenting that getMatch should not depend on text before the match, or pinning this case in a test. The comment above ("matched as it was") overstates it a little.
| return true; | ||
| } | ||
| // A detached node, which the update's garbage collection removes. | ||
| const made = createNode( |
There was a problem hiding this comment.
Optional: this calls the caller's createNode on a detached node just to read the text it produces. Worth a sentence in the createNode JSDoc (it may be called on a detached node, so it should be free of side effects and not rely on the node's parent).
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Parked (10/30)
Draft. The loop in #9637 comes from the two transforms checking a match against different text:
createNodegives it.This pull request makes the forward check use the previous node's text. Each way of making the reverse check agree has broken a case main handles:
createNodethat changes the text (it lower-cases it, say);createNodeon a detached probe node changes whatcreateNodereceives. A factory that reads the source node's parent then throws and loses the typed text, and one that reads the source node's NodeState misses the entity (review: 2 of 2 each; main passes).Making the checks agree without calling the app's factory on a probe needs an API change: for example an optional pure
matches(text, context)callback, or a documented contract thatgetMatchdecides on the text alone, with no lookbehind, andcreateNodekeeps the text. #9637 stays open.Description
registerLexicalTextEntity's text node transform found the first match in a node in that node's own text, while the previous text node's transform checks a match at its end against the following node's text too. Take plain text between two text nodes of another format (bold "x", "dogcat", bold "y") and type a space after "dog":The first match in a node is now found after the text of a plain previous sibling, as that sibling's transform checks it, and so is the check against the next sibling's text that follows it. Later matches in the node are found as before. A first match found that way must also stay an entity under the check the entity's own transform makes: its whole text, as
createNodegives it, must be a match. A match that needs the text before it (a lookbehind) is left as text, and the rest of the node is matched as before; one thatcreateNodemakes a match of its own (say by lower-casing it) is an entity, as on main.Closes #9637
Changeset
lexical-text-entity-context.md(@lexical/text, fix): StopregisterLexicalTextEntitytransforms from looping when text is typed between two text nodes of another format.Test plan
registerLexicalTextEntityContext.test.ts(new, 11 tests):createNodemakes a match of its own (an entity).Before
On main (17154c2):
After
The issue's repro passes 4 of 4 (main: 2 of 4). The other
@lexical/text, hashtag, React and playground unit tests (577) pass.