fix(ui): websocket setup for editor, project import/export and workflow import #963
Conversation
WalkthroughThe changes modify several hooks and components to integrate asynchronous token retrieval using the Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Function Caller
participant Hook as Export/Import Hook
participant Auth as useAuth
participant WS as WebsocketProvider
Caller->>Hook: Trigger project export/import
Hook->>Auth: getAccessToken() (async call)
Auth-->>Hook: Return access token
Hook->>WS: Initialize with token parameter
WS-->>Hook: WebSocket connection established
sequenceDiagram
participant Editor as EditorComponent
participant Yjs as useYjsSetup
participant Auth as useAuth
participant WS as WebsocketProvider
Editor->>Yjs: Call useYjsSetup with isProtected: true
Yjs->>Auth: getAccessToken() (if isProtected)
Auth-->>Yjs: Return access token
Yjs->>WS: Initialize with token in params (if token provided)
WS-->>Yjs: WebSocket connection established
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
Documentation and Community
|
✅ Deploy Preview for reearth-flow ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (5)
ui/src/routes/workspaces.$workspaceId_.projects_.$projectId.lazy.tsx (1)
35-46: Consider removing unused token state.The component still maintains state for
accessTokenand retrieves it in the useEffect, but this token is no longer directly passed touseYjsSetup. Since the token retrieval is now handled inside the hook itself, this state management appears to be redundant.- const [accessToken, setAccessToken] = useState<string | undefined>(undefined); - - const { getAccessToken } = useAuth(); - - useEffect(() => { - if (!accessToken) { - (async () => { - const token = await getAccessToken(); - setAccessToken(token); - })(); - } - }, [accessToken, getAccessToken]);ui/src/hooks/useProjectImport.ts (1)
82-90: Add WebSocket error handling.The current implementation doesn't handle potential connection failures with the WebSocket provider. Adding event listeners for errors would improve reliability.
const yWebSocketProvider = new WebsocketProvider( websocket, `${project.id}:${DEFAULT_ENTRY_GRAPH_ID}`, yDoc, { params: { token } }, ); + yWebSocketProvider.on("connection-error", (error) => { + console.error("WebSocket connection error:", error); + setIsProjectImporting(false); + }); + + yWebSocketProvider.on("connection-close", () => { + console.warn("WebSocket connection closed unexpectedly"); + setIsProjectImporting(false); + }); yWebSocketProvider.once("sync", () => { // Existing code });ui/src/lib/yjs/useYjsSetup.ts (2)
41-70: Add WebSocket connection error handling.The current implementation only handles the successful
syncevent but doesn't account for potential connection errors. Adding error event handlers would improve reliability.yWebSocketProvider = new WebsocketProvider( websocket, `${projectId}:${workflowId}`, yDoc, { params, }, ); + yWebSocketProvider.on("connection-error", (error) => { + console.error("WebSocket connection error:", error); + // Consider implementing fallback behavior or retry mechanism + }); + + yWebSocketProvider.on("connection-close", () => { + console.warn("WebSocket connection closed unexpectedly"); + setIsSynced(false); + }); yWebSocketProvider.once("sync", () => { // Existing code });
33-85: Consider adding a connection status indicator.The hook manages the
isSyncedstate but doesn't track connection errors or retries. Adding a more comprehensive connection status would provide better feedback to users.Consider enhancing the hook to include a more detailed connection status:
// Add a more detailed connection status type + type ConnectionStatus = 'disconnected' | 'connecting' | 'connected' | 'error'; + const [connectionStatus, setConnectionStatus] = useState<ConnectionStatus>('disconnected'); - const [isSynced, setIsSynced] = useState(false); // Update status in the effect + setConnectionStatus('connecting'); // Update status in the event handlers + yWebSocketProvider.on('connection-error', () => { + setConnectionStatus('error'); + }); + yWebSocketProvider.on('connection-close', () => { + setConnectionStatus('disconnected'); + }); yWebSocketProvider.once("sync", () => { // Existing code + setConnectionStatus('connected'); - setIsSynced(true); }); // Update the return value return { state, - isSynced, + isSynced: connectionStatus === 'connected', + connectionStatus, undoManager, undoTrackerActionWrapper, };This would maintain backward compatibility while providing more detailed connection status information.
ui/src/hooks/useWorkflowImport.ts (1)
97-116: Implementation of secure WebSocket connection with authentication.The code now properly implements an authenticated WebSocket connection by:
- Using an async IIFE to await the token retrieval
- Passing the token in the params object to the WebsocketProvider
- Setting up the sync event handler within the async context
However, there's no error handling for token retrieval or WebSocket connection failures.
Consider adding error handling to provide better feedback:
(async () => { - const token = await getAccessToken(); + try { + const token = await getAccessToken(); + + const yWebSocketProvider = new WebsocketProvider( + websocket, + `${project.id}:${DEFAULT_ENTRY_GRAPH_ID}`, + yDoc, + { params: { token } }, + ); + + yWebSocketProvider.once("sync", () => { + const yWorkflows = yDoc.getArray<YWorkflow>("workflows"); + if (!yWorkflows.length) { + console.warn("Imported project has no workflows"); + } + + setIsWorkflowImporting(false); + }); + + yWebSocketProvider.on("connection-error", (err) => { + console.error("WebSocket connection error:", err); + setIsWorkflowImporting(false); + }); + } catch (error) { + console.error("Failed to authenticate WebSocket connection:", error); + setIsWorkflowImporting(false); + } })();
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
ui/src/hooks/useProjectExport.ts(2 hunks)ui/src/hooks/useProjectImport.ts(2 hunks)ui/src/hooks/useWorkflowImport.ts(4 hunks)ui/src/lib/yjs/useYjsSetup.ts(3 hunks)ui/src/routes/workspaces.$workspaceId_.projects_.$projectId.lazy.tsx(1 hunks)
🔇 Additional comments (8)
ui/src/routes/workspaces.$workspaceId_.projects_.$projectId.lazy.tsx (1)
72-72: LGTM! Properly updated parameter to match new API.The change from passing
accessTokento usingisProtected: truecorrectly adapts to the updated API in theuseYjsSetuphook.ui/src/hooks/useProjectExport.ts (1)
9-9: LGTM! Proper auth integration for WebSocket connections.The addition of the
useAuthhook and includinggetAccessTokenin the dependencies array is correct.Also applies to: 16-16, 83-83
ui/src/hooks/useProjectImport.ts (1)
8-8: LGTM! Auth hook properly integrated.The addition of the
useAuthhook and inclusion ofgetAccessTokenin the dependencies array is correct.Also applies to: 16-17, 97-97
ui/src/lib/yjs/useYjsSetup.ts (2)
13-21: LGTM! Good approach to conditional auth.The parameter change from
accessTokentoisProtectedis a good design choice. It makes the API more intention-revealing and hides the authentication implementation details from consumers.
8-8: LGTM! Proper auth hook integration.The auth hook is correctly imported and integrated, and
getAccessTokenis properly added to the effect dependencies.Also applies to: 22-22, 85-85
ui/src/hooks/useWorkflowImport.ts (3)
7-7: Good addition of auth dependency.Adding the
useAuthhook is the correct approach for obtaining authentication tokens needed for secure WebSocket connections.
23-24: Proper extraction of the authentication token retrieval function.The hook correctly extracts the
getAccessTokenfunction fromuseAuth, which will be used later for WebSocket authentication.
127-127: Correct dependency array update.Adding
getAccessTokento the dependency array is necessary since it's now used within thehandleWorkflowFileUploadfunction. This follows React's best practices for hooks and prevents potential stale closure issues.
| let yWebSocketProvider: WebsocketProvider | null = null; | ||
|
|
||
| if (websocket && project.id) { |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Ensure WebSocket provider variable safety.
The yWebSocketProvider variable is declared outside the async IIFE but initialized inside it, creating a potential race condition where the variable might be accessed before it's initialized.
Verify how the variable is used outside the IIFE:
Also applies to: 29-40
🏁 Script executed:
#!/bin/bash
# Search for references to yWebSocketProvider outside the IIFE
rg -n "yWebSocketProvider" "ui/src/hooks/useProjectExport.ts" | grep -v "yWebSocketProvider\s*="Length of output: 259
Attention: yWebSocketProvider Initialization Order
There's a potential race condition with the yWebSocketProvider variable: it’s declared at the top level (line 26) but only assigned inside the async IIFE (lines 29–40). This creates a risk where usage in line 42—calling yWebSocketProvider.once("sync", …)—may occur before it’s properly initialized. Although the usage in line 79 uses optional chaining (yWebSocketProvider?.destroy()), the unguarded call at line 42 could lead to runtime errors if yWebSocketProvider is still null.
Please ensure one of the following:
- Defer any usage of
yWebSocketProvideruntil after it’s guaranteed to be set. - Add an explicit null check before calling methods like
.onceto safeguard against race conditions.
| (async () => { | ||
| const token = await getAccessToken(); | ||
| yWebSocketProvider = new WebsocketProvider( | ||
| websocket, | ||
| `${project.id}:${DEFAULT_ENTRY_GRAPH_ID}`, | ||
| yDoc, | ||
| { | ||
| params: { | ||
| token, | ||
| }, | ||
| }, | ||
| ); | ||
|
|
||
| yWebSocketProvider.once("sync", async () => { | ||
| if (yWorkflows.length === 0) { | ||
| yDoc.transact(() => { | ||
| const yWorkflow = yWorkflowConstructor( | ||
| DEFAULT_ENTRY_GRAPH_ID, | ||
| "Main Workflow", | ||
| ); | ||
| yWorkflows.insert(0, [yWorkflow]); | ||
| }); | ||
| } | ||
| yWebSocketProvider.once("sync", async () => { | ||
| if (yWorkflows.length === 0) { | ||
| yDoc.transact(() => { | ||
| const yWorkflow = yWorkflowConstructor( | ||
| DEFAULT_ENTRY_GRAPH_ID, | ||
| "Main Workflow", | ||
| ); | ||
| yWorkflows.insert(0, [yWorkflow]); | ||
| }); | ||
| } | ||
|
|
||
| const zip = new JSZip(); | ||
| const zip = new JSZip(); | ||
|
|
||
| const yDocBinary = Y.encodeStateAsUpdate(yDoc); | ||
| zip.file("ydoc.bin", yDocBinary); | ||
| const yDocBinary = Y.encodeStateAsUpdate(yDoc); | ||
| zip.file("ydoc.bin", yDocBinary); | ||
|
|
||
| const projectData = { | ||
| id: generateUUID(), | ||
| name: project.name, | ||
| description: project.description, | ||
| }; | ||
| zip.file("projectMeta.json", JSON.stringify(projectData, null, 2)); | ||
| const projectData = { | ||
| id: generateUUID(), | ||
| name: project.name, | ||
| description: project.description, | ||
| }; | ||
| zip.file("projectMeta.json", JSON.stringify(projectData, null, 2)); | ||
|
|
||
| const zipBlob = await zip.generateAsync({ type: "blob" }); | ||
| const date = new Date(); | ||
| const timeStamp = [ | ||
| date.getFullYear(), | ||
| String(date.getMonth() + 1).padStart(2, "0"), | ||
| String(date.getDate()).padStart(2, "0"), | ||
| String(date.getHours()).padStart(2, "0"), | ||
| String(date.getMinutes()).padStart(2, "0"), | ||
| String(date.getSeconds()).padStart(2, "0"), | ||
| ].join(""); | ||
| const zipName = `${project.name}_${timeStamp}.flow.zip`; | ||
| saveAs(zipBlob, zipName); | ||
| setIsExporting(false); | ||
| const zipBlob = await zip.generateAsync({ type: "blob" }); | ||
| const date = new Date(); | ||
| const timeStamp = [ | ||
| date.getFullYear(), | ||
| String(date.getMonth() + 1).padStart(2, "0"), | ||
| String(date.getDate()).padStart(2, "0"), | ||
| String(date.getHours()).padStart(2, "0"), | ||
| String(date.getMinutes()).padStart(2, "0"), | ||
| String(date.getSeconds()).padStart(2, "0"), | ||
| ].join(""); | ||
| const zipName = `${project.name}_${timeStamp}.flow.zip`; | ||
| saveAs(zipBlob, zipName); | ||
| setIsExporting(false); | ||
|
|
||
| yWebSocketProvider?.destroy(); | ||
| }); | ||
| yWebSocketProvider?.destroy(); | ||
| }); | ||
| })(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for token retrieval.
The current implementation doesn't handle potential failures when retrieving the access token. If getAccessToken() fails, the WebSocket connection won't be established and the export process will hang.
(async () => {
- const token = await getAccessToken();
+ try {
+ const token = await getAccessToken();
yWebSocketProvider = new WebsocketProvider(
websocket,
`${project.id}:${DEFAULT_ENTRY_GRAPH_ID}`,
yDoc,
{
params: {
token,
},
},
);
yWebSocketProvider.once("sync", async () => {
// ... existing code
});
+ } catch (error) {
+ console.error("Failed to get access token for project export:", error);
+ setIsExporting(false);
+ }
})();📝 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.
| (async () => { | |
| const token = await getAccessToken(); | |
| yWebSocketProvider = new WebsocketProvider( | |
| websocket, | |
| `${project.id}:${DEFAULT_ENTRY_GRAPH_ID}`, | |
| yDoc, | |
| { | |
| params: { | |
| token, | |
| }, | |
| }, | |
| ); | |
| yWebSocketProvider.once("sync", async () => { | |
| if (yWorkflows.length === 0) { | |
| yDoc.transact(() => { | |
| const yWorkflow = yWorkflowConstructor( | |
| DEFAULT_ENTRY_GRAPH_ID, | |
| "Main Workflow", | |
| ); | |
| yWorkflows.insert(0, [yWorkflow]); | |
| }); | |
| } | |
| yWebSocketProvider.once("sync", async () => { | |
| if (yWorkflows.length === 0) { | |
| yDoc.transact(() => { | |
| const yWorkflow = yWorkflowConstructor( | |
| DEFAULT_ENTRY_GRAPH_ID, | |
| "Main Workflow", | |
| ); | |
| yWorkflows.insert(0, [yWorkflow]); | |
| }); | |
| } | |
| const zip = new JSZip(); | |
| const zip = new JSZip(); | |
| const yDocBinary = Y.encodeStateAsUpdate(yDoc); | |
| zip.file("ydoc.bin", yDocBinary); | |
| const yDocBinary = Y.encodeStateAsUpdate(yDoc); | |
| zip.file("ydoc.bin", yDocBinary); | |
| const projectData = { | |
| id: generateUUID(), | |
| name: project.name, | |
| description: project.description, | |
| }; | |
| zip.file("projectMeta.json", JSON.stringify(projectData, null, 2)); | |
| const projectData = { | |
| id: generateUUID(), | |
| name: project.name, | |
| description: project.description, | |
| }; | |
| zip.file("projectMeta.json", JSON.stringify(projectData, null, 2)); | |
| const zipBlob = await zip.generateAsync({ type: "blob" }); | |
| const date = new Date(); | |
| const timeStamp = [ | |
| date.getFullYear(), | |
| String(date.getMonth() + 1).padStart(2, "0"), | |
| String(date.getDate()).padStart(2, "0"), | |
| String(date.getHours()).padStart(2, "0"), | |
| String(date.getMinutes()).padStart(2, "0"), | |
| String(date.getSeconds()).padStart(2, "0"), | |
| ].join(""); | |
| const zipName = `${project.name}_${timeStamp}.flow.zip`; | |
| saveAs(zipBlob, zipName); | |
| setIsExporting(false); | |
| const zipBlob = await zip.generateAsync({ type: "blob" }); | |
| const date = new Date(); | |
| const timeStamp = [ | |
| date.getFullYear(), | |
| String(date.getMonth() + 1).padStart(2, "0"), | |
| String(date.getDate()).padStart(2, "0"), | |
| String(date.getHours()).padStart(2, "0"), | |
| String(date.getMinutes()).padStart(2, "0"), | |
| String(date.getSeconds()).padStart(2, "0"), | |
| ].join(""); | |
| const zipName = `${project.name}_${timeStamp}.flow.zip`; | |
| saveAs(zipBlob, zipName); | |
| setIsExporting(false); | |
| yWebSocketProvider?.destroy(); | |
| }); | |
| yWebSocketProvider?.destroy(); | |
| }); | |
| })(); | |
| (async () => { | |
| + try { | |
| + const token = await getAccessToken(); | |
| yWebSocketProvider = new WebsocketProvider( | |
| websocket, | |
| `${project.id}:${DEFAULT_ENTRY_GRAPH_ID}`, | |
| yDoc, | |
| { | |
| params: { | |
| token, | |
| }, | |
| }, | |
| ); | |
| yWebSocketProvider.once("sync", async () => { | |
| if (yWorkflows.length === 0) { | |
| yDoc.transact(() => { | |
| const yWorkflow = yWorkflowConstructor( | |
| DEFAULT_ENTRY_GRAPH_ID, | |
| "Main Workflow", | |
| ); | |
| yWorkflows.insert(0, [yWorkflow]); | |
| }); | |
| } | |
| const zip = new JSZip(); | |
| const yDocBinary = Y.encodeStateAsUpdate(yDoc); | |
| zip.file("ydoc.bin", yDocBinary); | |
| const projectData = { | |
| id: generateUUID(), | |
| name: project.name, | |
| description: project.description, | |
| }; | |
| zip.file("projectMeta.json", JSON.stringify(projectData, null, 2)); | |
| const zipBlob = await zip.generateAsync({ type: "blob" }); | |
| const date = new Date(); | |
| const timeStamp = [ | |
| date.getFullYear(), | |
| String(date.getMonth() + 1).padStart(2, "0"), | |
| String(date.getDate()).padStart(2, "0"), | |
| String(date.getHours()).padStart(2, "0"), | |
| String(date.getMinutes()).padStart(2, "0"), | |
| String(date.getSeconds()).padStart(2, "0"), | |
| ].join(""); | |
| const zipName = `${project.name}_${timeStamp}.flow.zip`; | |
| saveAs(zipBlob, zipName); | |
| setIsExporting(false); | |
| yWebSocketProvider?.destroy(); | |
| }); | |
| + } catch (error) { | |
| + console.error("Failed to get access token for project export:", error); | |
| + setIsExporting(false); | |
| + } | |
| })(); |
| (async () => { | ||
| const token = await getAccessToken(); | ||
|
|
||
| const yWebSocketProvider = new WebsocketProvider( | ||
| websocket, | ||
| `${project.id}:${DEFAULT_ENTRY_GRAPH_ID}`, | ||
| yDoc, | ||
| { params: { token } }, | ||
| ); | ||
|
|
||
| yWebSocketProvider.once("sync", () => { | ||
| const yWorkflows = yDoc.getArray<YWorkflow>("workflows"); | ||
| if (!yWorkflows.length) { | ||
| console.warn("Imported project has no workflows"); | ||
| } | ||
|
|
||
| setIsProjectImporting(false); | ||
| }); | ||
| })(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for token retrieval.
There's no error handling for the token retrieval process. If getAccessToken() fails, the WebSocket connection won't be established and the import process will appear to hang without proper user feedback.
(async () => {
- const token = await getAccessToken();
+ try {
+ const token = await getAccessToken();
const yWebSocketProvider = new WebsocketProvider(
websocket,
`${project.id}:${DEFAULT_ENTRY_GRAPH_ID}`,
yDoc,
{ params: { token } },
);
yWebSocketProvider.once("sync", () => {
const yWorkflows = yDoc.getArray<YWorkflow>("workflows");
if (!yWorkflows.length) {
console.warn("Imported project has no workflows");
}
setIsProjectImporting(false);
});
+ } catch (error) {
+ console.error("Failed to get access token for project import:", error);
+ setIsProjectImporting(false);
+ }
})();📝 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.
| (async () => { | |
| const token = await getAccessToken(); | |
| const yWebSocketProvider = new WebsocketProvider( | |
| websocket, | |
| `${project.id}:${DEFAULT_ENTRY_GRAPH_ID}`, | |
| yDoc, | |
| { params: { token } }, | |
| ); | |
| yWebSocketProvider.once("sync", () => { | |
| const yWorkflows = yDoc.getArray<YWorkflow>("workflows"); | |
| if (!yWorkflows.length) { | |
| console.warn("Imported project has no workflows"); | |
| } | |
| setIsProjectImporting(false); | |
| }); | |
| })(); | |
| } | |
| (async () => { | |
| try { | |
| const token = await getAccessToken(); | |
| const yWebSocketProvider = new WebsocketProvider( | |
| websocket, | |
| `${project.id}:${DEFAULT_ENTRY_GRAPH_ID}`, | |
| yDoc, | |
| { params: { token } }, | |
| ); | |
| yWebSocketProvider.once("sync", () => { | |
| const yWorkflows = yDoc.getArray<YWorkflow>("workflows"); | |
| if (!yWorkflows.length) { | |
| console.warn("Imported project has no workflows"); | |
| } | |
| setIsProjectImporting(false); | |
| }); | |
| } catch (error) { | |
| console.error("Failed to get access token for project import:", error); | |
| setIsProjectImporting(false); | |
| } | |
| })(); | |
| } |
| (async () => { | ||
| const params: Record<string, string> = {}; | ||
| if (isProtected) { | ||
| const token = await getAccessToken(); | ||
| params.token = token; | ||
| } | ||
|
|
||
| setIsSynced(true); // Mark as synced | ||
| }); | ||
| yWebSocketProvider = new WebsocketProvider( | ||
| websocket, | ||
| `${projectId}:${workflowId}`, | ||
| yDoc, | ||
| { | ||
| params, | ||
| }, | ||
| ); | ||
|
|
||
| yWebSocketProvider.once("sync", () => { | ||
| if (yWorkflows.length === 0) { | ||
| yDoc.transact(() => { | ||
| const yWorkflow = yWorkflowConstructor( | ||
| DEFAULT_ENTRY_GRAPH_ID, | ||
| "Main Workflow", | ||
| ); | ||
| yWorkflows.insert(0, [yWorkflow]); | ||
| }); | ||
| } | ||
|
|
||
| setIsSynced(true); // Mark as synced | ||
| }); | ||
| })(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for token retrieval.
The implementation lacks error handling for the token retrieval process. If getAccessToken() fails, the WebSocket connection won't be established correctly, which could lead to silent failures.
(async () => {
- const params: Record<string, string> = {};
- if (isProtected) {
- const token = await getAccessToken();
- params.token = token;
- }
+ try {
+ const params: Record<string, string> = {};
+ if (isProtected) {
+ const token = await getAccessToken();
+ params.token = token;
+ }
+
+ yWebSocketProvider = new WebsocketProvider(
+ websocket,
+ `${projectId}:${workflowId}`,
+ yDoc,
+ {
+ params,
+ },
+ );
+
+ yWebSocketProvider.once("sync", () => {
+ if (yWorkflows.length === 0) {
+ yDoc.transact(() => {
+ const yWorkflow = yWorkflowConstructor(
+ DEFAULT_ENTRY_GRAPH_ID,
+ "Main Workflow",
+ );
+ yWorkflows.insert(0, [yWorkflow]);
+ });
+ }
+
+ setIsSynced(true); // Mark as synced
+ });
+ } catch (error) {
+ console.error("Failed to set up WebSocket connection:", error);
+ // Consider implementing fallback behavior or retry mechanism
+ }
-
- yWebSocketProvider = new WebsocketProvider(
- websocket,
- `${projectId}:${workflowId}`,
- yDoc,
- {
- params,
- },
- );
-
- yWebSocketProvider.once("sync", () => {
- if (yWorkflows.length === 0) {
- yDoc.transact(() => {
- const yWorkflow = yWorkflowConstructor(
- DEFAULT_ENTRY_GRAPH_ID,
- "Main Workflow",
- );
- yWorkflows.insert(0, [yWorkflow]);
- });
- }
-
- setIsSynced(true); // Mark as synced
- });
})();📝 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.
| (async () => { | |
| const params: Record<string, string> = {}; | |
| if (isProtected) { | |
| const token = await getAccessToken(); | |
| params.token = token; | |
| } | |
| setIsSynced(true); // Mark as synced | |
| }); | |
| yWebSocketProvider = new WebsocketProvider( | |
| websocket, | |
| `${projectId}:${workflowId}`, | |
| yDoc, | |
| { | |
| params, | |
| }, | |
| ); | |
| yWebSocketProvider.once("sync", () => { | |
| if (yWorkflows.length === 0) { | |
| yDoc.transact(() => { | |
| const yWorkflow = yWorkflowConstructor( | |
| DEFAULT_ENTRY_GRAPH_ID, | |
| "Main Workflow", | |
| ); | |
| yWorkflows.insert(0, [yWorkflow]); | |
| }); | |
| } | |
| setIsSynced(true); // Mark as synced | |
| }); | |
| })(); | |
| (async () => { | |
| try { | |
| const params: Record<string, string> = {}; | |
| if (isProtected) { | |
| const token = await getAccessToken(); | |
| params.token = token; | |
| } | |
| yWebSocketProvider = new WebsocketProvider( | |
| websocket, | |
| `${projectId}:${workflowId}`, | |
| yDoc, | |
| { | |
| params, | |
| }, | |
| ); | |
| yWebSocketProvider.once("sync", () => { | |
| if (yWorkflows.length === 0) { | |
| yDoc.transact(() => { | |
| const yWorkflow = yWorkflowConstructor( | |
| DEFAULT_ENTRY_GRAPH_ID, | |
| "Main Workflow", | |
| ); | |
| yWorkflows.insert(0, [yWorkflow]); | |
| }); | |
| } | |
| setIsSynced(true); // Mark as synced | |
| }); | |
| } catch (error) { | |
| console.error("Failed to set up WebSocket connection:", error); | |
| // Consider implementing fallback behavior or retry mechanism | |
| } | |
| })(); |
| (async () => { | ||
| const token = await getAccessToken(); | ||
|
|
||
| const yWebSocketProvider = new WebsocketProvider( | ||
| websocket, | ||
| `${project.id}:${DEFAULT_ENTRY_GRAPH_ID}`, | ||
| yDoc, | ||
| { params: { token } }, | ||
| ); | ||
|
|
||
| yWebSocketProvider.once("sync", () => { | ||
| const yWorkflows = yDoc.getArray<YWorkflow>("workflows"); | ||
| if (!yWorkflows.length) { | ||
| console.warn("Imported project has no workflows"); | ||
| } | ||
|
|
||
| setIsWorkflowImporting(false); | ||
| }); | ||
| })(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Missing WebSocket provider cleanup.
The code initializes a WebSocket provider but doesn't store a reference to it for later cleanup, which could lead to resource leaks or lingering connections.
Consider storing the WebSocket provider reference and cleaning it up when no longer needed:
+ const wsProviderRef = useRef<WebsocketProvider | null>(null);
+ // Add cleanup in useEffect
+ useEffect(() => {
+ return () => {
+ if (wsProviderRef.current) {
+ wsProviderRef.current.destroy();
+ wsProviderRef.current = null;
+ }
+ };
+ }, []);
// Then in the connection code:
(async () => {
const token = await getAccessToken();
const yWebSocketProvider = new WebsocketProvider(
websocket,
`${project.id}:${DEFAULT_ENTRY_GRAPH_ID}`,
yDoc,
{ params: { token } },
);
+ wsProviderRef.current = yWebSocketProvider;
// ... rest of the code
})();Also, don't forget to add the appropriate imports:
- import { ChangeEvent, useCallback, useRef, useState } from "react";
+ import { ChangeEvent, useCallback, useEffect, useRef, useState } from "react";📝 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.
| (async () => { | |
| const token = await getAccessToken(); | |
| const yWebSocketProvider = new WebsocketProvider( | |
| websocket, | |
| `${project.id}:${DEFAULT_ENTRY_GRAPH_ID}`, | |
| yDoc, | |
| { params: { token } }, | |
| ); | |
| yWebSocketProvider.once("sync", () => { | |
| const yWorkflows = yDoc.getArray<YWorkflow>("workflows"); | |
| if (!yWorkflows.length) { | |
| console.warn("Imported project has no workflows"); | |
| } | |
| setIsWorkflowImporting(false); | |
| }); | |
| })(); | |
| } | |
| // Import with the additional useEffect hook | |
| import { ChangeEvent, useCallback, useEffect, useRef, useState } from "react"; | |
| // ... other code in the file | |
| // Add a ref for the WebSocket provider instance | |
| const wsProviderRef = useRef<WebsocketProvider | null>(null); | |
| // Add cleanup for the WebSocket provider on unmount | |
| useEffect(() => { | |
| return () => { | |
| if (wsProviderRef.current) { | |
| wsProviderRef.current.destroy(); | |
| wsProviderRef.current = null; | |
| } | |
| }; | |
| }, []); | |
| // ... code where the async connection is made | |
| (async () => { | |
| const token = await getAccessToken(); | |
| const yWebSocketProvider = new WebsocketProvider( | |
| websocket, | |
| `${project.id}:${DEFAULT_ENTRY_GRAPH_ID}`, | |
| yDoc, | |
| { params: { token } }, | |
| ); | |
| wsProviderRef.current = yWebSocketProvider; | |
| yWebSocketProvider.once("sync", () => { | |
| const yWorkflows = yDoc.getArray<YWorkflow>("workflows"); | |
| if (!yWorkflows.length) { | |
| console.warn("Imported project has no workflows"); | |
| } | |
| setIsWorkflowImporting(false); | |
| }); | |
| })(); | |
| // ... remaining code in the file |
Overview
What I've done
What I haven't done
How I tested
Screenshot
Which point I want you to review particularly
Memo
Summary by CodeRabbit