[lexical] surface more error details in reconciler #6511
Conversation
potatowagon
requested review from
zurfyx,
fantactuka,
acywatson,
Fetz,
ivailop7 and
Sahejkm
as code owners
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
facebook-github-bot
added
the
CLA Signed
size-limit report 📦
|
| @@ -501,7 +512,22 @@ function $reconcileChildren( | |||
| } else { | |||
| const lastDOM = getPrevElementByKeyOrThrow(prevFirstChildKey); | |||
| const replacementDOM = $createNode(nextFrstChildKey, null, null); | |||
| dom.replaceChild(replacementDOM, lastDOM); | |||
| try { | |||
| dom.replaceChild(replacementDOM, lastDOM); | |||
There was a problem hiding this comment.
what could cause The node to be replaced is not a child of this node error here?
seems like the error suggest lastDOM is not a child of dom. did it get deleted before this line exec? and how?
| @@ -340,7 +340,18 @@ function reconcileElementTerminatingLineBreak( | |||
| const element = dom.__lexicalLineBreak; | |||
|
|
|||
| if (element != null) { | |||
| dom.removeChild(element); | |||
| try { | |||
| dom.removeChild(element); | |||
There was a problem hiding this comment.
what could cause The node to be removed is not a child of this node error here?
is dom.__lexicalLineBreak above deleted or returning outdated value?
| } catch (error) { | ||
| if (typeof error === 'object' && error != null) { | ||
| const msg = `${error.toString()} Parent: ${ | ||
| dom.tagName |
There was a problem hiding this comment.
what are the best practices when logging errors in lexical? any methods to log the sanitised/privacy safe domNode/lexical node/editor state?
There was a problem hiding this comment.
@potatowagon we'd use invariant and you can pass any params in, also keep in mind that internally logged errors have a not very generous max length
|
funny the extended e2e tests signal didnt show up in the signal box but i can see they ran successfully (no failures) under the Checks (51) at the top of page https://github.com/facebook/lexical/actions/runs/10352963152 |
Description
since 25 jul, observing a high number of
The node to be replaced/removed is not a child of this nodeerrors, at the following 2 lines where im adding logging. These errors spiked on 6 Aug but have since died down (on its own) to ~ 2 errors a dailyi've not been able to reproduce those errors so surfacing more details in the error msg to get more clues
i made sure to only surface privacy safe details in the error
fix attempt: #6513
Test plan
threw a new error there to emulate the error.
to trigger that line, replace an entire paragraph node, eg. pasting over the entire paragraph node