[Experimental] CRDT tests with Grida Canvas #376
Conversation
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
6 Skipped Deployments
|
|
Caution Review failedThe pull request is closed. WalkthroughThis update introduces real-time multiplayer editing features to the editor, including Yjs-based document and cursor synchronization, remote cursor visualization, and a "follow" mode for tracking another user's viewport. It adds new React components, hooks, plugins, and types to support multiplayer state, improves sidebar and import/export modularity, and cleans up or refactors related UI components. Changes
Sequence Diagram(s)Multiplayer Cursor Sync and Follow FlowsequenceDiagram
participant UserA as User A (Local)
participant EditorA as Editor (Local)
participant Yjs as Yjs Provider
participant UserB as User B (Remote)
participant EditorB as Editor (Remote)
UserA->>EditorA: Move cursor / select node
EditorA->>Yjs: Update awareness (cursor, selection)
Yjs->>EditorB: Broadcast awareness state
EditorB->>UserB: Render remote cursor, selection
UserB->>EditorB: Click "Follow" on User A
EditorB->>EditorB: Activate follow mode (track User A)
EditorB->>Yjs: Subscribe to User A's cursor/scene changes
Yjs->>EditorB: Receive User A's viewport/scene updates
EditorB->>EditorB: Update viewport to follow User A
Possibly related PRs
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
softmarshmallow
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: 11
🧹 Nitpick comments (10)
editor/package.json (1)
142-146: Consolidate deep-equality libraries
You now depend on bothdeep-equalandfast-deep-equal. Consider removing one to reduce bundle size if only one implementation is used across the codebase.editor/app/(dev)/canvas/tools/io-figma/page.tsx (1)
13-13: ReorderedLinkimport for clarity
Placing external library imports after local imports improves readability.editor/grida-canvas-react-starter-kit/starterkit-import/index.tsx (1)
39-42: Consider more specific error messaging.The generic error message could be enhanced to provide more specific feedback about what went wrong during file parsing.
} catch (error) { - toast.error("Failed to parse the file. Please check the format."); + toast.error(`Failed to parse the file: ${error instanceof Error ? error.message : 'Please check the format.'}`); console.error(error); }editor/components/multiplayer/avatar.tsx (2)
37-46: Consider adding accessibility attributes.The button could benefit from proper accessibility attributes for screen readers.
<button + aria-label={`User avatar: ${avatar.fallback}${type === 'local' ? ' (You)' : ''}`} data-selected={selected} className="size-7 rounded-full bg-muted border-2 border-background focus:border-ring hover:border-ring hover:!z-10 transition data-[selected='true']:border-foreground data-[selected='true']:!z-10" style={{ borderColor: type === "local" ? undefined : colors.ring, backgroundColor: type === "local" ? undefined : colors.fill, zIndex: zIndex, }} onClick={onClick} >
68-70: Consider edge case handling for empty fallback text.The fallback function could handle edge cases where the input text might be empty or very short.
const fallback = (txt: string) => { - return txt.slice(0, 2).toUpperCase(); + return (txt || "??").slice(0, 2).toUpperCase(); };editor/grida-canvas/plugins/sync-y.ts (1)
127-127: Address the TODO comment for palette syncing.The palette is being sent with every awareness update, which is inefficient as noted in the TODO comment.
Would you like me to implement a solution that syncs the palette only once during initialization or when it changes?
editor/scaffolds/playground-canvas/library.tsx (1)
210-210: Consider moving external URLs to configuration.Hardcoded URLs for external resources make it difficult to change endpoints or use different environments.
Consider creating a configuration file or environment variables for these URLs:
https://reflect-icons.s3.us-west-1.amazonaws.com/all.jsonhttps://grida-std.s3.us-west-1.amazonaws.com/shapes-basicThis would make the application more maintainable and environment-flexible.
Also applies to: 239-240
editor/scaffolds/playground-canvas/playground.tsx (1)
385-385: Fix the spelling of "Presense" to "Presence".The component name has a spelling error.
Apply this diff to fix the spelling:
-function Presense() { +function Presence() {Also update the usage in line 435:
- <Presense /> + <Presence />editor/grida-canvas/editor.ts (2)
65-65: Consider documenting theonCreatecallback and its timing.The
onCreatecallback is invoked immediately in the constructor, which might be too early if the callback needs to access editor methods that depend on full initialization. Consider:
- Adding JSDoc documentation explaining the callback's purpose and timing
- Potentially deferring the callback execution to ensure the editor is fully initialized
constructor( initialState: editor.state.IEditorStateInit, instanceConfig: { pointer_move_throttle_ms: number; + /** + * Callback invoked after the editor instance is created. + * Note: Called synchronously within the constructor. + */ onCreate?: (editor: Editor) => void; } = { pointer_move_throttle_ms: 30 } ) {Also applies to: 70-72
1336-1341: Inconsistent use of thesyncparameter in transform calls.While
zoomandpanmethods explicitly passsync: truetotransform(), thescalemethod at line 1425 omits it, relying on the default value. For consistency:- this.transform(next); + this.transform(next, true);Also applies to: 1371-1371, 1376-1378, 1425-1425
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (30)
editor/app/(dev)/canvas/minimal/page.tsx(1 hunks)editor/app/(dev)/canvas/room/[room]/page.tsx(1 hunks)editor/app/(dev)/canvas/tools/io-figma/page.tsx(1 hunks)editor/app/(dev)/ui/page.tsx(3 hunks)editor/app/(tools)/(playground)/playground/image/_page.tsx(1 hunks)editor/components/multiplayer/avatar.tsx(1 hunks)editor/components/multiplayer/cursor.tsx(2 hunks)editor/grida-canvas-react-starter-kit/starterkit-hierarchy/index.tsx(2 hunks)editor/grida-canvas-react-starter-kit/starterkit-import/index.tsx(2 hunks)editor/grida-canvas-react/plugins/use-follow.ts(1 hunks)editor/grida-canvas-react/plugins/use-recorder.ts(2 hunks)editor/grida-canvas-react/provider.tsx(1 hunks)editor/grida-canvas-react/viewport/surface.tsx(8 hunks)editor/grida-canvas-react/viewport/ui/layer.tsx(2 hunks)editor/grida-canvas-react/viewport/ui/marquee.tsx(2 hunks)editor/grida-canvas/action.ts(1 hunks)editor/grida-canvas/editor.ts(12 hunks)editor/grida-canvas/index.ts(5 hunks)editor/grida-canvas/plugins/follow.ts(1 hunks)editor/grida-canvas/plugins/sync-y.ts(1 hunks)editor/grida-canvas/reducers/index.ts(2 hunks)editor/package.json(1 hunks)editor/scaffolds/editor/editor.tsx(1 hunks)editor/scaffolds/editor/multiplayer/multiplayer.tsx(2 hunks)editor/scaffolds/playground-canvas/library.tsx(1 hunks)editor/scaffolds/playground-canvas/modals/import-from-grida-file.tsx(0 hunks)editor/scaffolds/playground-canvas/playground.tsx(7 hunks)editor/scaffolds/sidebar/sidebar-mode-design.tsx(1 hunks)editor/scaffolds/sidebar/sidebar-node-hierarchy-list.tsx(0 hunks)editor/scaffolds/workbench/players.tsx(1 hunks)
💤 Files with no reviewable changes (2)
- editor/scaffolds/sidebar/sidebar-node-hierarchy-list.tsx
- editor/scaffolds/playground-canvas/modals/import-from-grida-file.tsx
🧰 Additional context used
🧬 Code Graph Analysis (7)
editor/grida-canvas-react/viewport/ui/marquee.tsx (1)
editor/grida-canvas/index.ts (1)
Marquee(247-250)
editor/grida-canvas-react-starter-kit/starterkit-import/index.tsx (5)
packages/grida-canvas-io-figma/lib.ts (1)
document(173-240)packages/grida-canvas-io/index.ts (1)
LoadedDocument(229-232)editor/components/picker/index.tsx (1)
useFilePicker(293-293)editor/components/ui/label.tsx (1)
Label(24-24)editor/components/ui/card.tsx (1)
Card(85-85)
editor/scaffolds/editor/editor.tsx (1)
editor/grida-canvas/editor.ts (1)
state(57-59)
editor/grida-canvas-react/provider.tsx (3)
editor/grida-canvas/index.ts (1)
IEditorMultiplayerCursorState(397-402)editor/grida-canvas-react/use-editor.tsx (2)
useCurrentEditor(25-33)useEditorState(35-47)editor/grida-canvas/editor.ts (1)
state(57-59)
editor/app/(dev)/ui/page.tsx (1)
editor/components/multiplayer/cursor.tsx (1)
PointerCursor(5-70)
editor/grida-canvas-react-starter-kit/starterkit-hierarchy/index.tsx (2)
editor/grida-canvas-react/index.ts (1)
useCurrentEditor(1-1)editor/components/ui/sidebar.tsx (4)
SidebarGroup(705-705)SidebarGroupLabel(708-708)SidebarGroupAction(706-706)SidebarGroupContent(707-707)
editor/grida-canvas/index.ts (2)
packages/grida-cmath/index.ts (1)
Transform(130-130)editor/grida-canvas/plugins/follow.ts (1)
cursor_id(17-19)
🪛 Biome (1.9.4)
editor/scaffolds/playground-canvas/library.tsx
[error] 203-203: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 205-205: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
🔇 Additional comments (45)
editor/app/(dev)/canvas/minimal/page.tsx (1)
10-12: LGTM! Clean import consolidation.The import path update correctly reflects the relocation of
NodeHierarchyGroupto the starter kit hierarchy module, maintaining consistency with the broader refactoring effort.editor/scaffolds/sidebar/sidebar-mode-design.tsx (1)
4-7: LGTM! Consistent import consolidation.The import path update properly reflects the component relocation from the local module to the centralized starter kit hierarchy module.
editor/app/(tools)/(playground)/playground/image/_page.tsx (1)
36-39: LGTM! Import path correctly updated.The import consolidation for
NodeHierarchyGroupis consistent with the component relocation to the starter kit hierarchy module.editor/grida-canvas-react-starter-kit/starterkit-hierarchy/index.tsx (3)
37-44: LGTM! Proper imports for sidebar UI components.The imports correctly include the necessary sidebar UI components (
SidebarGroup,SidebarGroupAction, etc.) and thePlusIconfor the create scene functionality.
581-601: LGTM! Well-structured ScenesGroup component.The
ScenesGroupcomponent properly wraps the existingScenesListwith sidebar UI components and includes:
- A labeled "Scenes" section with appropriate styling
- A plus button that correctly calls
editor.createScene()- Prevention of default context menu behavior
- Proper overflow handling for the scene list
The implementation maintains the existing functionality while adding consistent UI structure.
603-612: LGTM! Clean NodeHierarchyGroup implementation.The
NodeHierarchyGroupcomponent appropriately wraps theNodeHierarchyListwith sidebar UI components:
- Uses
flex-1class for proper layout- Includes a "Layers" label
- Prevents default context menu behavior
- Maintains the existing hierarchy functionality
The implementation is clean and consistent with the
ScenesGrouppattern.editor/grida-canvas-react/plugins/use-recorder.ts (2)
5-5: Usefast-deep-equalimport correctly
The import was switched from the ESM-specific path to the main package—this aligns with other modules and avoids bundle duplication.
15-15: Pass the correct comparator to the selector hook
Updating the equality function toequalensures deep comparisons work as intended inuseSyncExternalStoreWithSelector.editor/package.json (1)
217-221: Add Yjs collaboration dependencies
The newyjs,y-protocols,y-websocket, andy-webrtcentries are required for real‐time syncing. Ensure these versions align with your server-side setup and peer dependencies.editor/app/(dev)/canvas/tools/io-figma/page.tsx (1)
8-8: Switched Figma import dialog to the starter-kit module
ImportingImportFromFigmaDialogand its types from the unifiedstarterkit-importkeeps dialogs consolidated.editor/scaffolds/editor/multiplayer/multiplayer.tsx (2)
2-2: Remove unuseduseStateimport
Cleaning up this import reduces clutter; no further changes are needed here.
17-17: UpdatePointerCursorimport path
Switching to the centralized cursor component under@/components/multiplayer/cursorensures consistent styling and behavior.editor/scaffolds/editor/editor.tsx (1)
203-210: Performance improvement with selective subscription looks good!The change from subscribing to all state changes to specifically targeting
state.documentchanges is an excellent optimization. This will prevent unnecessary save operations when non-document state changes occur, reducing overhead and improving performance.The implementation correctly uses
subscribeWithSelectorwith the document state selector and maintains the existing debounced save behavior.editor/grida-canvas-react/provider.tsx (1)
816-819: Well-implemented multiplayer cursor state hook!The hook follows the established patterns in this file and correctly:
- Uses
useCurrentEditor()to get the editor instance- Subscribes to
state.cursorsusinguseEditorState- Has proper TypeScript return type annotation
This provides a clean interface for components to access multiplayer cursor state.
editor/grida-canvas-react/viewport/ui/marquee.tsx (2)
15-21: Good backward-compatible extension for multiplayer support!The optional
colorprop withhueandfillproperties is well-designed and maintains backward compatibility. This allows the marquee to be styled with different colors for multiplayer cursors while preserving existing functionality.
32-33: Conditional styling implementation is correct.The conditional application of custom colors using the ternary operator is appropriate and maintains the default CSS classes when no custom color is provided.
editor/components/multiplayer/cursor.tsx (2)
38-40: Performance optimizations look good.The removal of transition classes and addition of
willChange: "transform"are good performance optimizations for frequently updating cursor positions.
123-123: Message bubble styling update approved.The removal of transition properties from the message bubble is consistent with the cursor performance optimizations.
editor/grida-canvas-react/viewport/ui/layer.tsx (1)
12-13: LGTM! Clean prop extension for multiplayer cursor support.The addition of
borderColorandborderWidthprops is well-implemented with proper TypeScript typing and direct style application. This enables dynamic border styling for remote cursor visualization in the multiplayer features.Also applies to: 20-21, 42-43
editor/grida-canvas/reducers/index.ts (2)
128-132: LGTM! Improved code formatting.The multi-line formatting of the
prototypevariable assignment improves readability without changing the logic.
154-167:✅ Verification successful
Approve the sync optimization, but verify sync flag usage.
The conditional execution of
self_updateSurfaceHoverStatebased on thesyncflag is a good optimization for multiplayer scenarios, allowing transforms to skip cursor recalculation when not needed (e.g., for remote transforms).Verify that the
syncflag is properly set across all transform action dispatches:
🏁 Script executed:
#!/bin/bash # Description: Check all transform action dispatches to ensure sync flag is properly set # Expected: All transform dispatches should explicitly set the sync flag # Search for transform action dispatches rg -A 3 -B 3 'type.*:.*["\']transform["\']'Length of output: 116
🏁 Script executed:
#!/bin/bash # Description: Re-check all transform action dispatches to ensure the `sync` flag is explicitly set # Searching for occurrences of type: 'transform' with context rg -n -C3 "type\s*:\s*['\"]transform['\"]"Length of output: 890
Approve sync optimization – sync flag always included
I verified that all
transformactions are dispatched via thetransform(transform: cmath.Transform, sync: boolean = true)method ineditor/grida-canvas/editor.ts. That method always passes thesyncproperty (defaulting totrue) in the payload:• editor/grida-canvas/editor.ts (around line 1336):
transform(transform: cmath.Transform, sync: boolean = true) { this.dispatch({ type: "transform", transform, sync }); }No other direct
transformdispatches were found.editor/app/(dev)/ui/page.tsx (1)
12-12: LGTM! Good addition of multiplayer cursor demo component.The
__MultiplayerCursorcomponent provides a useful visual demonstration of the multiplayer cursor functionality. The implementation correctly:
- Imports the
PointerCursorcomponent- Sets up two cursors with distinct colors for testing
- Uses proper container styling with relative positioning
- Follows the established pattern of other demo components
This is valuable for development and testing of the multiplayer features.
Also applies to: 32-35, 115-135
editor/grida-canvas-react/plugins/use-follow.ts (1)
1-24: LGTM! Well-implemented React hook following best practices.The
useFollowPluginhook is excellently designed with:
- Proper external store integration using
useSyncExternalStoreWithSelector- Correct method binding for plugin subscription
- Deep equality optimization with
fast-deep-equal- Memoized return object to prevent unnecessary re-renders
- Clean API exposing follow state and control methods
The implementation correctly integrates the
EditorFollowPluginwith React's state management patterns and provides a clean interface for components to interact with the follow functionality.editor/app/(dev)/canvas/room/[room]/page.tsx (1)
9-21: LGTM! Clean Next.js app router implementation.The component correctly handles the async params promise and follows Next.js 13+ patterns. The full-screen layout and room_id prop passing are appropriate for a collaborative canvas editor.
editor/grida-canvas-react-starter-kit/starterkit-import/index.tsx (1)
20-46: Well-implemented import dialog with good UX patterns.The component properly handles file selection, loading states, and error scenarios. The disabled state management and toast notifications provide good user feedback.
editor/scaffolds/workbench/players.tsx (1)
4-4: Good refactoring to use shared component.Consolidating the PlayerAvatar component logic into a shared module reduces duplication and improves maintainability.
editor/components/multiplayer/avatar.tsx (1)
10-66: Well-designed multiplayer avatar component with clean API.The component provides excellent customization options and properly handles different user types. The conditional styling based on user type is well-implemented.
editor/grida-canvas/plugins/follow.ts (1)
1-105: Well-implemented follow plugin with proper state management.The plugin correctly handles cursor following with viewport fitting, subscription management, and state updates. The implementation follows good patterns with proper cleanup.
editor/grida-canvas-react/viewport/surface.tsx (3)
12-12: LGTM!The new imports are properly added to support multiplayer cursor functionality.
Also applies to: 42-42, 62-62
441-483: LGTM!The
RemoteCursorOverlaycomponent properly renders remote cursors with their selections, marquees, and appropriate color palettes.
900-908: LGTM!The optional
borderColorandborderWidthprops are properly added and forwarded to support visual differentiation of remote cursor selections.Also applies to: 940-941
editor/grida-canvas/index.ts (4)
18-64: LGTM!The
throttleutility function is well-implemented with proper TypeScript generics, context binding, and comprehensive documentation.
369-402: LGTM!The multiplayer cursor type definitions are comprehensive and well-structured, properly modeling cursor state with color palettes, position, selection, and other necessary fields.
634-634: LGTM!The editor state is properly extended with multiplayer cursor state and correctly initialized with an empty cursors array.
Also applies to: 738-738
1614-1618: LGTM!The
IFollowPluginActionsinterface properly defines the follow/unfollow actions for cursor following functionality.editor/scaffolds/playground-canvas/playground.tsx (2)
133-152: LGTM!The
useSyncMultiplayerCursorshook properly manages theEditorYSyncPluginlifecycle with correct cleanup on unmount and conditional activation based onroom_id.
269-269: LGTM!The export logic has been properly simplified by using
instance.archive()directly, which provides better abstraction.Also applies to: 614-617
editor/grida-canvas/editor.ts (8)
16-17: LGTM!The new imports are appropriately placed and follow the existing import conventions.
77-84: Well-implemented transaction tracking system.The transaction ID implementation provides a reliable way to track state mutations. The explicit documentation about not clearing on reset is helpful.
101-104: Consider using consistent arrow function syntax.While the change to use
reduceis correct, the syntax differs from otherreducecalls in the codebase. For consistency:- this.reduce((state) => { - state.debug = value; - return state; - }); + this.reduce((state) => ({ + ...state, + debug: value + }));However, if you're using Immer (which seems to be the case given the
produceimport), the current mutation style is actually correct and my suggestion above would be incorrect. Please disregard this suggestion if Immer is being used for state management.
116-116: Good addition of transaction ID return value.Returning the transaction ID from
resetallows callers to track when the reset operation occurred, which is useful for synchronization scenarios.Also applies to: 125-126
166-173: LGTM! Clean implementation for cursor synchronization.The
__sync_cursorsmethod properly updates the multiplayer cursor state using the reduce pattern, maintaining consistency with the state management approach.
181-182: Correct placement of transaction ID increments.The transaction ID is properly incremented after state mutations in both
reduceanddispatchmethods, ensuring accurate tracking of state changes.Also applies to: 188-189
197-218: Excellent implementation of selective subscription pattern.The
subscribeWithSelectormethod is well-designed with:
- Proper change detection using customizable equality checks
- Prevention of recursive dispatch loops by updating
previousbefore invoking the listener- Clear inline documentation explaining the critical ordering
This pattern significantly improves performance by allowing components to subscribe only to relevant state slices.
1526-1526: Good refactoring to use centralized throttle utility.Replacing the local throttle implementation with the imported
editor.throttleutility improves code maintainability and reduces duplication.
| /** | ||
| * if true, the transform will also re-calculate the cursor position. | ||
| */ | ||
| sync: boolean; | ||
| }; |
There was a problem hiding this comment.
Avoid breaking changes by making sync optional
Requiring a new sync boolean on every camera action will break existing dispatches. Change it to sync?: boolean and handle a default in the reducer.
Apply this diff:
-export type EditorCameraAction = {
+export type EditorCameraAction = {
type: "transform";
transform: cmath.Transform;
- /**
- * if true, the transform will also re-calculate the cursor position.
- */
- sync: boolean;
+ /**
+ * if true, the transform will also re-calculate the cursor position.
+ */
+ sync?: boolean;
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * if true, the transform will also re-calculate the cursor position. | |
| */ | |
| sync: boolean; | |
| }; | |
| export type EditorCameraAction = { | |
| type: "transform"; | |
| transform: cmath.Transform; | |
| /** | |
| * if true, the transform will also re-calculate the cursor position. | |
| */ | |
| sync?: boolean; | |
| }; |
🤖 Prompt for AI Agents
In editor/grida-canvas/action.ts around lines 386 to 390, the `sync` property is
currently required, which can break existing code that dispatches camera actions
without it. Change the `sync` property to be optional by adding a question mark
(`sync?: boolean`). Then, update the reducer handling this action to provide a
default value for `sync` when it is undefined, ensuring backward compatibility.
| width="12" | ||
| height="16" | ||
| viewBox="0 0 18 24" |
There was a problem hiding this comment.
Fix viewBox to match the new dimensions.
The SVG dimensions were reduced to 12x16, but the viewBox still shows "0 0 18 24". This mismatch will cause the cursor to be scaled incorrectly.
Apply this fix:
- width="12"
- height="16"
- viewBox="0 0 18 24"
+ width="12"
+ height="16"
+ viewBox="0 0 12 16"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| width="12" | |
| height="16" | |
| viewBox="0 0 18 24" | |
| width="12" | |
| height="16" | |
| viewBox="0 0 12 16" |
🤖 Prompt for AI Agents
In editor/components/multiplayer/cursor.tsx around lines 34 to 36, the SVG width
and height were changed to 12 and 16 respectively, but the viewBox remains "0 0
18 24", causing incorrect scaling. Update the viewBox attribute to "0 0 12 16"
to match the new SVG dimensions and ensure proper rendering.
| private throttle_ms: number = 5; | ||
| private _tid: number = 0; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider increasing the throttle interval.
A 5ms throttle interval is very aggressive and could cause performance issues with rapid state updates. Consider increasing it to at least 16ms (60fps) or higher.
- private throttle_ms: number = 5;
+ private throttle_ms: number = 50; // 20 updates per second📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private throttle_ms: number = 5; | |
| private _tid: number = 0; | |
| private throttle_ms: number = 50; // 20 updates per second | |
| private _tid: number = 0; |
🤖 Prompt for AI Agents
In editor/grida-canvas/plugins/sync-y.ts around lines 28 to 29, the throttle
interval is set to 5ms, which is too aggressive and may cause performance
issues. Increase the throttle_ms value to at least 16ms to align with a 60fps
update rate or higher to reduce the frequency of state updates and improve
performance.
| this.provider = new WebsocketProvider( | ||
| "ws://localhost:1234", | ||
| room_id, | ||
| this.doc | ||
| ); |
There was a problem hiding this comment.
Make WebSocket URL configurable.
The WebSocket URL is hardcoded to ws://localhost:1234, which won't work in production environments. This should be passed as a configuration parameter.
Apply this diff to make the URL configurable:
constructor(
private readonly _editor: Editor,
private readonly room_id: string,
private readonly cursor: {
palette: editor.state.MultiplayerCursorColorPalette;
- }
+ },
+ private readonly websocket_url: string = "ws://localhost:1234"
) {
this.doc = new Y.Doc();
this.provider = new WebsocketProvider(
- "ws://localhost:1234",
+ websocket_url,
room_id,
this.doc
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| this.provider = new WebsocketProvider( | |
| "ws://localhost:1234", | |
| room_id, | |
| this.doc | |
| ); | |
| constructor( | |
| private readonly _editor: Editor, | |
| private readonly room_id: string, | |
| private readonly cursor: { | |
| palette: editor.state.MultiplayerCursorColorPalette; | |
| }, | |
| private readonly websocket_url: string = "ws://localhost:1234" | |
| ) { | |
| this.doc = new Y.Doc(); | |
| this.provider = new WebsocketProvider( | |
| websocket_url, | |
| room_id, | |
| this.doc | |
| ); | |
| // ...rest of constructor implementation... | |
| } |
🤖 Prompt for AI Agents
In editor/grida-canvas/plugins/sync-y.ts around lines 39 to 43, the WebSocket
URL is hardcoded to "ws://localhost:1234", which is not suitable for production.
Modify the code to accept the WebSocket URL as a configuration parameter, such
as passing it into the constructor or method that creates the WebsocketProvider
instance, and use that parameter instead of the hardcoded string.
| type ReflectUIIconData = { | ||
| host: "material" | "ant-design" | "radix-ui" | "unicons" | (string | {}); | ||
| family: string; | ||
| variant: "default" | (string | {}); | ||
| default_size: number; | ||
| }; |
There was a problem hiding this comment.
Replace {} with proper type definitions.
Using {} as a type means "any non-nullable value" which defeats TypeScript's type safety.
type ReflectUIIconData = {
- host: "material" | "ant-design" | "radix-ui" | "unicons" | (string | {});
+ host: "material" | "ant-design" | "radix-ui" | "unicons" | string;
family: string;
- variant: "default" | (string | {});
+ variant: "default" | string;
default_size: number;
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| type ReflectUIIconData = { | |
| host: "material" | "ant-design" | "radix-ui" | "unicons" | (string | {}); | |
| family: string; | |
| variant: "default" | (string | {}); | |
| default_size: number; | |
| }; | |
| type ReflectUIIconData = { | |
| host: "material" | "ant-design" | "radix-ui" | "unicons" | string; | |
| family: string; | |
| variant: "default" | string; | |
| default_size: number; | |
| }; |
🧰 Tools
🪛 Biome (1.9.4)
[error] 203-203: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 205-205: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
🤖 Prompt for AI Agents
In editor/scaffolds/playground-canvas/library.tsx around lines 202 to 207, the
type definitions use {} which is too broad and undermines type safety. Replace
{} with more specific type definitions that accurately represent the expected
structure or values for those fields, such as defining proper interfaces or
union types instead of using {}. This will improve type safety and clarity in
the ReflectUIIconData type.
| function useReflectIconsData() { | ||
| const json = "https://reflect-icons.s3.us-west-1.amazonaws.com/all.json"; | ||
| const [icons, setIcons] = useState<{ | ||
| [key: string]: ReflectUIIconData; | ||
| }>({}); | ||
| useEffect(() => { | ||
| fetch(json).then((res) => { | ||
| res.json().then((data) => { | ||
| setIcons(data); | ||
| }); | ||
| }); | ||
| }, []); | ||
|
|
||
| return icons; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and cleanup to data fetching.
The hook doesn't handle fetch errors or cleanup on unmount, which could cause memory leaks or unhandled promise rejections.
function useReflectIconsData() {
const json = "https://reflect-icons.s3.us-west-1.amazonaws.com/all.json";
const [icons, setIcons] = useState<{
[key: string]: ReflectUIIconData;
}>({});
useEffect(() => {
- fetch(json).then((res) => {
- res.json().then((data) => {
- setIcons(data);
- });
- });
+ const controller = new AbortController();
+
+ fetch(json, { signal: controller.signal })
+ .then((res) => {
+ if (!res.ok) throw new Error(`HTTP error! status: ${res.status}`);
+ return res.json();
+ })
+ .then((data) => {
+ setIcons(data);
+ })
+ .catch((error) => {
+ if (error.name !== 'AbortError') {
+ console.error('Failed to fetch icons:', error);
+ toast.error('Failed to load icons');
+ }
+ });
+
+ return () => controller.abort();
}, []);
return icons;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function useReflectIconsData() { | |
| const json = "https://reflect-icons.s3.us-west-1.amazonaws.com/all.json"; | |
| const [icons, setIcons] = useState<{ | |
| [key: string]: ReflectUIIconData; | |
| }>({}); | |
| useEffect(() => { | |
| fetch(json).then((res) => { | |
| res.json().then((data) => { | |
| setIcons(data); | |
| }); | |
| }); | |
| }, []); | |
| return icons; | |
| } | |
| function useReflectIconsData() { | |
| const json = "https://reflect-icons.s3.us-west-1.amazonaws.com/all.json"; | |
| const [icons, setIcons] = useState<{ | |
| [key: string]: ReflectUIIconData; | |
| }>({}); | |
| useEffect(() => { | |
| const controller = new AbortController(); | |
| fetch(json, { signal: controller.signal }) | |
| .then((res) => { | |
| if (!res.ok) throw new Error(`HTTP error! status: ${res.status}`); | |
| return res.json(); | |
| }) | |
| .then((data) => { | |
| setIcons(data); | |
| }) | |
| .catch((error) => { | |
| if (error.name !== 'AbortError') { | |
| console.error('Failed to fetch icons:', error); | |
| toast.error('Failed to load icons'); | |
| } | |
| }); | |
| return () => { | |
| controller.abort(); | |
| }; | |
| }, []); | |
| return icons; | |
| } |
🤖 Prompt for AI Agents
In editor/scaffolds/playground-canvas/library.tsx around lines 209 to 223, the
useReflectIconsData hook lacks error handling for the fetch call and does not
clean up on component unmount, risking memory leaks and unhandled promise
rejections. Modify the hook to catch and handle fetch or JSON parsing errors
gracefully, and implement a cleanup mechanism using a flag or AbortController to
prevent state updates if the component unmounts before the fetch completes.
| function FollowingFrameOverlay() { | ||
| const instance = useCurrentEditor(); | ||
| const { isFollowing, cursor: cursorId } = useFollowPlugin( | ||
| instance.__pligin_follow |
There was a problem hiding this comment.
Fix the typo in property name.
The property name __pligin_follow contains a typo and should be __plugin_follow.
Apply this diff to fix the typo:
- const { isFollowing, cursor: cursorId } = useFollowPlugin(
- instance.__pligin_follow
- );
+ const { isFollowing, cursor: cursorId } = useFollowPlugin(
+ instance.__plugin_follow
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| instance.__pligin_follow | |
| const { isFollowing, cursor: cursorId } = useFollowPlugin( | |
| instance.__plugin_follow | |
| ); |
🤖 Prompt for AI Agents
In editor/grida-canvas-react/viewport/surface.tsx at line 403, there is a typo
in the property name `__pligin_follow`. Correct the property name to
`__plugin_follow` to fix the typo.
| useEffect(() => { | ||
| if (!src) return; | ||
| fetch(src).then((res) => { | ||
| res.json().then((file) => { | ||
| instance.reset( | ||
| editor.state.init({ | ||
| editable: true, | ||
| document: file.document, | ||
| }), | ||
| src | ||
| ); | ||
| }); | ||
| }); | ||
| }, [src]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for document loading.
The fetch operation should handle potential errors when loading the document from the src URL.
Apply this diff to add error handling:
useEffect(() => {
if (!src) return;
- fetch(src).then((res) => {
- res.json().then((file) => {
- instance.reset(
- editor.state.init({
- editable: true,
- document: file.document,
- }),
- src
- );
- });
- });
+ fetch(src)
+ .then((res) => {
+ if (!res.ok) throw new Error(`Failed to load document: ${res.statusText}`);
+ return res.json();
+ })
+ .then((file) => {
+ instance.reset(
+ editor.state.init({
+ editable: true,
+ document: file.document,
+ }),
+ src
+ );
+ })
+ .catch((error) => {
+ console.error('Failed to load document:', error);
+ toast.error('Failed to load document');
+ });
}, [src]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| useEffect(() => { | |
| if (!src) return; | |
| fetch(src).then((res) => { | |
| res.json().then((file) => { | |
| instance.reset( | |
| editor.state.init({ | |
| editable: true, | |
| document: file.document, | |
| }), | |
| src | |
| ); | |
| }); | |
| }); | |
| }, [src]); | |
| useEffect(() => { | |
| if (!src) return; | |
| fetch(src) | |
| .then((res) => { | |
| if (!res.ok) throw new Error(`Failed to load document: ${res.statusText}`); | |
| return res.json(); | |
| }) | |
| .then((file) => { | |
| instance.reset( | |
| editor.state.init({ | |
| editable: true, | |
| document: file.document, | |
| }), | |
| src | |
| ); | |
| }) | |
| .catch((error) => { | |
| console.error('Failed to load document:', error); | |
| toast.error('Failed to load document'); | |
| }); | |
| }, [src]); |
🤖 Prompt for AI Agents
In editor/scaffolds/playground-canvas/playground.tsx around lines 189 to 202,
the fetch call to load the document from src lacks error handling. Wrap the
fetch and subsequent JSON parsing in try-catch blocks or use .catch() on the
promises to handle any network or parsing errors gracefully. Log or handle these
errors appropriately to avoid unhandled promise rejections and improve
robustness.
editor/grida-canvas/editor.ts
Outdated
| public archive(): Blob { | ||
| const documentData = { | ||
| version: "0.0.1-beta.1+20250303", | ||
| document: this.getSnapshot().document, | ||
| } satisfies io.JSONDocumentFileModel; | ||
|
|
||
| const blob = new Blob([io.archive.pack(documentData)], { | ||
| type: "application/zip", | ||
| }); | ||
|
|
||
| return blob; | ||
| } |
There was a problem hiding this comment.
Consider externalizing the version string.
The hardcoded version string "0.0.1-beta.1+20250303" will become outdated and doesn't reflect the actual version or build date. Consider:
- Reading the version from package.json
- Using a build-time constant
- Accepting version as a parameter
- public archive(): Blob {
+ public archive(version: string = editor.config.ARCHIVE_VERSION): Blob {
const documentData = {
- version: "0.0.1-beta.1+20250303",
+ version,
document: this.getSnapshot().document,
} satisfies io.JSONDocumentFileModel;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public archive(): Blob { | |
| const documentData = { | |
| version: "0.0.1-beta.1+20250303", | |
| document: this.getSnapshot().document, | |
| } satisfies io.JSONDocumentFileModel; | |
| const blob = new Blob([io.archive.pack(documentData)], { | |
| type: "application/zip", | |
| }); | |
| return blob; | |
| } | |
| public archive(version: string = editor.config.ARCHIVE_VERSION): Blob { | |
| const documentData = { | |
| version, | |
| document: this.getSnapshot().document, | |
| } satisfies io.JSONDocumentFileModel; | |
| const blob = new Blob([io.archive.pack(documentData)], { | |
| type: "application/zip", | |
| }); | |
| return blob; | |
| } |
🤖 Prompt for AI Agents
In editor/grida-canvas/editor.ts around lines 128 to 139, the version string is
hardcoded, which can become outdated. To fix this, externalize the version by
either importing it from package.json, defining it as a build-time constant, or
modifying the archive method to accept the version as a parameter. Update the
code to use this externalized version value instead of the hardcoded string.
editor/grida-canvas/editor.ts
Outdated
| readonly __pligin_follow: EditorFollowPlugin = new EditorFollowPlugin(this); | ||
| // #region IFollowPluginActions implementation | ||
| follow(cursor_id: string): void { | ||
| this.__pligin_follow.follow(cursor_id); | ||
| } | ||
|
|
||
| unfollow(): void { | ||
| this.__pligin_follow.unfollow(); | ||
| } | ||
| // #endregion IFollowPluginActions implementation |
There was a problem hiding this comment.
Fix typo in property name.
There's a typo in the property name __pligin_follow - it should be __plugin_follow.
- readonly __pligin_follow: EditorFollowPlugin = new EditorFollowPlugin(this);
+ readonly __plugin_follow: EditorFollowPlugin = new EditorFollowPlugin(this);
// #region IFollowPluginActions implementation
follow(cursor_id: string): void {
- this.__pligin_follow.follow(cursor_id);
+ this.__plugin_follow.follow(cursor_id);
}
unfollow(): void {
- this.__pligin_follow.unfollow();
+ this.__plugin_follow.unfollow();
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| readonly __pligin_follow: EditorFollowPlugin = new EditorFollowPlugin(this); | |
| // #region IFollowPluginActions implementation | |
| follow(cursor_id: string): void { | |
| this.__pligin_follow.follow(cursor_id); | |
| } | |
| unfollow(): void { | |
| this.__pligin_follow.unfollow(); | |
| } | |
| // #endregion IFollowPluginActions implementation | |
| readonly __plugin_follow: EditorFollowPlugin = new EditorFollowPlugin(this); | |
| // #region IFollowPluginActions implementation | |
| follow(cursor_id: string): void { | |
| this.__plugin_follow.follow(cursor_id); | |
| } | |
| unfollow(): void { | |
| this.__plugin_follow.unfollow(); | |
| } | |
| // #endregion IFollowPluginActions implementation |
🤖 Prompt for AI Agents
In editor/grida-canvas/editor.ts around lines 1741 to 1750, the property name
__pligin_follow contains a typo and should be corrected to __plugin_follow.
Rename all occurrences of __pligin_follow to __plugin_follow to fix the typo
consistently in the property declaration and its usage within the follow and
unfollow methods.
Experimental CRDT canvas is only available in localhost.
npx y-websoketto boot up yjs ws serverPt.1 Awareness
day-241-grida-canvas-multiplayer-pt1-awareness.mp4
Pt.2 Follow
day-242-grida-canvas-multiplayer-pt2-follow.mp4
Summary by CodeRabbit
New Features
.gridaand.jsonfile formats.Improvements
Bug Fixes
Chores
Refactor