Excalidraw perf release #7651
Excalidraw perf release #7651
Conversation
WalkthroughThis pull request updates the package version and refactors the collaboration logic for scene synchronization in the Excalidraw whiteboard module. In the Collab class, socket initialization logic is streamlined by removing the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Collab
participant Portal
Client->>Collab: startCollaboration()
alt Scene not initialized
Collab->>Portal: Check sceneInitilized status
Portal-->>Collab: false
Collab->>Collab: Initialize scene data
else Scene initialized
Note right of Collab: Skip initialization
end
Client->>Collab: syncScene(elements, files)
Collab->>Collab: throttledSyncScene(elements, files)
Collab->>Client: Acknowledge sync action
Possibly related PRs
Suggested reviewers
✨ 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
Documentation and Community
|
![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 (5)
src/domain/common/whiteboard/excalidraw/collab/excalidrawAppConstants.ts (1)
44-47: Performance optimization: Adjusted sync intervals for better resource utilization.The changes reduce network traffic by:
- Doubling the full scene sync interval to 10 seconds
- Reducing cursor update frequency to 10fps
- Adding scene sync throttling at 10ms
Consider monitoring these new timings in production to ensure they provide a good balance between performance and user experience.
src/domain/common/whiteboard/excalidraw/collab/Portal.ts (2)
46-46: Fix typo in property name.The property name contains a typo: "sceneInitilized" should be "sceneInitialized".
- sceneInitilized: boolean = false; // we don't want the socket to emit any updates until it is fully initialized + sceneInitialized: boolean = false; // we don't want the socket to emit any updates until it is fully initialized
139-139: Consistent property naming.Update the property name here as well to fix the typo.
- return !!(this.sceneInitilized && this.socket && this.roomId); + return !!(this.sceneInitialized && this.socket && this.roomId);src/domain/common/whiteboard/excalidraw/collab/Collab.ts (2)
187-202: Improved scene initialization logic.The scene initialization check is now more explicit and includes proper state updates.
Fix the typo in the property name here as well:
- if (!this.portal.sceneInitilized) { + if (!this.portal.sceneInitialized) {
426-442: Performance optimization: Added throttled scene synchronization.The scene synchronization is now throttled to prevent excessive updates while maintaining responsiveness.
The throttle timeout of 10ms provides a good balance between responsiveness and performance, but monitor its impact in production, especially with many concurrent users.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
package.json(1 hunks)src/domain/common/whiteboard/excalidraw/collab/Collab.ts(6 hunks)src/domain/common/whiteboard/excalidraw/collab/Portal.ts(2 hunks)src/domain/common/whiteboard/excalidraw/collab/excalidrawAppConstants.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- package.json
🧰 Additional context used
📓 Path-based instructions (1)
`src/**/*.{ts,tsx,js}`: Review the React.js/TypeScript/JavaS...
src/**/*.{ts,tsx,js}: Review the React.js/TypeScript/JavaScript code for best practices and potential bugs.
Ensure that the code adheres to TypeScript's typing system and modern standards.
Use the following code guide documentation in the codebase: 'docs/code-guidelines.md'.
Ensure sufficient error handling and logging is present, but note:
- We have a global error handler in place. So local
try/catchblocks are not required unless they provide value beyond what the global error handler already covers.- Avoid flagging the absence of local error handling in cases where the global error handler suffices.
- Avoid flagging the absence of logging in cases where the global logger is already in place (graphql usage).
Inform about the recommendation of not using non-null assertion,
except on GraphQL queries where the variable is asserted non-null and followed by a
skip:with the same variable.
Examples:
- Not Required: there's a skip
{
variables: { templateId: templateId! },
skip: !templateId,
}Check for common security vulnerabilities such as:
- SQL Injection
- XSS (Cross-Site Scripting)
- CSRF (Cross-Site Request Forgery)
- Insecure dependencies
- Sensitive data exposure
src/domain/common/whiteboard/excalidraw/collab/Portal.tssrc/domain/common/whiteboard/excalidraw/collab/excalidrawAppConstants.tssrc/domain/common/whiteboard/excalidraw/collab/Collab.ts
🔇 Additional comments (1)
src/domain/common/whiteboard/excalidraw/collab/Collab.ts (1)
218-228: Performance optimization: Deferred mouse location handling.Mouse location updates are now processed only after scene initialization, which can improve initial loading times.
#7641
Summary by CodeRabbit
Chores
Refactor