Fix collab editor sometimes not loading, again #788
Conversation
Dug deeper and filed ueberdosis/tiptap#7346 to TipTap. When `useEditor`'s parameters change, it appears to call the `useEditorState`'s selectors with an uninitialized editor, causing errors. This patch works around that by checking the editor's `isInitialized` field in each selector.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #788 +/- ##
=======================================
Coverage 91.34% 91.34%
=======================================
Files 367 367
Lines 20728 20728
=======================================
+ Hits 18934 18935 +1
+ Misses 1794 1793 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Add an error boundary component that catches render errors and renders an error alert. Previously React would simply remove the entire form without conveying anything wrong to the user.
There was a problem hiding this comment.
Pull request overview
This PR addresses a recurring issue where the collaborative editor sometimes fails to load. The fix works around a TipTap bug where useEditor's parameters changes cause useEditorState selectors to be called with an uninitialized editor. The PR also adds an ErrorBoundary component to gracefully handle editor errors.
- Adds
isInitializedchecks in all editor state selectors to prevent errors when the editor is not ready - Introduces an ErrorBoundary component to catch and display errors in the collaborative editor
- Wraps all form components with ErrorBoundary for better error handling
- Includes minor CSS formatting fixes (indentation and color hex case)
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| javascript/src/frontend/collab_forms/rich_text_editor/table.tsx | Adds isInitialized check to TableCaptionBookmarkButton selector |
| javascript/src/frontend/collab_forms/rich_text_editor/link.tsx | Adds isInitialized check to LinkButton selector |
| javascript/src/frontend/collab_forms/rich_text_editor/index.tsx | Adds isInitialized check to FormatButton selector |
| javascript/src/frontend/collab_forms/rich_text_editor/heading.tsx | Adds isInitialized check to HeadingIdButton selector |
| javascript/src/frontend/collab_forms/rich_text_editor/evidence/index.tsx | Adds isInitialized check to EvidenceButton selector |
| javascript/src/frontend/collab_forms/rich_text_editor/color.tsx | Adds isInitialized check to ColorButton selector |
| javascript/src/frontend/collab_forms/error_boundary.tsx | New ErrorBoundary component to catch React errors and display user-friendly message |
| javascript/src/frontend/collab_forms/forms/reportobservationlink.tsx | Wraps component with ErrorBoundary |
| javascript/src/frontend/collab_forms/forms/reportfindinglink.tsx | Wraps component with ErrorBoundary |
| javascript/src/frontend/collab_forms/forms/report_field.tsx | Wraps component with ErrorBoundary |
| javascript/src/frontend/collab_forms/forms/project_collabnote.tsx | Wraps component with ErrorBoundary |
| javascript/src/frontend/collab_forms/forms/observation.tsx | Wraps component with ErrorBoundary |
| javascript/src/frontend/collab_forms/forms/finding.tsx | Wraps component with ErrorBoundary |
| javascript/src/frontend/collab_forms/editor.scss | Fixes indentation and color hex formatting |
| selector: ({ editor }) => | ||
| editor.can().setTableCaptionBookmark("example"), | ||
| selector: ({ editor }) => { | ||
| if (!editor.isInitialized) return { enabled: false, active: false }; |
There was a problem hiding this comment.
The selector returns an object { enabled: false, active: false } when the editor is not initialized (line 16), but returns a boolean from editor.can().setTableCaptionBookmark("example") when initialized (line 17). This type inconsistency will cause issues because the variable enabled on line 13 expects a consistent type. The selector should return the same type in both cases.
| if (!editor.isInitialized) return { enabled: false, active: false }; | |
| if (!editor.isInitialized) return false; |
| this.state = { hasError: false }; | ||
| } | ||
|
|
||
| static getDerivedStateFromError(error: Error) { |
There was a problem hiding this comment.
The getDerivedStateFromError method has an error parameter that is declared but never used. Consider removing the parameter name or prefixing it with an underscore (e.g., _error) to indicate it's intentionally unused.
| static getDerivedStateFromError(error: Error) { | |
| static getDerivedStateFromError(_error: Error) { |
| return { hasError: true }; | ||
| } | ||
|
|
||
| componentDidCatch(error: Error, errorInfo: React.ErrorInfo): void { |
There was a problem hiding this comment.
The componentDidCatch method has error and errorInfo parameters that are declared but never used. The comment indicates "React will print the error," but if this method serves no purpose beyond that, consider removing it entirely since React handles error logging by default. If you want to keep it for future error logging, prefix the parameters with underscores (e.g., _error, _errorInfo).
| componentDidCatch(error: Error, errorInfo: React.ErrorInfo): void { | |
| componentDidCatch(_error: Error, _errorInfo: React.ErrorInfo): void { |
3 participants
Dug deeper and filed ueberdosis/tiptap#7346 to TipTap. When
useEditor's parameters change, it appears to call theuseEditorState's selectors with an uninitialized editor, causing errors. This patch works around that by checking the editor'sisInitializedfield in each selector.