[PE-210] feat: editor performance #6269
Conversation
WalkthroughThis pull request focuses on updating dependencies and refactoring the editor-related code in the Plane project. The changes primarily involve upgrading package versions across multiple files, including Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Palanikannan1437
force-pushed
the
fix/upgrade-editor
branch
from
cd8212f to
a207653
Compare
Palanikannan1437
force-pushed
the
fix/upgrade-editor
branch
2 times, most recently
from
6985452 to
bbe378e
Compare
Palanikannan1437
force-pushed
the
fix/upgrade-editor
branch
from
bbe378e to
5ff8514
Compare
Palanikannan1437
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/editor/src/core/helpers/insert-content-at-cursor-position.ts (1)
14-15: Consider using early return pattern for better readability.The position calculation could be simplified using early returns.
- const docSize = editor.state.doc.content.size; - const safePosition = Math.max(0, Math.min(editor.state.selection.anchor, docSize)); + if (!editor.state.selection.anchor) return; + const safePosition = Math.min(editor.state.selection.anchor, editor.state.doc.content.size);packages/editor/src/core/hooks/use-editor.ts (2)
117-121: Improve error boundary for selection handling.Consider wrapping the selection handling in a separate try-catch block to prevent the entire content sync from failing.
try { editor.commands.setContent(value, false, { preserveWhitespace: "full" }); + } catch (error) { + console.error("Error syncing editor content:", error); + } + try { if (editor.state.selection) { const docLength = editor.state.doc.content.size; const relativePosition = Math.min(editor.state.selection.from, docLength - 1); editor.commands.setTextSelection(relativePosition); } } catch (error) { - console.error("Error syncing editor content with external value:", error); + console.error("Error restoring selection:", error); }
Line range hint
236-254: Optimize text selection performance.The
getSelectedTextimplementation could be optimized by reducing DOM operations.if (empty) return null; const nodesArray: string[] = []; + const tempDiv = document.createElement("div"); state.doc.nodesBetween(from, to, (node, _pos, parent) => { if (parent === state.doc && editor) { const serializer = DOMSerializer.fromSchema(editor.schema); const dom = serializer.serializeNode(node); - const tempDiv = document.createElement("div"); tempDiv.appendChild(dom); nodesArray.push(tempDiv.innerHTML); + tempDiv.innerHTML = ''; } });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (7)
live/package.json(2 hunks)packages/editor/package.json(3 hunks)packages/editor/src/core/components/menus/menu-items.ts(1 hunks)packages/editor/src/core/helpers/insert-content-at-cursor-position.ts(1 hunks)packages/editor/src/core/hooks/use-editor.ts(10 hunks)packages/editor/src/core/hooks/use-read-only-editor.ts(5 hunks)packages/editor/src/core/types/editor.ts(0 hunks)
💤 Files with no reviewable changes (1)
- packages/editor/src/core/types/editor.ts
🔇 Additional comments (9)
packages/editor/src/core/helpers/insert-content-at-cursor-position.ts (1)
3-4: LGTM! Improved function signature.The change from
MutableRefObject<Editor | null>to directEditorinstance reduces indirection and simplifies the code.packages/editor/src/core/hooks/use-read-only-editor.ts (2)
43-46: Performance optimization through render control.Good addition of
immediatelyRenderandshouldRerenderOnTransactionflags to optimize rendering performance.
78-81: Verify the impact of skipImageDeletion meta flag.The
skipImageDeletionmeta flag is used in clearEditor. Ensure this doesn't affect image cleanup.Also applies to: 84-85
✅ Verification successful
The skipImageDeletion meta flag implementation is correct
The flag is properly used in the TrackImageDeletionPlugin to skip cleanup only during programmatic content clearing, while maintaining normal image deletion functionality for user actions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for skipImageDeletion usage and related image cleanup logic rg -A 5 "skipImageDeletion|imageCleanup"Length of output: 1937
Script:
#!/bin/bash # Search for image-related tests and handlers rg -g "*.test.*" -A 5 "image|Image" # Search for additional image-related logic in plugins ast-grep --pattern 'class $_ extends Node { $$$ get name() { return "image" } $$$ }'Length of output: 285
Script:
#!/bin/bash # Search for test files fd -e test.ts -e test.tsx -x rg -l "image|Image" {} # Search for image node and deletion logic rg -g "*.ts" -A 10 "class.*Image|deleteImage|onDelete.*image" # Search for image-related extensions or plugins rg -g "*.ts" "addImage|createImage|uploadImage"Length of output: 4970
packages/editor/src/core/components/menus/menu-items.ts (1)
205-205: LGTM! Simplified selection handling.Good change to use editor's selection state directly instead of maintaining a separate saved selection.
packages/editor/src/core/hooks/use-editor.ts (1)
77-78: Performance optimization through render control.Good addition of render control flags. However, verify if setting both flags to false doesn't affect editor responsiveness.
live/package.json (2)
43-43: Verify y-prosemirror and yjs compatibilityThe updates to y-prosemirror (1.2.9 → 1.2.15) and yjs (13.6.14 → 13.6.20) appear to be patch versions, but we should verify compatibility between these interdependent packages.
Also applies to: 45-45
✅ Verification successful
✓ Compatibility verified between y-prosemirror 1.2.15 and yjs 13.6.20
Both updates contain only bug fixes and improvements with no breaking changes or compatibility issues reported in their release notes. The changes enhance stability and fix various edge cases while maintaining backward compatibility.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for breaking changes in release notes echo "Checking y-prosemirror changelog..." gh api repos/yjs/y-prosemirror/releases | jq -r ".[].body" echo "Checking yjs changelog..." gh api repos/yjs/yjs/releases | jq -r ".[].body"Length of output: 20227
19-22: Verify @hocuspocus packages compatibilityThe @hocuspocus packages have been updated to v2.15.0. While this appears to be a minor version bump, we should verify there are no breaking changes.
✅ Verification successful
@hocuspocus package updates appear safe
The minor version bump (2.15.0) with consistent versioning across all @hocuspocus packages, combined with standard API usage patterns in the codebase, suggests this update is safe. The codebase uses well-established APIs that are unlikely to change in minor versions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for breaking changes in release notes for pkg in extension-database extension-logger extension-redis server; do echo "Checking @hocuspocus/$pkg changelog..." gh api repos/ueberdosis/hocuspocus/releases | jq -r ".[] | select(.tag_name | contains(\"$pkg\")) | .body" doneLength of output: 1180
Script:
#!/bin/bash # Check peer dependencies and usage patterns echo "Checking package dependencies..." fd package.json --exec cat {} \; | jq -r '.peerDependencies | select(. != null)' echo -e "\nChecking @hocuspocus usage patterns..." rg "@hocuspocus" --type ts --type js -A 2 -B 2Length of output: 2690
packages/editor/package.json (2)
15-15: Verify impact of removing module exportsThe removal of the "module" field from exports could affect bundlers' module resolution. Please ensure this change doesn't impact build optimization.
Consider testing the build process to verify:
- Bundle size hasn't increased
- Tree-shaking still works effectively
- No resolution errors in dependent packages
Also applies to: 20-20
37-37: Package versions aligned with live packageGood job aligning the versions across packages:
- @hocuspocus/provider: ^2.15.0
- tiptap-markdown: ^0.8.10
- y-prosemirror: ^1.2.15
- yjs: ^13.6.20
Let's verify the tiptap-markdown update:
Also applies to: 68-68, 71-71, 73-73
✅ Verification successful
Package updates verified as safe
All package version updates are safe to use:
- @hocuspocus/provider ^2.15.0: Adds thread features and bug fixes
- y-prosemirror ^1.2.15: Contains cursor selection bug fixes
- yjs ^13.6.20: Adds
isDestroyedproperty and bug fixes🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check tiptap-markdown changelog for breaking changes echo "Checking tiptap-markdown changelog..." gh api repos/aguingand/tiptap-markdown/releases | jq -r ".[].body"Length of output: 2422
Script:
#!/bin/bash # Check @hocuspocus/provider changelog echo "Checking @hocuspocus/provider changelog..." gh api repos/ueberdosis/hocuspocus/releases | jq -r ".[].body" # Check y-prosemirror changelog echo "Checking y-prosemirror changelog..." gh api repos/yjs/y-prosemirror/releases | jq -r ".[].body" # Check yjs changelog echo "Checking yjs changelog..." gh api repos/yjs/yjs/releases | jq -r ".[].body"Length of output: 41975
Palanikannan1437
changed the title
Palanikannan1437
changed the title
Palanikannan1437
changed the title
Description
Type of Change
Screenshots and Media (if applicable)
Test Scenarios
References