fix(core): log missing errorInfo in React 18 onRecoverableError callback#9387
fix(core): log missing errorInfo in React 18 onRecoverableError callback#9387slorber merged 2 commits intofacebook:mainfrom
Conversation
✅ [V2]
To edit notification comments on pull requests, go to your Netlify site configuration. |
⚡️ Lighthouse report for the deploy preview of this PR
|
slorber
left a comment
There was a problem hiding this comment.
looks like lint/prettier fails
|
|
||
| const onRecoverableError = (error: unknown): void => { | ||
| console.error('Docusaurus React Root onRecoverableError:', error); | ||
| const onRecoverableError = (error: unknown, errorInfo: { digest?: string | null, componentStack?: string | null }): void => { |
There was a problem hiding this comment.
Thanks, missed that we could get some additional info in the callback
Why not using the official type ReactDOM.ErrorInfo?
There was a problem hiding this comment.
the official type ReactDOM.ErrorInfo
Oh. where is that type found? I used the implementation I found in the React codebase - do you have another link?
There was a problem hiding this comment.
Doesn't show up in the repo? https://github.com/search?q=repo%3Afacebook%2Freact%20ReactDOM.ErrorInfo&type=code
There was a problem hiding this comment.
probably only exists on external typedefs, fixed
There was a problem hiding this comment.
Oh so the type you're using is from DefinitelyTyped: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/29f3f65685aba9e9b8a105a55494923980e2f16e/types/react/index.d.ts#L3240
Ironically, I'm one of the maintainers of that. It's slightly inaccurate too :-). But it doesn't really matter - we just want the console.log to take place
There was a problem hiding this comment.
I've raised a PR to update the Definitely Typed definition to be more accurate: DefinitelyTyped/DefinitelyTyped#66987
There was a problem hiding this comment.
yes I know that's why I didn't understand 😅 great if you catched a typo
Pre-flight checklist
Motivation
See #9379
Test Plan
I hope to use this to diagose #9379
Test links
Related issues/PRs
See #9379