-
Notifications
You must be signed in to change notification settings - Fork 68
fix: update wss url to fix co-editing #404
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThis pull request primarily consists of formatting and whitespace adjustments across demo files, along with configuration updates for collaborative editing. Key changes include updating WebSocket server endpoints and integrating Yjs-based real-time collaboration dependencies. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
packages/docs/fluent-editor/demos/ai.vuepackages/docs/fluent-editor/demos/collaborative-editing.vuepackages/docs/fluent-editor/demos/file-upload-handle.vuepackages/docs/fluent-editor/demos/flow-chart-background.vuepackages/docs/fluent-editor/demos/flow-chart-grid.vuepackages/docs/fluent-editor/demos/flow-chart-resize.vuepackages/docs/fluent-editor/demos/flow-chart.vuepackages/docs/fluent-editor/demos/format-painter.vuepackages/docs/fluent-editor/demos/get-content-delta.vuepackages/docs/fluent-editor/demos/get-content-html.vuepackages/docs/fluent-editor/demos/header-list.vuepackages/docs/fluent-editor/demos/mind-map-background.vuepackages/docs/fluent-editor/demos/mind-map-line.vuepackages/docs/fluent-editor/demos/mind-map-resize.vuepackages/docs/fluent-editor/demos/mind-map-theme.vuepackages/docs/fluent-editor/demos/mind-map.vuepackages/docs/fluent-editor/demos/screenshot.vuepackages/docs/fluent-editor/demos/table-up-menu.vuepackages/docs/fluent-editor/demos/table-up-shortcut.vuepackages/fluent-editor/scripts/pre-release.cjspackages/projects/src/views/yuque/YuQue.vue
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-29T06:42:59.434Z
Learnt from: vaebe
Repo: opentiny/tiny-editor PR: 365
File: packages/fluent-editor/vite.config.ts:18-26
Timestamp: 2025-10-29T06:42:59.434Z
Learning: In projects using Quill editor: `Quill.import('parchment')` dynamically loads parchment from Quill's internal registry at runtime and does not require a direct dependency on the parchment package. Only type imports like `import type { Root } from 'parchment'` may be needed, which can be satisfied by having parchment in devDependencies.
Applied to files:
packages/docs/fluent-editor/demos/flow-chart.vuepackages/docs/fluent-editor/demos/flow-chart-background.vuepackages/docs/fluent-editor/demos/file-upload-handle.vuepackages/docs/fluent-editor/demos/mind-map-line.vuepackages/docs/fluent-editor/demos/mind-map-resize.vuepackages/docs/fluent-editor/demos/mind-map-theme.vuepackages/projects/src/views/yuque/YuQue.vuepackages/docs/fluent-editor/demos/flow-chart-grid.vuepackages/docs/fluent-editor/demos/mind-map-background.vuepackages/docs/fluent-editor/demos/mind-map.vuepackages/docs/fluent-editor/demos/collaborative-editing.vue
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (24)
packages/docs/fluent-editor/demos/table-up-shortcut.vue (1)
23-24: Formatting improvement looks good.The added whitespace between the Promise.all destructuring and the subsequent FluentEditor.register call improves readability by clearly separating the async import section from the initialization logic.
packages/docs/fluent-editor/demos/header-list.vue (1)
12-17: Verify that whitespace-only changes align with PR intent.Line 16 removes a blank line between the
Promise.allblock and the subsequentifstatement. This is a purely cosmetic formatting change with no functional impact. However, the PR title indicates this is a fix for WebSocket URLs in co-editing, which this file does not address.Please clarify whether this formatting change is intentional or if it's part of a separate cleanup. If it's formatting-only, consider batching it into a dedicated whitespace/formatting PR rather than mixing it with functional fixes.
packages/docs/fluent-editor/demos/format-painter.vue (1)
5-5: LGTM! Formatting cleanup.Removed trailing whitespace from the variable declaration. This is good code hygiene with no functional impact.
packages/docs/fluent-editor/demos/table-up-menu.vue (1)
21-21: LGTM – Formatting improvement.The added blank line appropriately separates the import statements from the subsequent logic.
packages/docs/fluent-editor/demos/ai.vue (1)
17-17: LGTM – Consistent formatting.Trailing comma follows JavaScript/TypeScript conventions and improves diff readability.
packages/docs/fluent-editor/demos/flow-chart-background.vue (1)
11-15: LGTM – Formatting consistency.The restructured destructuring assignment improves readability with no behavioral changes.
packages/docs/fluent-editor/demos/mind-map-background.vue (1)
9-24: LGTM – Formatting improvements.The reformatted destructuring and added blank line enhance code readability with no functional changes.
packages/docs/fluent-editor/demos/screenshot.vue (1)
21-21: LGTM – Whitespace cleanup.The formatting adjustment removes unnecessary trailing whitespace.
packages/docs/fluent-editor/demos/file-upload-handle.vue (1)
13-19: Formatting improvement looks good.The alignment adjustment to the destructuring assignment improves readability without affecting functionality.
packages/docs/fluent-editor/demos/mind-map.vue (1)
9-23: Formatting improvement looks good.The alignment adjustment to the destructuring assignment is consistent with the broader formatting cleanup across demo files.
packages/docs/fluent-editor/demos/mind-map-line.vue (1)
9-24: Formatting improvement looks good.The alignment adjustment and added blank line improve code readability without affecting functionality.
packages/docs/fluent-editor/demos/collaborative-editing.vue (1)
28-46: Formatting improvement looks good.The alignment adjustment to the destructuring assignment is consistent with the broader formatting cleanup.
packages/docs/fluent-editor/demos/get-content-html.vue (1)
59-63: Good consistency improvement.Changing module configuration keys to quoted string literals improves consistency across the codebase, especially since some keys (like
'table-up') require quotes due to special characters.packages/fluent-editor/scripts/pre-release.cjs (1)
27-27: Good improvement: const over let.Making
defaultVersionimmutable is the correct choice since it's never reassigned. However, this change appears unrelated to the PR's stated objective of fixing co-editing by updating the WSS URL.packages/docs/fluent-editor/demos/flow-chart.vue (1)
11-19: Formatting-only change.The whitespace adjustments to the destructuring assignment have no functional impact. However, these changes appear unrelated to the PR's stated objective of fixing co-editing.
packages/docs/fluent-editor/demos/mind-map-resize.vue (2)
17-32: Formatting-only change.The whitespace and indentation adjustments have no functional impact. These changes are unrelated to the PR's stated objective of fixing co-editing.
54-54: No functional change detected.Line 54 appears to have only formatting adjustments.
packages/docs/fluent-editor/demos/get-content-delta.vue (1)
60-64: Style change: quoted object keys.Changing from unquoted to quoted object keys is functionally equivalent in JavaScript. This may be for linter compliance or consistency. However, this change is unrelated to the PR's stated objective of fixing co-editing.
packages/docs/fluent-editor/demos/flow-chart-grid.vue (1)
11-19: Formatting-only change.The whitespace adjustments are functionally equivalent to the previous code. These changes appear unrelated to the PR's stated objective of fixing co-editing.
packages/docs/fluent-editor/demos/flow-chart-resize.vue (1)
11-15: LGTM! Formatting improvements.The whitespace and trailing comma adjustments improve consistency without affecting runtime behavior.
Also applies to: 20-20
packages/docs/fluent-editor/demos/mind-map-theme.vue (1)
9-16: LGTM! Formatting improvements.Consistent whitespace adjustments with no runtime impact.
Also applies to: 24-24
packages/projects/src/views/yuque/YuQue.vue (3)
172-179: LGTM! Collaborative editing dependencies properly configured.The deps object correctly wires all the Yjs ecosystem dependencies to the collaborative-editing module. This enables real-time synchronization, awareness, and cursor tracking.
183-183: WebSocket URL migration is complete and consistent.The old IP-based URL (
wss://120.26.92.145:1234) has been completely removed from the codebase. The new domain-based URL (wss://ai.opentiny.design/tiny-editor/) is correctly configured in the collaborative-editing provider and appears consistently across all necessary locations (YuQue.vue and collaborative-editing.vue). The configuration structure is proper with serverUrl correctly set in the provider options.
5-5: All collaborative editing dependencies (yjs, y-protocols, y-quill, y-websocket, y-indexeddb, quill-cursors) are already declared in packages/projects/package.json and available at runtime.Likely an incorrect or invalid review comment.
| type: 'websocket', | ||
| options: { | ||
| serverUrl: 'wss://120.26.92.145:1234', | ||
| serverUrl: 'wss://ai.opentiny.design/tiny-editor/', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the file and its context
cat -n packages/docs/fluent-editor/demos/collaborative-editing.vue | sed -n '90,102p'Repository: opentiny/tiny-editor
Length of output: 532
🏁 Script executed:
# Test the WebSocket endpoint accessibility
curl -i -N \
-H "Connection: Upgrade" \
-H "Upgrade: websocket" \
-H "Sec-WebSocket-Version: 13" \
-H "Sec-WebSocket-Key: x3JJHMbDL1EzLkh9GBhXDw==" \
--max-time 10 \
"https://ai.opentiny.design/tiny-editor/" 2>&1Repository: opentiny/tiny-editor
Length of output: 652
🏁 Script executed:
# Also check for any related WebSocket endpoint references in the file
rg "serverUrl|websocket|wss://" packages/docs/fluent-editor/demos/collaborative-editing.vue -A 2 -B 2Repository: opentiny/tiny-editor
Length of output: 400
WebSocket endpoint is not responding with proper upgrade protocol.
The endpoint wss://ai.opentiny.design/tiny-editor/ is accessible (HTTP 200), but it is not returning the required HTTP 101 Switching Protocols response for WebSocket connections. A proper WebSocket endpoint must respond with HTTP 101 to complete the protocol upgrade. Without this, the y-websocket provider will fail to establish a WebSocket connection, breaking collaborative editing functionality. Verify that the server-side WebSocket handler is properly configured and deployed.
🤖 Prompt for AI Agents
In packages/docs/fluent-editor/demos/collaborative-editing.vue around line 96,
the configured serverUrl 'wss://ai.opentiny.design/tiny-editor/' is responding
HTTP 200 instead of the required HTTP 101 upgrade for WebSocket, so fix this by
either pointing serverUrl to a properly configured WebSocket endpoint (ensure it
supports the WebSocket upgrade and returns HTTP 101) or update the server:
enable a WebSocket handler (e.g., correct WebSocket/upgrade route, remove
HTTP-only middleware on that path, mount a websocket server like ws/y-websocket,
and redeploy) and verify using a websocket client that a handshake returns 101;
after confirming, update the URL (remove or add trailing slash as needed) in
this file to the working wss/ws endpoint.
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.