Refactor(collaborative-editing): 协同编辑模块依赖注入#380
Conversation
WalkthroughIntroduced dependency injection for Yjs and collaborative editing components across multiple modules. Added new runtime dependencies (yjs, y-websocket, y-webrtc, y-quill, y-protocols, y-indexeddb, quill-cursors) and created a CollaborativeEditingDeps interface to abstract these dependencies. Refactored providers, awareness utilities, and the collaborative editor core to accept and validate injected dependencies instead of direct imports. Changes
Sequence DiagramsequenceDiagram
participant App as Application
participant CE as CollaborativeEditor
participant Registry as Provider Registry
participant Provider as WebsocketProvider
participant Awareness as Awareness Module
participant Deps as Injected Deps
App->>App: Prepare deps object<br/>(Y, Awareness, WebsocketProvider, etc.)
App->>CE: Initialize with options.deps
CE->>Deps: Resolve Y, Awareness,<br/>QuillBinding, QuillCursors
alt Missing dependencies
Deps-->>CE: Error (throw)
else All present
CE->>CE: Store deps on instance
CE->>Registry: Create provider<br/>with deps
Registry->>Provider: Instantiate<br/>WebsocketProvider
Provider->>Deps: Validate WebsocketProvider
alt Valid
Provider->>Provider: Initialize
else Invalid
Deps-->>Provider: Error (throw)
end
CE->>Awareness: bindAwarenessToCursors<br/>(awareness, cursors, quill,<br/>yText, Yjs namespace)
Awareness->>Deps: Use injected Yjs<br/>for position utilities
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/fluent-editor/src/modules/collaborative-editing/awareness/y-indexeddb.ts (1)
12-16: Store persistence instance and use clearData() for cleanup.The current cleanup uses
indexedDB.deleteDatabase(), but the learned best practice is to usepersistence.clearData()which properly closes connections and removes database metadata. The persistence instance should be stored to enable proper cleanup.Based on learnings
Apply this diff:
export function setupIndexedDB(doc: Y.Doc, IndexeddbPersistenceClass?: typeof IndexeddbPersistence): () => void { if (!IndexeddbPersistenceClass) { console.warn('[yjs] IndexeddbPersistence not provided, offline support disabled') return () => {} } const fullDbName = `tiny-editor-${doc.guid}` - new IndexeddbPersistenceClass(fullDbName, doc) + const persistence = new IndexeddbPersistenceClass(fullDbName, doc) return (): void => { - indexedDB.deleteDatabase(fullDbName) + persistence.clearData() } }
📜 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 (10)
packages/docs/fluent-editor/demos/collaborative-editing.vue(2 hunks)packages/docs/fluent-editor/docs/demo/collaborative-editing.md(7 hunks)packages/docs/package.json(1 hunks)packages/fluent-editor/src/modules/collaborative-editing/awareness/awareness.ts(4 hunks)packages/fluent-editor/src/modules/collaborative-editing/awareness/y-indexeddb.ts(1 hunks)packages/fluent-editor/src/modules/collaborative-editing/collaborative-editing.ts(4 hunks)packages/fluent-editor/src/modules/collaborative-editing/provider/providerRegistry.ts(2 hunks)packages/fluent-editor/src/modules/collaborative-editing/provider/webrtc.ts(2 hunks)packages/fluent-editor/src/modules/collaborative-editing/provider/websocket.ts(2 hunks)packages/fluent-editor/src/modules/collaborative-editing/types.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-09-11T03:19:54.210Z
Learnt from: vaebe
Repo: opentiny/tiny-editor PR: 338
File: packages/fluent-editor/src/modules/collaborative-editing/awareness/y-indexeddb.ts:9-13
Timestamp: 2025-09-11T03:19:54.210Z
Learning: In y-indexeddb cleanup functions, use persistence.clearData() instead of raw indexedDB.deleteDatabase() calls. The clearData() method properly closes connections and removes database metadata, while raw deleteDatabase() can leave hanging promises and open handles.
Applied to files:
packages/fluent-editor/src/modules/collaborative-editing/awareness/y-indexeddb.tspackages/fluent-editor/src/modules/collaborative-editing/collaborative-editing.ts
📚 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/collaborative-editing.vuepackages/docs/package.jsonpackages/docs/fluent-editor/docs/demo/collaborative-editing.mdpackages/fluent-editor/src/modules/collaborative-editing/awareness/awareness.tspackages/fluent-editor/src/modules/collaborative-editing/types.ts
📚 Learning: 2025-09-23T02:27:09.479Z
Learnt from: Yinlin124
Repo: opentiny/tiny-editor PR: 346
File: packages/fluent-editor/src/modules/collaborative-editing/awareness/awareness.ts:65-70
Timestamp: 2025-09-23T02:27:09.479Z
Learning: The moveCursor method from quill-cursors library has built-in negative value checking/handling, so manual normalization of negative length values is not required when calling cursorsModule.moveCursor().
Applied to files:
packages/fluent-editor/src/modules/collaborative-editing/awareness/awareness.ts
🧬 Code graph analysis (5)
packages/fluent-editor/src/modules/collaborative-editing/provider/providerRegistry.ts (1)
packages/fluent-editor/src/modules/collaborative-editing/types.ts (1)
CollaborativeEditingDeps(38-46)
packages/fluent-editor/src/modules/collaborative-editing/provider/websocket.ts (1)
packages/fluent-editor/src/modules/collaborative-editing/types.ts (2)
CollaborativeEditingDeps(38-46)ProviderEventHandlers(11-16)
packages/fluent-editor/src/modules/collaborative-editing/provider/webrtc.ts (1)
packages/fluent-editor/src/modules/collaborative-editing/types.ts (2)
CollaborativeEditingDeps(38-46)ProviderEventHandlers(11-16)
packages/fluent-editor/src/modules/collaborative-editing/collaborative-editing.ts (4)
packages/fluent-editor/src/modules/collaborative-editing/provider/providerRegistry.ts (1)
UnifiedProvider(20-29)packages/fluent-editor/src/modules/collaborative-editing/types.ts (2)
CollaborativeEditingDeps(38-46)YjsOptions(48-60)packages/fluent-editor/src/modules/collaborative-editing/awareness/awareness.ts (1)
bindAwarenessToCursors(35-142)packages/fluent-editor/src/modules/collaborative-editing/awareness/y-indexeddb.ts (1)
setupIndexedDB(4-17)
packages/fluent-editor/src/modules/collaborative-editing/types.ts (1)
packages/fluent-editor/src/modules/collaborative-editing/awareness/awareness.ts (1)
AwarenessOptions(17-21)
🔇 Additional comments (14)
packages/fluent-editor/src/modules/collaborative-editing/provider/providerRegistry.ts (1)
17-17: LGTM!The addition of the optional
depsfield toProviderConstructorPropscorrectly enables dependency injection for provider implementations, aligning with the broader refactoring goal.packages/docs/fluent-editor/demos/collaborative-editing.vue (2)
54-69: LGTM!The dynamic imports and destructuring correctly implement the dependency injection pattern, enabling runtime loading of Yjs-related modules.
108-115: Deps object correctly configured.The dependency injection configuration properly wires all required Yjs components (Y, Awareness, QuillBinding, QuillCursors, WebsocketProvider, IndexeddbPersistence) for collaborative editing.
packages/fluent-editor/src/modules/collaborative-editing/awareness/y-indexeddb.ts (1)
5-8: LGTM!The validation and graceful degradation when
IndexeddbPersistenceis not provided ensures offline support can be optionally disabled without breaking the application.packages/fluent-editor/src/modules/collaborative-editing/awareness/awareness.ts (2)
35-41: LGTM!The addition of the
Yjsparameter enables dependency injection for the Y namespace, replacing the previous direct import pattern. This aligns with the broader refactoring goal.
55-56: Yjs namespace correctly used throughout.All Y.* API calls have been properly updated to use the injected
Yjsparameter, ensuring consistency with the dependency injection pattern.Also applies to: 79-80, 84-85
packages/fluent-editor/src/modules/collaborative-editing/types.ts (2)
38-46: LGTM!The
CollaborativeEditingDepsinterface correctly defines the dependency injection contract, with core dependencies (Y, Awareness, QuillBinding, QuillCursors) as required and provider-specific dependencies (WebsocketProvider, WebrtcProvider, IndexeddbPersistence) as optional.
54-54: Good type improvements.Adding the
depsfield toYjsOptionsand tightening theonErrorcallback type to(error: Error) => voidimprove type safety.Also applies to: 58-58
packages/fluent-editor/src/modules/collaborative-editing/collaborative-editing.ts (3)
24-32: Good validation for required dependencies.The dependency validation ensures that core collaborative editing components (Y, Awareness, QuillBinding, QuillCursors) are available before proceeding. The fallback to
windowwhendepsis not provided maintains backward compatibility while the validation catches missing dependencies.
34-50: Correct dependency usage in initialization.The injected dependencies are properly used to instantiate the Yjs document, cursors module, and awareness, ensuring the refactored code uses the provided implementations.
64-64: Dependencies correctly propagated.The
depsobject is properly passed to provider creation (line 64), theYnamespace to awareness binding (line 78), andIndexeddbPersistenceto offline storage setup (line 90).Also applies to: 78-78, 90-90
packages/fluent-editor/src/modules/collaborative-editing/provider/websocket.ts (2)
11-11: Good type improvements.Tightening the
awarenesstype fromanytoAwarenessand adding explicit optional fields (resyncInterval,maxBackoffTime,disableBc) toWebsocketProviderOptionsimproves type safety.Also applies to: 15-17
78-97: Dependency injection correctly implemented.The constructor properly accepts and validates the
depsparameter, extracting required Yjs components (Y, Awareness, WebsocketProvider) and using them to instantiate the document, awareness, and provider. The validation on lines 92-94 ensuresWebsocketProvideris available before proceeding.packages/docs/package.json (1)
27-41: No security advisories found; verify quill-cursors minor patch availability.Verification confirms that all Yjs-related dependencies are free from known security vulnerabilities. Six packages are already on their latest versions, and
quill-cursors@4.0.3has a newer patch available (4.0.4). Consider updatingquill-cursorsto the latest patch version if compatibility is confirmed.
| import FluentEditor from '@opentiny/fluent-editor' | ||
|
|
||
| let editor | ||
| const editorRef = document.querySelector('#editor') | ||
|
|
||
| Promise.all([ | ||
| import('@opentiny/fluent-editor'), | ||
| import('yjs'), | ||
| import('y-protocols/awareness'), | ||
| import('y-quill'), | ||
| import('y-websocket'), | ||
| import('y-indexeddb'), | ||
| import('quill-cursors'), | ||
| ]).then( | ||
| ([ | ||
| { default: FluentEditor, CollaborationModule }, | ||
| Y, | ||
| { Awareness }, | ||
| { QuillBinding }, | ||
| { WebsocketProvider }, | ||
| { IndexeddbPersistence }, | ||
| { default: QuillCursors }, | ||
| ]) => { | ||
| FluentEditor.register('modules/collaborative-editing', CollaborationModule, true) | ||
|
|
||
| editor = new FluentEditor(editorRef, { | ||
| theme: 'snow', | ||
| modules: { | ||
| 'collaborative-editing': { | ||
| deps: { Y, Awareness, QuillBinding, QuillCursors, WebsocketProvider, IndexeddbPersistence }, | ||
| provider: { | ||
| type: 'websocket', | ||
| options: { | ||
| serverUrl: 'wss://demos.yjs.dev/ws', | ||
| roomName: 'Tiny-Editor-Demo', | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }) | ||
| }, | ||
| ).catch((error) => { | ||
| console.error('Failed to initialize FluentEditor:', error) | ||
| }) |
There was a problem hiding this comment.
Remove the static import to keep the example fully dynamic.
This section is supposed to demonstrate lazy-loading all collaborative-editing dependencies, but the top-level import FluentEditor from '@opentiny/fluent-editor' keeps the module (and its transitive deps) eagerly bundled. That defeats the goal of dependency injection and duplicates the subsequent dynamic import call. Please drop the static import and rely on the FluentEditor binding returned inside Promise.all.
-import FluentEditor from '@opentiny/fluent-editor'
-
-let editor
+let editor🤖 Prompt for AI Agents
packages/docs/fluent-editor/docs/demo/collaborative-editing.md lines 36-79:
remove the top-level static import "import FluentEditor from
'@opentiny/fluent-editor'" so the example truly lazy-loads all dependencies;
rely on the FluentEditor binding returned by the dynamic import in Promise.all
(ensure the destructured default FluentEditor is used where the static one was
referenced) and keep the rest of the Promise.all dynamic imports/usage
unchanged.
| import FluentEditor from '@opentiny/fluent-editor' | ||
|
|
||
| let editor | ||
| const editorRef = document.querySelector('#editor') | ||
|
|
||
| Promise.all([ | ||
| import('@opentiny/fluent-editor'), | ||
| import('yjs'), | ||
| import('y-protocols/awareness'), | ||
| import('y-quill'), | ||
| import('y-webrtc'), | ||
| import('y-indexeddb'), | ||
| import('quill-cursors'), | ||
| ]).then( | ||
| ([ | ||
| { default: FluentEditor, CollaborationModule }, | ||
| Y, | ||
| { Awareness }, | ||
| { QuillBinding }, | ||
| { WebrtcProvider }, | ||
| { IndexeddbPersistence }, | ||
| { default: QuillCursors }, | ||
| ]) => { | ||
| FluentEditor.register('modules/collaborative-editing', CollaborationModule, true) | ||
|
|
||
| editor = new FluentEditor(editorRef, { | ||
| theme: 'snow', | ||
| modules: { | ||
| 'collaborative-editing': { | ||
| deps: { Y, Awareness, QuillBinding, QuillCursors, WebrtcProvider, IndexeddbPersistence }, | ||
| provider: { | ||
| type: 'webrtc', | ||
| options: { | ||
| roomName: 'Tiny-Editor-WebRTC', | ||
| signaling: ['wss://signaling.yjs.dev'], | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }) | ||
| }, | ||
| ).catch((error) => { | ||
| console.error('Failed to initialize FluentEditor:', error) | ||
| }) |
There was a problem hiding this comment.
Mirror the dynamic loading pattern in the WebRTC example.
The WebRTC snippet still performs a static import FluentEditor ..., so the collaborative-editing bundle is eagerly pulled in despite the new lazy Promise.all loader. Remove that static import so the docs accurately reflect the dynamic dependency injection workflow and avoid bundling regressions for consumers.
-import FluentEditor from '@opentiny/fluent-editor'
-
-let editor
+let editor📝 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.
| import FluentEditor from '@opentiny/fluent-editor' | |
| let editor | |
| const editorRef = document.querySelector('#editor') | |
| Promise.all([ | |
| import('@opentiny/fluent-editor'), | |
| import('yjs'), | |
| import('y-protocols/awareness'), | |
| import('y-quill'), | |
| import('y-webrtc'), | |
| import('y-indexeddb'), | |
| import('quill-cursors'), | |
| ]).then( | |
| ([ | |
| { default: FluentEditor, CollaborationModule }, | |
| Y, | |
| { Awareness }, | |
| { QuillBinding }, | |
| { WebrtcProvider }, | |
| { IndexeddbPersistence }, | |
| { default: QuillCursors }, | |
| ]) => { | |
| FluentEditor.register('modules/collaborative-editing', CollaborationModule, true) | |
| editor = new FluentEditor(editorRef, { | |
| theme: 'snow', | |
| modules: { | |
| 'collaborative-editing': { | |
| deps: { Y, Awareness, QuillBinding, QuillCursors, WebrtcProvider, IndexeddbPersistence }, | |
| provider: { | |
| type: 'webrtc', | |
| options: { | |
| roomName: 'Tiny-Editor-WebRTC', | |
| signaling: ['wss://signaling.yjs.dev'], | |
| }, | |
| }, | |
| }, | |
| }, | |
| }, | |
| }) | |
| }, | |
| ).catch((error) => { | |
| console.error('Failed to initialize FluentEditor:', error) | |
| }) | |
| let editor | |
| const editorRef = document.querySelector('#editor') | |
| Promise.all([ | |
| import('@opentiny/fluent-editor'), | |
| import('yjs'), | |
| import('y-protocols/awareness'), | |
| import('y-quill'), | |
| import('y-webrtc'), | |
| import('y-indexeddb'), | |
| import('quill-cursors'), | |
| ]).then( | |
| ([ | |
| { default: FluentEditor, CollaborationModule }, | |
| Y, | |
| { Awareness }, | |
| { QuillBinding }, | |
| { WebrtcProvider }, | |
| { IndexeddbPersistence }, | |
| { default: QuillCursors }, | |
| ]) => { | |
| FluentEditor.register('modules/collaborative-editing', CollaborationModule, true) | |
| editor = new FluentEditor(editorRef, { | |
| theme: 'snow', | |
| modules: { | |
| 'collaborative-editing': { | |
| deps: { Y, Awareness, QuillBinding, QuillCursors, WebrtcProvider, IndexeddbPersistence }, | |
| provider: { | |
| type: 'webrtc', | |
| options: { | |
| roomName: 'Tiny-Editor-WebRTC', | |
| signaling: ['wss://signaling.yjs.dev'], | |
| }, | |
| }, | |
| }, | |
| }, | |
| }) | |
| }, | |
| ).catch((error) => { | |
| console.error('Failed to initialize FluentEditor:', error) | |
| }) |
🤖 Prompt for AI Agents
packages/docs/fluent-editor/docs/demo/collaborative-editing.md around lines 257
to 300: the file still uses a static top-level "import FluentEditor ..." which
defeats the lazy Promise.all dynamic loader; remove that static import line so
FluentEditor is only loaded via the Promise.all import (which already returns
default: FluentEditor), and ensure there are no duplicate declarations or
unresolved references to FluentEditor elsewhere in this snippet so the dynamic
import result is used exclusively.
3 participants
PR
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Summary by CodeRabbit
New Features
Documentation
Chores