[WIKI-844] fix: realtime sync post vite migration with title editor sync and indexed db access#8294
Conversation
|
Warning Rate limit exceeded@Palanikannan1437 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 21 minutes and 40 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughAdds server-side title synchronization with debounced, abortable persistence; converts and persists missing document binary formats on fetch; introduces Yjs/Hocuspocus collaboration plumbing and a dedicated title editor/navigation; refactors editor components to use a CollaborationProvider; and adds UI sync status and content-limit handling. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant TitleEd as Title Editor
participant TitleSync as TitleSyncExtension
participant Debounce as DebounceManager
participant TitleMgr as TitleUpdateManager
participant PageSvc as PageService
participant DB as Database
User->>TitleEd: Edit title
TitleEd->>TitleSync: Title change event (Yjs)
TitleSync->>Debounce: schedule(updateTitle, newTitle)
Note over Debounce: wait (e.g., 5s) / trailing-edge
Debounce->>TitleMgr: invoke updateTitle(newTitle)
TitleMgr->>PageSvc: updatePageProperties(id, { name: newTitle }, abortSignal)
PageSvc->>DB: persist title
DB-->>PageSvc: ack / error
PageSvc-->>TitleMgr: result
alt success
TitleMgr-->>Debounce: clear lastTitle
TitleSync->>TitleSync: broadcast realtime title event
else error
TitleMgr->>Debounce: schedule retry
end
sequenceDiagram
participant Client as Editor UI
participant CollabProv as CollaborationProvider
participant YjsProv as HocuspocusProvider
participant Store as PageStore
participant UI as SyncBadge/Banner
Client->>CollabProv: initialize with docId
CollabProv->>YjsProv: connect (auth, URL)
YjsProv-->>CollabProv: onStateChange(collabState)
CollabProv-->>Client: forward state
Client->>Store: setSyncingStatus(mappedStatus)
Store-->>UI: isSyncingWithServer updates
UI->>UI: render badge / show banner on errors
alt disconnected
Client->>Client: trigger usePageFallback (fetch fallback binary)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas needing extra attention:
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/editor/src/core/components/editors/document/page-renderer.tsx (1)
29-39: Fix duplicate property declarations in Props type.The
Propstype has duplicate declarations that cause TypeScript compilation errors:
flaggedExtensionsis declared on both lines 29 and 35disabledExtensionsis declared on both lines 21 and 36Remove the duplicate declarations on lines 34-38.
Apply this diff to fix the duplicate declarations:
tabIndex?: number; - extendedEditorProps?: IEditorPropsExtended; - flaggedExtensions: IEditorProps["flaggedExtensions"]; - disabledExtensions: IEditorProps["disabledExtensions"]; provider?: HocuspocusProvider; state?: TCollabValue["state"]; };packages/editor/src/core/components/editors/document/collaborative-editor.tsx (1)
57-83: MissingupdatePagePropertiesprop inuseCollaborativeEditorcall.The
updatePagePropertiesprop is destructured frompropsat line 49 but is not passed touseCollaborativeEditor. According to the hook's type definition inuse-collaborative-editor.ts, this prop is expected for title editor functionality.Add the missing prop:
onTransaction, placeholder, tabIndex, titleRef, user, actions, + updatePageProperties, });
🧹 Nitpick comments (17)
apps/live/src/extensions/title-update/title-utils.ts (1)
1-8: Duplicate implementation exists inpackages/editor/src/core/helpers/yjs-utils.ts.This function is identical to
extractTextFromHTMLat lines 207-211 inyjs-utils.ts. Consider importing from a shared location or consolidating into a single utility module to avoid drift between implementations.-/** - * Utility function to extract text from HTML content - */ -export const extractTextFromHTML = (html: string): string => { - // Use a regex to extract text between tags - const textMatch = html.replace(/<[^>]*>/g, ""); - return textMatch || ""; -}; +// Re-export from shared location or import from @plane/editor/helpers +export { extractTextFromHTML } from "@plane/editor/core/helpers/yjs-utils";Alternatively, if cross-package import isn't feasible, extract to a shared
@plane/utilspackage.packages/editor/src/styles/title-editor.css (1)
35-49: Consider consolidating duplicate placeholder styles.The two placeholder selectors (
.is-editor-empty:first-child::beforeand.is-empty::before) have identical styling. If both are needed for different TipTap states, consider extracting the common styles to reduce duplication.-.page-title-editor .ProseMirror h1.is-editor-empty:first-child::before { - content: attr(data-placeholder); - float: left; - color: var(--color-placeholder); - pointer-events: none; - height: 0; -} - -.page-title-editor .ProseMirror h1.is-empty::before { +.page-title-editor .ProseMirror h1.is-editor-empty:first-child::before, +.page-title-editor .ProseMirror h1.is-empty::before { content: attr(data-placeholder); float: left; color: var(--color-placeholder); pointer-events: none; height: 0; }apps/web/core/hooks/use-page-fallback.ts (1)
25-26: Consider optimizing the dependency chain.The current implementation has
hasConnectionFailed(derived fromcollaborationState) in thehandleUpdateDescriptiondependency array (line 73), which causes the callback to be recreated on every collaboration state change. This then triggers the useEffect (lines 75-82), creating a circular dependency pattern.While the logic is functionally safe due to the early return check (line 29), consider refactoring to improve efficiency:
- const handleUpdateDescription = useCallback(async () => { - if (!hasConnectionFailed) return; + const handleUpdateDescription = useCallback(async (connectionFailed: boolean) => { + if (!connectionFailed) return; const editor = editorRef.current; if (!editor) return; // ... rest of the function - }, [editorRef, fetchPageDescription, hasConnectionFailed, updatePageDescription]); + }, [editorRef, fetchPageDescription, updatePageDescription]); useEffect(() => { if (hasConnectionFailed) { - handleUpdateDescription(); + handleUpdateDescription(hasConnectionFailed); } else { // Reset toast flag when connection is restored hasShownFallbackToast.current = false; } }, [handleUpdateDescription, hasConnectionFailed]);This way,
handleUpdateDescriptionis only recreated when its actual dependencies change, not on every collaboration state change.Also applies to: 73-82
packages/editor/src/core/types/collaboration.ts (1)
29-33: Consider addingisDocReadyto the public state type.Based on the
useYjsSetuphook context provided,isDocReadyis computed and exposed in the returned state object. If consumers rely on this property (as mentioned in theCollaborationProvidercomments), consider adding it toCollaborationStatefor type safety.export type CollaborationState = { stage: CollabStage; isServerSynced: boolean; isServerDisconnected: boolean; + isDocReady: boolean; };packages/editor/src/core/components/editors/editor-container.tsx (2)
75-103: Effect cleanup may not run for all code paths.The cleanup function only executes if the
if (nodeId && !hasScrolledOnce.current)condition is true. If the condition is false, no cleanup runs, but this appears intentional since no listeners are attached in that case. However, consider restructuring to make the cleanup logic clearer.Consider restructuring for clarity:
useEffect(() => { const nodeId = window.location.href.split("#")[1]; + if (!nodeId || hasScrolledOnce.current) return; const handleSynced = () => scrollToNode(nodeId); + let listenerAttached = false; - if (nodeId && !hasScrolledOnce.current) { if (provider && state) { const { hasCachedContent } = state; if (hasCachedContent) { const hasScrolled = handleSynced(); if (!hasScrolled) { provider.on("synced", handleSynced); + listenerAttached = true; } } else if (provider.isSynced) { handleSynced(); } else { provider.on("synced", handleSynced); + listenerAttached = true; } } else { handleSynced(); } - return () => { - if (provider) { - provider.off("synced", handleSynced); - } - }; - } + + return () => { + if (listenerAttached && provider) { + provider.off("synced", handleSynced); + } + }; }, [scrollToNode, provider, state]);
76-76: Consider usingURLAPI for hash extraction.Using
split("#")[1]works but doesn't handle edge cases like encoded characters. Consider using theURLAPI for more robust parsing.- const nodeId = window.location.href.split("#")[1]; + const nodeId = window.location.hash.slice(1); // removes the leading '#'Or for full URL parsing:
const nodeId = new URL(window.location.href).hash.slice(1);packages/editor/src/core/helpers/yjs-utils.ts (1)
208-212: Consider handling HTML entities and consolidating duplicate code.
The regex-based approach doesn't decode HTML entities like
&,<,>, . If titles contain these, the output will include raw entity strings.This function duplicates
extractTextFromHTMLinapps/live/src/extensions/title-update/title-utils.ts. Consider exporting from a shared location to avoid drift.packages/editor/src/core/hooks/use-editor-navigation.ts (1)
114-118: Race condition risk withsetTimeoutfor focus after deletion.Using
setTimeout(..., 0)to ensure node deletion completes before focusing is fragile. The deletion is synchronous in ProseMirror, so thesetTimeoutmay not be necessary, or a more robust approach would be to userequestAnimationFrameor chain the focus command.Consider chaining the operations:
if (firstNode.content.size === 0) { - editor.commands.deleteNode("paragraph"); - // Use setTimeout to ensure the node is deleted before changing focus - setTimeout(() => titleEditor.commands.focus("end"), 0); + editor.commands.deleteNode("paragraph"); + titleEditor.commands.focus("end"); return true; }apps/live/src/extensions/title-update/title-update-manager.ts (2)
34-40: Minor: Avoid creating bound function on each call.
this.updateTitle.bind(this)creates a new function instance on everyscheduleUpdatecall. Consider binding once in the constructor or using an arrow function property.+ private boundUpdateTitle = this.updateTitle.bind(this); + scheduleUpdate(title: string): void { this.lastTitle = title; - this.debounceManager.schedule(this.updateTitle.bind(this), title); + this.debounceManager.schedule(this.boundUpdateTitle, title); }
73-81: Same bound function optimization applies here.
forceSavealso creates a new bound function. If implementing the constructor binding suggested earlier, update this call as well.async forceSave(): Promise<void> { if (!this.lastTitle) { return; } - await this.debounceManager.flush(this.updateTitle.bind(this)); + await this.debounceManager.flush(this.boundUpdateTitle); }apps/live/src/extensions/title-sync.ts (1)
102-106: Consider simplifying the title extraction logic.The loop overwrites
titleon each iteration, effectively only using the last event's title. If this is intentional, consider using the last event directly for clarity:private handleTitleChange(documentName: string, events: Y.YEvent<any>[]) { - let title = ""; - events.forEach((event) => { - title = extractTextFromHTML(event.currentTarget.toJSON()); - }); + const lastEvent = events.at(-1); + const title = lastEvent ? extractTextFromHTML(lastEvent.currentTarget.toJSON()) : "";packages/editor/src/core/hooks/use-yjs-setup.ts (2)
18-30: Unusedoptionsparameter in hook arguments.The
optionsparameter withmaxConnectionAttemptsis defined inUseYjsSetupArgsbut never destructured or used. The code usesDEFAULT_MAX_RETRIESconstant instead.Either remove the unused option or wire it up:
-export const useYjsSetup = ({ docId, serverUrl, authToken, onStateChange }: UseYjsSetupArgs) => { +export const useYjsSetup = ({ docId, serverUrl, authToken, onStateChange, options }: UseYjsSetupArgs) => { + const maxRetries = options?.maxConnectionAttempts ?? DEFAULT_MAX_RETRIES;Then replace
DEFAULT_MAX_RETRIESusages withmaxRetries.
234-243: Missing stage transition inhandleOnlinehandler.Unlike
handleVisibilityChangewhich sets stage toconnectingbefore reconnecting,handleOnlinedoesn't update the stage. This could leave the UI showing a stale disconnected state during reconnection.const handleOnline = () => { if (isDisposedRef.current) return; const wsProvider = provider.configuration.websocketProvider; if (wsProvider) { + // Reset retry count and update stage for fresh reconnection + retryCountRef.current = 0; + const newStage = { kind: "connecting" as const }; + stageRef.current = newStage; + setStage(newStage); + wsProvider.shouldConnect = true; wsProvider.disconnect(); wsProvider.connect(); } };apps/web/core/components/pages/editor/page-root.tsx (1)
165-165: Consider adding dismiss capability to the content limit banner.The
ContentLimitBannercomponent supports anonDismissprop for dismissibility, but it's not provided here. Once the banner appears, it persists until the page is reloaded. Consider whether users should be able to dismiss it:- {showContentTooLargeBanner && <ContentLimitBanner className="px-page-x" />} + {showContentTooLargeBanner && ( + <ContentLimitBanner + className="px-page-x" + onDismiss={() => setShowContentTooLargeBanner(false)} + /> + )}apps/live/src/extensions/title-update/debounce.ts (1)
193-206: Arbitrary delay after abort is fragile.The 20ms delay to "ensure the abort has had time to propagate" is a race condition waiting to happen. Abort signals are synchronous—handlers receive them immediately. The double-check afterwards suggests uncertainty about the state machine.
Consider restructuring to avoid the delay:
private async abortOngoingOperation(): Promise<void> { if (this.state.inProgress && this.state.abortController) { this.state.abortController.abort(); - - // Small delay to ensure the abort has had time to propagate - await new Promise((resolve) => setTimeout(resolve, 20)); - - // Double-check that state has been reset, force it if not - if (this.state.inProgress || this.state.abortController) { - this.state.inProgress = false; - this.state.abortController = null; - } + // State will be reset in performFunction's finally block } }The
finallyblock inperformFunctionalready resetsinProgressandabortController, so this double-reset is unnecessary if you trust the execution flow.apps/web/core/components/pages/editor/editor-body.tsx (1)
147-155: Consider using ref pattern foronCollaborationStateChangecallback.Including
onCollaborationStateChangein the dependency array could cause unnecessary effect runs if the parent recreates the callback. Theuse-yjs-setup.tshook uses a ref pattern for this exact reason.+ // Use ref to avoid effect re-runs when callback changes + const onCollaborationStateChangeRef = useRef(onCollaborationStateChange); + onCollaborationStateChangeRef.current = onCollaborationStateChange; + // Set syncing status when page changes and reset collaboration state useEffect(() => { setSyncingStatus("syncing"); - onCollaborationStateChange?.({ + onCollaborationStateChangeRef.current?.({ stage: { kind: "connecting" }, isServerSynced: false, isServerDisconnected: false, }); - }, [pageId, setSyncingStatus, onCollaborationStateChange]); + }, [pageId, setSyncingStatus]);packages/editor/src/core/hooks/use-collaborative-editor.ts (1)
170-191: Avoid type assertion; align types directly.The type assertion
as Parameters<typeof useTitleEditor>[0]at line 191 indicates a type mismatch between the inline type definition (lines 170-178) and whatuseTitleEditorexpects. This bypasses TypeScript's type checking and can hide errors if the hook's signature changes.Consider importing or reusing the parameter type from
useTitleEditordirectly:+import type { TitleEditorConfig } from "./use-title-editor"; // or define shared type + - const titleEditorConfig = useMemo<{ - id: string; - editable: boolean; - provider: HocuspocusProvider; - titleRef?: React.MutableRefObject<EditorTitleRefApi | null>; - updatePageProperties?: ICollaborativeDocumentEditorProps["updatePageProperties"]; - extensions: Extensions; - extendedEditorProps?: IEditorPropsExtended; - }>( + const titleEditorConfig = useMemo<TitleEditorConfig>( () => ({ ... }), [...] ); - const titleEditor = useTitleEditor(titleEditorConfig as Parameters<typeof useTitleEditor>[0]); + const titleEditor = useTitleEditor(titleEditorConfig);
📜 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 (31)
apps/live/src/extensions/database.ts(2 hunks)apps/live/src/extensions/title-sync.ts(1 hunks)apps/live/src/extensions/title-update/debounce.ts(1 hunks)apps/live/src/extensions/title-update/title-update-manager.ts(1 hunks)apps/live/src/extensions/title-update/title-utils.ts(1 hunks)apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/header.tsx(2 hunks)apps/web/core/components/pages/editor/content-limit-banner.tsx(1 hunks)apps/web/core/components/pages/editor/editor-body.tsx(9 hunks)apps/web/core/components/pages/editor/page-root.tsx(6 hunks)apps/web/core/components/pages/header/syncing-badge.tsx(1 hunks)apps/web/core/hooks/use-page-fallback.ts(3 hunks)apps/web/core/store/pages/base-page.ts(5 hunks)packages/editor/package.json(1 hunks)packages/editor/src/core/components/editors/document/collaborative-editor.tsx(4 hunks)packages/editor/src/core/components/editors/document/page-renderer.tsx(5 hunks)packages/editor/src/core/components/editors/editor-container.tsx(2 hunks)packages/editor/src/core/components/editors/editor-content.tsx(1 hunks)packages/editor/src/core/contexts/collaboration-context.tsx(1 hunks)packages/editor/src/core/contexts/index.ts(1 hunks)packages/editor/src/core/extensions/title-extension.ts(1 hunks)packages/editor/src/core/helpers/yjs-utils.ts(7 hunks)packages/editor/src/core/hooks/use-collaborative-editor.ts(3 hunks)packages/editor/src/core/hooks/use-editor-navigation.ts(1 hunks)packages/editor/src/core/hooks/use-title-editor.ts(1 hunks)packages/editor/src/core/hooks/use-yjs-setup.ts(1 hunks)packages/editor/src/core/plugins/highlight.ts(1 hunks)packages/editor/src/core/types/collaboration.ts(1 hunks)packages/editor/src/core/types/editor.ts(4 hunks)packages/editor/src/core/types/hook.ts(1 hunks)packages/editor/src/styles/index.css(1 hunks)packages/editor/src/styles/title-editor.css(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,mts,cts}
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/*.{ts,tsx,mts,cts}: Useconsttype parameters for more precise literal inference in TypeScript 5.0+
Use thesatisfiesoperator to validate types without widening them
Leverage inferred type predicates to reduce the need for explicitisreturn types in filter/check functions
UseNoInfer<T>utility to block inference for specific type arguments when they should be determined by other arguments
Utilize narrowing inswitch(true)blocks for control flow analysis (TypeScript 5.3+)
Rely on narrowing from direct boolean comparisons for type guards
Trust preserved narrowing in closures when variables aren't modified after the check (TypeScript 5.4+)
Use constant indices to narrow object/array properties (TypeScript 5.5+)
Use standard ECMAScript decorators (Stage 3) instead of legacyexperimentalDecorators
Useusingdeclarations for explicit resource management with Disposable pattern instead of manual cleanup (TypeScript 5.2+)
Usewith { type: "json" }for import attributes; avoid deprecatedassertsyntax (TypeScript 5.3/5.8+)
Useimport typeexplicitly when importing types to ensure they are erased during compilation, respectingverbatimModuleSyntaxflag
Use.ts,.mts,.ctsextensions inimport typestatements (TypeScript 5.2+)
Useimport type { Type } from "mod" with { "resolution-mode": "import" }for specific module resolution contexts (TypeScript 5.3+)
Use new iterator methods (map, filter, etc.) if targeting modern environments (TypeScript 5.6+)
Utilize newSetmethods likeunion,intersection, etc., when available (TypeScript 5.5+)
UseObject.groupBy/Map.groupBystandard methods for grouping instead of external libraries (TypeScript 5.4+)
UsePromise.withResolvers()for creating promises with exposed resolve/reject functions (TypeScript 5.7+)
Use copying array methods (toSorted,toSpliced,with) for immutable array operations (TypeScript 5.2+)
Avoid accessing instance fields viasuperin classes (TypeScript 5....
Files:
packages/editor/src/core/contexts/index.tspackages/editor/src/core/types/hook.tsapps/live/src/extensions/title-update/title-utils.tspackages/editor/src/core/plugins/highlight.tspackages/editor/src/core/contexts/collaboration-context.tsxapps/live/src/extensions/title-update/title-update-manager.tsapps/web/core/components/pages/header/syncing-badge.tsxapps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/header.tsxapps/web/core/hooks/use-page-fallback.tspackages/editor/src/core/components/editors/document/collaborative-editor.tsxpackages/editor/src/core/hooks/use-editor-navigation.tsapps/web/core/store/pages/base-page.tspackages/editor/src/core/hooks/use-title-editor.tsapps/live/src/extensions/title-update/debounce.tspackages/editor/src/core/extensions/title-extension.tspackages/editor/src/core/components/editors/editor-content.tsxpackages/editor/src/core/helpers/yjs-utils.tsapps/web/core/components/pages/editor/content-limit-banner.tsxapps/live/src/extensions/title-sync.tspackages/editor/src/core/hooks/use-collaborative-editor.tspackages/editor/src/core/components/editors/editor-container.tsxapps/live/src/extensions/database.tspackages/editor/src/core/hooks/use-yjs-setup.tspackages/editor/src/core/types/editor.tspackages/editor/src/core/types/collaboration.tsapps/web/core/components/pages/editor/page-root.tsxapps/web/core/components/pages/editor/editor-body.tsxpackages/editor/src/core/components/editors/document/page-renderer.tsx
🧠 Learnings (7)
📚 Learning: 2025-10-09T20:42:31.843Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7922
File: apps/admin/app/(all)/(dashboard)/ai/form.tsx:19-19
Timestamp: 2025-10-09T20:42:31.843Z
Learning: In the makeplane/plane repository, React types are globally available through TypeScript configuration. Type annotations like React.FC, React.ReactNode, etc. can be used without explicitly importing the React namespace. The codebase uses the modern JSX transform, so React imports are not required for JSX or type references.
Applied to files:
apps/web/core/hooks/use-page-fallback.tspackages/editor/src/core/components/editors/document/collaborative-editor.tsxpackages/editor/src/core/components/editors/editor-content.tsxpackages/editor/src/core/components/editors/editor-container.tsxapps/web/core/components/pages/editor/page-root.tsxapps/web/core/components/pages/editor/editor-body.tsxpackages/editor/src/core/components/editors/document/page-renderer.tsx
📚 Learning: 2025-10-21T17:22:05.204Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7989
File: apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/[pageId]/page.tsx:45-46
Timestamp: 2025-10-21T17:22:05.204Z
Learning: In the makeplane/plane repository, the refactor from useParams() to params prop is specifically scoped to page.tsx and layout.tsx files in apps/web/app (Next.js App Router pattern). Other components (hooks, regular client components, utilities) should continue using the useParams() hook as that is the correct pattern for non-route components.
Applied to files:
apps/web/core/hooks/use-page-fallback.tspackages/editor/src/core/components/editors/document/collaborative-editor.tsxapps/web/core/components/pages/editor/page-root.tsxapps/web/core/components/pages/editor/editor-body.tsx
📚 Learning: 2025-10-01T15:30:17.605Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7888
File: packages/propel/src/avatar/avatar.stories.tsx:2-3
Timestamp: 2025-10-01T15:30:17.605Z
Learning: In the makeplane/plane repository, avoid suggesting inline type imports (e.g., `import { Avatar, type TAvatarSize }`) due to bundler compatibility issues. Keep type imports and value imports as separate statements.
Applied to files:
apps/web/core/hooks/use-page-fallback.tsapps/web/core/components/pages/editor/editor-body.tsx
📚 Learning: 2025-11-25T10:18:05.172Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-11-25T10:18:05.172Z
Learning: Applies to **/*.{ts,tsx,mts,cts} : Use `.ts`, `.mts`, `.cts` extensions in `import type` statements (TypeScript 5.2+)
Applied to files:
packages/editor/src/core/extensions/title-extension.ts
📚 Learning: 2025-11-25T10:18:05.172Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-11-25T10:18:05.172Z
Learning: Applies to **/*.{ts,tsx,mts,cts} : Use `with { type: "json" }` for import attributes; avoid deprecated `assert` syntax (TypeScript 5.3/5.8+)
Applied to files:
packages/editor/src/core/helpers/yjs-utils.ts
📚 Learning: 2025-06-04T16:22:44.344Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7164
File: packages/ui/.storybook/main.ts:24-47
Timestamp: 2025-06-04T16:22:44.344Z
Learning: In packages/ui/.storybook/main.ts, the webpackFinal function intentionally overrides the CSS loader strategy by finding and replacing existing CSS rules. This is a temporary workaround for a known upstream issue in Storybook's CSS handling that has been communicated to the Storybook maintainers. The current implementation should not be changed until the upstream issue is resolved.
Applied to files:
packages/editor/src/styles/index.css
📚 Learning: 2025-11-25T10:18:05.172Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-11-25T10:18:05.172Z
Learning: Applies to **/*.{js,mjs,cjs} : Use `import` tags in JSDoc for cleaner type imports in JavaScript files when working in a mixed codebase (TypeScript 5.5+)
Applied to files:
packages/editor/src/styles/index.css
🧬 Code graph analysis (16)
packages/editor/src/core/types/hook.ts (1)
packages/editor/src/core/types/editor.ts (1)
ICollaborativeDocumentEditorProps(187-205)
apps/live/src/extensions/title-update/title-utils.ts (1)
packages/editor/src/core/helpers/yjs-utils.ts (1)
extractTextFromHTML(208-212)
packages/editor/src/core/contexts/collaboration-context.tsx (1)
packages/editor/src/core/hooks/use-yjs-setup.ts (1)
useYjsSetup(30-369)
apps/live/src/extensions/title-update/title-update-manager.ts (4)
apps/live/src/types/index.ts (1)
HocusPocusServerContext(23-29)apps/live/src/extensions/title-update/debounce.ts (1)
DebounceManager(42-277)apps/live/src/services/page/handler.ts (1)
getPageService(6-16)apps/live/src/lib/errors.ts (1)
AppError(14-73)
apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/header.tsx (1)
apps/web/core/components/pages/header/syncing-badge.tsx (1)
PageSyncingBadge(9-72)
apps/web/core/hooks/use-page-fallback.ts (3)
packages/editor/src/core/types/collaboration.ts (1)
CollaborationState(29-33)packages/types/src/page/core.ts (1)
TDocumentPayload(66-70)packages/propel/src/toast/toast.tsx (1)
setToast(202-222)
packages/editor/src/core/components/editors/document/collaborative-editor.tsx (4)
packages/editor/src/core/types/editor.ts (1)
ICollaborativeDocumentEditorProps(187-205)packages/editor/src/core/contexts/collaboration-context.tsx (2)
useCollaboration(26-32)CollaborationProvider(14-24)packages/editor/src/core/hooks/use-collaborative-editor.ts (1)
useCollaborativeEditor(33-204)packages/editor/src/core/components/editors/document/page-renderer.tsx (1)
PageRenderer(41-114)
packages/editor/src/core/hooks/use-title-editor.ts (5)
packages/editor/src/core/types/editor.ts (2)
EditorTitleRefApi(146-146)ICollaborativeDocumentEditorProps(187-205)packages/editor/src/core/hooks/use-editor.ts (1)
useEditor(19-157)apps/live/src/services/page/core.service.ts (1)
updatePageProperties(55-104)packages/editor/src/core/extensions/title-extension.ts (1)
TitleExtensions(6-14)packages/editor/src/core/helpers/editor-ref.ts (1)
getEditorRefHelpers(24-282)
packages/editor/src/core/helpers/yjs-utils.ts (2)
packages/editor/src/core/extensions/title-extension.ts (1)
TitleExtensions(6-14)apps/live/src/extensions/title-update/title-utils.ts (1)
extractTextFromHTML(4-8)
apps/live/src/extensions/title-sync.ts (5)
apps/live/src/extensions/title-update/title-update-manager.ts (1)
TitleUpdateManager(11-90)apps/live/src/types/index.ts (2)
OnLoadDocumentPayloadWithContext(8-10)HocusPocusServerContext(23-29)apps/live/src/utils/document.ts (1)
generateTitleProsemirrorJson(1-21)packages/editor/src/core/types/document-collaborative-events.ts (1)
createRealtimeEvent(43-60)apps/live/src/utils/broadcast-message.ts (1)
broadcastMessageToPage(7-34)
packages/editor/src/core/components/editors/editor-container.tsx (2)
packages/editor/src/core/contexts/collaboration-context.tsx (1)
TCollabValue(5-5)packages/editor/src/core/plugins/highlight.ts (1)
nodeHighlightPluginKey(13-13)
apps/live/src/extensions/database.ts (1)
packages/editor/src/core/helpers/yjs-utils.ts (1)
getAllDocumentFormatsFromDocumentEditorBinaryData(120-157)
packages/editor/src/core/types/editor.ts (2)
packages/editor/src/ce/types/editor-extended.ts (1)
TExtendedEditorRefApi(11-11)packages/editor/src/core/types/document-collaborative-events.ts (1)
EventToPayloadMap(24-26)
apps/web/core/components/pages/editor/page-root.tsx (4)
apps/web/core/hooks/use-realtime-page-events.tsx (2)
TCustomEventHandlers(25-27)PageUpdateHandler(18-22)packages/editor/src/core/types/collaboration.ts (1)
CollaborationState(29-33)packages/editor/src/core/types/editor.ts (1)
EditorRefApi(144-144)apps/web/core/components/pages/editor/content-limit-banner.tsx (1)
ContentLimitBanner(9-35)
apps/web/core/components/pages/editor/editor-body.tsx (4)
apps/web/core/hooks/use-realtime-page-events.tsx (1)
TCustomEventHandlers(25-27)apps/web/ce/hooks/pages/use-extended-editor-extensions.ts (1)
TExtendedEditorExtensionsConfig(16-16)packages/editor/src/core/types/collaboration.ts (2)
CollaborationState(29-33)TServerHandler(35-37)packages/editor/src/core/types/editor.ts (1)
EditorTitleRefApi(146-146)
packages/editor/src/core/components/editors/document/page-renderer.tsx (7)
packages/editor/src/core/types/editor.ts (1)
IEditorProps(149-179)packages/editor/src/core/contexts/collaboration-context.tsx (1)
TCollabValue(5-5)packages/editor/src/core/components/editors/editor-container.tsx (1)
EditorContainer(29-179)packages/editor/src/core/components/editors/editor-content.tsx (1)
EditorContentWrapper(13-26)apps/api/plane/tests/unit/models/test_issue_comment_modal.py (1)
state(28-35)packages/editor/src/core/components/menus/bubble-menu/root.tsx (1)
EditorBubbleMenu(70-229)packages/editor/src/core/components/menus/block-menu.tsx (1)
BlockMenu(38-249)
🪛 Biome (2.1.2)
apps/live/src/extensions/title-sync.ts
[error] 45-45: Unsafe usage of optional chaining.
If it short-circuits with 'undefined' the evaluation will throw TypeError here:
(lint/correctness/noUnsafeOptionalChaining)
packages/editor/src/core/components/editors/document/page-renderer.tsx
[error] 56-56: Declarations inside of a const declaration may not have duplicates
a second declaration of flaggedExtensions is not allowed
flaggedExtensions is first declared here
(parse)
[error] 57-57: Declarations inside of a const declaration may not have duplicates
a second declaration of disabledExtensions is not allowed
disabledExtensions is first declared here
(parse)
🪛 GitHub Actions: Build and lint web apps
packages/editor/src/core/components/editors/document/page-renderer.tsx
[error] 21-21: Duplicate identifier 'disabledExtensions'. TS2300
⏰ 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: Analyze (javascript)
🔇 Additional comments (44)
packages/editor/package.json (1)
47-47: TipTap extension additions are correctly versioned and well-integrated.The three extensions (@tiptap/extension-document, @tiptap/extension-heading, @tiptap/extension-text) are consistently versioned at ^2.22.3, matching all other TipTap packages in the file. v2.22.3 is a stable patch release with full compatibility across the @tiptap ecosystem. Placement maintains alphabetical order and supports the PR's title synchronization feature.
packages/editor/src/core/components/editors/editor-content.tsx (1)
13-26: LGTM!Clean refactor to add
classNameprop support. The component signature change toFC<Props>and direct prop application to the wrapper div are appropriate for this extensibility enhancement.packages/editor/src/core/plugins/highlight.ts (1)
53-92: Plugin implementation is well-structured.The plugin correctly uses
PluginKeyfor state access, handles decoration rebuilding on node ID changes, and exposes decorations via theprops.decorationsgetter. The use of early return inbuildDecorationsand stopping traversal after finding the target node is efficient.packages/editor/src/core/types/editor.ts (2)
102-146: LGTM!The type refactoring cleanly separates core API (
CoreEditorRefApi) from extended API, enabling composition via intersection type. TheEditorTitleRefApialias provides semantic clarity for title-editor contexts.
196-204: LGTM!The new optional props for
ICollaborativeDocumentEditorPropsare well-typed. The generic constraint onupdatePagePropertiesproperly restrictsactionTypeto valid event keys while inferring the correct payload type.packages/editor/src/styles/index.css (1)
6-6: LGTM!Import follows the established pattern for CSS modules in this file. The referenced
title-editor.cssfile exists at the expected location.apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/header.tsx (1)
13-13: LGTM! Clean integration of syncing status badge.The import and usage of
PageSyncingBadgeare correct. The component is properly placed in the header's right section and receives the syncing status from the page state.Also applies to: 99-99
packages/editor/src/core/contexts/index.ts (1)
1-1: LGTM! Standard barrel export pattern.This centralized export enables cleaner imports from the contexts module.
packages/editor/src/core/types/hook.ts (1)
60-63: LGTM! Clean type extension.The addition of
titleRefandupdatePagePropertiesas optional properties correctly extends the collaborative editor hook props to support the new title editing and page property update workflows.apps/web/core/components/pages/editor/content-limit-banner.tsx (1)
1-35: LGTM! Well-structured banner component.The component is cleanly implemented with:
- Proper semantic HTML structure
- Accessibility label on the dismiss button
- Optional dismiss functionality via callback prop
- Appropriate styling with Tailwind utilities
Based on learnings, React.FC usage is correct as React types are globally available in this codebase.
apps/web/core/components/pages/header/syncing-badge.tsx (1)
9-72: LGTM! Solid implementation of sync status indicator.The component correctly handles three sync states with smooth transitions:
- Uses state and refs to track visibility and previous status
- Implements a 300ms delay for exit animations
- Early returns prevent rendering when not needed
- Provides helpful tooltips for each status
- Includes visual indicators (pulsing animation for syncing, icon for error)
The logic is well-structured and defensive.
apps/web/core/hooks/use-page-fallback.ts (1)
34-71: Excellent improvements to error handling and user feedback.The additions significantly enhance the user experience:
- Toast notifications inform users when fallback mechanism activates
- Loading state (
isFetchingFallbackBinary) enables UI feedback during fallback operations- Single-toast guarantee via
hasShownFallbackToastref prevents notification spam- Proper error handling with user-facing error messages
- Toast reset on reconnection allows proper notification on subsequent disconnections
apps/web/core/store/pages/base-page.ts (1)
16-16: LGTM!The
isSyncingWithServerobservable is properly typed as a union of string literals and correctly initialized. Theobservable.refannotation is appropriate for a primitive value that doesn't need deep observation.Also applies to: 78-78
packages/editor/src/core/contexts/collaboration-context.tsx (2)
14-24: LGTM!The
CollaborationProvidercorrectly gates content rendering untiluseYjsSetupreturns a non-null value, allowing consumers to checkstate.isDocReadyfor finer-grained control. The pattern of showing a fallback while setup is initializing is appropriate.
26-32: LGTM!The
useCollaborationhook properly enforces context usage with a clear error message, and the return type is correctly inferred as non-null.packages/editor/src/core/types/collaboration.ts (2)
1-5: LGTM!Well-structured discriminated union for collaboration errors with clear type discrimination. The variant payloads appropriately capture the relevant context for each error type.
17-23: LGTM!The
CollabStagediscriminated union withkinddiscriminator is well-designed. The JSDoc clearly documents the state machine invariants and stage transitions.packages/editor/src/core/hooks/use-title-editor.ts (1)
56-58: Verify dependency array completeness.The
useEditordependency array includesdocKeywhich is derived fromprovider?.document?.guid. Ifprovideror its document changes without changing the guid, the editor won't recreate. Ensure this is the intended behavior.packages/editor/src/core/components/editors/editor-container.tsx (1)
34-73: LGTM!The
scrollToNodeimplementation correctly traverses the document to find a node by ID, applies a highlight via the plugin, scrolls it into view, and clears the highlight on focus. UsingrequestAnimationFrameensures the DOM is ready before scrolling.packages/editor/src/core/components/editors/document/page-renderer.tsx (1)
70-110: LGTM on the rendering structure.The conditional title editor rendering and main editor flow are well-structured. The provider and state are properly propagated to
EditorContainer, and menus are correctly gated behind editability and touch device checks.packages/editor/src/core/components/editors/document/collaborative-editor.tsx (2)
133-149: Verify theauthTokenformat.The
authTokenis set toJSON.stringify(user), which serializes the entire user object. Ensure the Hocuspocus server expects this format. If only a specific token field is needed, consider passinguser.tokenor similar instead.
151-155: LGTM on theforwardRefpattern.Using
key={props.id}ensures the editor remounts when the document ID changes, which is appropriate for collaborative editing scenarios.packages/editor/src/core/hooks/use-editor-navigation.ts (2)
18-26:ArrowDownhandler behavior doesn't match documentation.The comment says "Arrow down at end of title - Move to main editor" but the implementation always moves to the main editor regardless of cursor position. This may be intentional for UX, but if the goal is to only navigate at the end, add a position check similar to
ArrowRight.If this is intentional behavior, update the comment:
- // Arrow down at end of title - Move to main editor + // Arrow down anywhere in title - Move to main editor ArrowDown: () => {
44-52: VerifyinsertContentAt(0)behavior.Position
0in ProseMirror is before the document's root node. Inserting at position0may not produce the expected result. Typically, position1is the start of the first block content.Consider using position
1:- mainEditor.chain().focus().insertContentAt(0, { type: "paragraph" }).run(); + mainEditor.chain().focus().insertContentAt(1, { type: "paragraph" }).run();apps/live/src/extensions/title-update/title-update-manager.ts (2)
11-29: LGTM on class structure and constructor.The constructor properly initializes the debounce manager with a reasonable 5-second default wait time and includes a log prefix for debugging.
62-67: Verify silent error handling is intentional.Errors during title update are logged but not propagated. This means failures are silent to callers. If title persistence is critical, consider exposing error state or implementing retry logic in the debounce manager.
apps/live/src/extensions/title-sync.ts (2)
138-146: LGTM!The
beforeUnloadDocumentmethod correctly forces an immediate save and cleans up the manager before document unload.
151-175: LGTM!The cleanup in
afterUnloadDocumentis thorough—it unregisters observers, cleans up observer data, and ensures the update manager is cancelled ifbeforeUnloadDocumentdidn't run.packages/editor/src/core/hooks/use-yjs-setup.ts (3)
265-287: LGTM!The IndexedDB persistence lifecycle is properly managed with cleanup on unmount.
321-338: LGTM!Good use of the ref pattern to avoid including the callback in dependencies while keeping it up-to-date.
349-368: LGTM!The null check before returning ensures consumers receive a valid session or null, making the API predictable.
apps/web/core/components/pages/editor/page-root.tsx (2)
104-120: LGTM!The error handler correctly checks for the
content_too_largeerror code and preserves the chain by calling the original error handler if it exists.
46-56: LGTM!The component destructuring and props handling is clean. Good use of the observer HOC with an arrow function.
apps/live/src/extensions/title-update/debounce.ts (3)
270-276:arraysEqualuses reference equality.This works for primitive values (strings, numbers) but will return
falsefor objects even if they're structurally identical. Given this is used to compare title strings, it's fine for the current use case.
62-95: LGTM!The
schedulemethod correctly handles the three cases: operation in progress, timer already scheduled, and fresh schedule. Timer management is clean.
227-241: LGTM!The
flushmethod properly clears pending timeouts, resets timing state, and executes immediately if there are pending args.apps/web/core/components/pages/editor/editor-body.tsx (3)
157-176: LGTM!The serverHandler correctly maps collaboration stages to UI syncing status with clear logic: disconnected → error, synced → synced, all others → syncing.
220-222: LGTM!Good loading guard that shows the loader when pageId is undefined or realtimeConfig couldn't be constructed.
52-69:customRealtimeEventHandlersprop may be unused in the component.The
customRealtimeEventHandlersprop is declared in the Props type but requires verification that it is not destructured or passed to child components elsewhere in the implementation.packages/editor/src/core/hooks/use-collaborative-editor.ts (4)
33-62: LGTM on the restructured hook signature.The refactor to accept an external
providerand typed configuration improves separation of concerns. The destructuring is clean and comprehensive.
66-102: Extensions memoization looks correct.The extension array is properly constructed with all necessary extensions including the Collaboration extension configured with the provider's document. The dependency array captures all relevant inputs.
193-198: Editor wiring effect is appropriate.The effect correctly synchronizes the main and title editors with the navigation context when both are available. The conditional guard ensures both editors exist before setting.
28-31: Theactionsparameter is not unused—it's part of the public API contract.The
signalForcedClosecallback is actively used inuse-yjs-setup.ts(line 366) and is expected to be provided by callers of this hook. While the parameter isn't directly used withinuse-collaborative-editor.tsitself, this is an intentional design where the hook accepts it as part of its configuration. This is not dead code.Likely an incorrect or invalid review comment.
apps/live/src/extensions/database.ts (1)
30-40: Race condition possible when multiple concurrent fetch requests trigger simultaneous writes.The code converts HTML to binary formats and persists them during the fetch operation when
binaryData.byteLength === 0. However, multiple concurrent fetch requests for the same document would each see the empty binary condition independently and trigger simultaneousupdateDescriptionBinarycalls without any client-side deduplication or locking.While the conversion is deterministic (same input yields same output), this write-during-read pattern creates a potential race condition. Address this by either:
- Adding client-side deduplication (e.g., tracking in-flight conversions per pageId)
- Verifying the backend PATCH operation is idempotent and handles concurrent writes safely
- Moving the one-time conversion to an initialization/migration step outside the fetch path
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/editor/src/core/components/editors/document/page-renderer.tsx (1)
18-36: Remove unusedaiHandlerprop.The
aiHandlerprop is defined in the Props interface but is never destructured or used within the component. Based on the AI summary indicating replacement of aiHandler-based rendering logic, this appears to be leftover from the refactoring.Apply this diff:
type Props = { - aiHandler?: TAIHandler; bubbleMenuEnabled: boolean; disabledExtensions: IEditorProps["disabledExtensions"];Also remove the
TAIHandlerimport from line 14 if it's no longer used elsewhere:IEditorProps, IEditorPropsExtended, - TAIHandler, TDisplayConfig,
♻️ Duplicate comments (1)
packages/editor/src/core/hooks/use-title-editor.ts (1)
40-44: Stale closure:editorisundefinedwhenonUpdateis first called.The
editorvariable is referenced inside theonUpdatecallback, but it captures the value beforeuseEditorreturns. Use the callback parameter instead:const editor = useEditor( { - onUpdate: () => { - updatePageProperties?.(id, "property_updated", { name: editor?.getText() }); + onUpdate: ({ editor: currentEditor }) => { + updatePageProperties?.(id, "property_updated", { name: currentEditor.getText() }); },
🧹 Nitpick comments (2)
packages/editor/src/core/hooks/use-collaborative-editor.ts (2)
105-156: Dependency array issues ineditorConfigmemoization.Two issues:
providerappears twice in the dependency array (lines 132 and 153)showPlaceholderOnEmptyis in the dependency array (line 152) but not included in the memoized config object() => ({ disabledExtensions, extendedEditorProps, id, editable, editorProps, editorClassName, enableHistory: false, extensions: editorExtensions, fileHandler, flaggedExtensions, forwardedRef, getEditorMetaData, handleEditorReady, isTouchDevice, mentionHandler, onAssetChange, onChange, onEditorFocus, onTransaction, placeholder, provider, + showPlaceholderOnEmpty, tabIndex, }), [ provider, disabledExtensions, extendedEditorProps, id, editable, editorProps, editorClassName, editorExtensions, fileHandler, flaggedExtensions, forwardedRef, getEditorMetaData, handleEditorReady, isTouchDevice, mentionHandler, onAssetChange, onChange, onEditorFocus, onTransaction, placeholder, showPlaceholderOnEmpty, - provider, tabIndex, ]
171-194: Type assertion suggests type mismatch betweentitleEditorConfiganduseTitleEditorprops.The explicit type annotation on
titleEditorConfigfollowed byas Parameters<typeof useTitleEditor>[0]indicates the types don't align. Consider aligning the inline type with thePropstype fromuseTitleEditoror importing and reusing that type directly to avoid the cast.+import type { Props as TitleEditorProps } from "./use-title-editor"; +// Or export the Props type from use-title-editor.ts - const titleEditorConfig = useMemo<{ - id: string; - editable: boolean; - provider: HocuspocusProvider; - titleRef?: React.MutableRefObject<EditorTitleRefApi | null>; - updatePageProperties?: ICollaborativeDocumentEditorProps["updatePageProperties"]; - extensions: Extensions; - extendedEditorProps?: IEditorPropsExtended; - getEditorMetaData?: IEditorProps["getEditorMetaData"]; - }>( + const titleEditorConfig = useMemo<Parameters<typeof useTitleEditor>[0]>( () => ({ ... }), [...] ); - const titleEditor = useTitleEditor(titleEditorConfig as Parameters<typeof useTitleEditor>[0]); + const titleEditor = useTitleEditor(titleEditorConfig);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/editor/src/core/components/editors/document/page-renderer.tsx(5 hunks)packages/editor/src/core/hooks/use-collaborative-editor.ts(3 hunks)packages/editor/src/core/hooks/use-title-editor.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,mts,cts}
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/*.{ts,tsx,mts,cts}: Useconsttype parameters for more precise literal inference in TypeScript 5.0+
Use thesatisfiesoperator to validate types without widening them
Leverage inferred type predicates to reduce the need for explicitisreturn types in filter/check functions
UseNoInfer<T>utility to block inference for specific type arguments when they should be determined by other arguments
Utilize narrowing inswitch(true)blocks for control flow analysis (TypeScript 5.3+)
Rely on narrowing from direct boolean comparisons for type guards
Trust preserved narrowing in closures when variables aren't modified after the check (TypeScript 5.4+)
Use constant indices to narrow object/array properties (TypeScript 5.5+)
Use standard ECMAScript decorators (Stage 3) instead of legacyexperimentalDecorators
Useusingdeclarations for explicit resource management with Disposable pattern instead of manual cleanup (TypeScript 5.2+)
Usewith { type: "json" }for import attributes; avoid deprecatedassertsyntax (TypeScript 5.3/5.8+)
Useimport typeexplicitly when importing types to ensure they are erased during compilation, respectingverbatimModuleSyntaxflag
Use.ts,.mts,.ctsextensions inimport typestatements (TypeScript 5.2+)
Useimport type { Type } from "mod" with { "resolution-mode": "import" }for specific module resolution contexts (TypeScript 5.3+)
Use new iterator methods (map, filter, etc.) if targeting modern environments (TypeScript 5.6+)
Utilize newSetmethods likeunion,intersection, etc., when available (TypeScript 5.5+)
UseObject.groupBy/Map.groupBystandard methods for grouping instead of external libraries (TypeScript 5.4+)
UsePromise.withResolvers()for creating promises with exposed resolve/reject functions (TypeScript 5.7+)
Use copying array methods (toSorted,toSpliced,with) for immutable array operations (TypeScript 5.2+)
Avoid accessing instance fields viasuperin classes (TypeScript 5....
Files:
packages/editor/src/core/hooks/use-collaborative-editor.tspackages/editor/src/core/hooks/use-title-editor.tspackages/editor/src/core/components/editors/document/page-renderer.tsx
🧠 Learnings (8)
📚 Learning: 2025-11-25T10:18:05.172Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-11-25T10:18:05.172Z
Learning: Applies to **/*.{ts,tsx,mts,cts} : Ensure variables are initialized before use to avoid errors with never-initialized variables (TypeScript 5.7+)
Applied to files:
packages/editor/src/core/components/editors/document/page-renderer.tsx
📚 Learning: 2025-11-25T10:18:05.172Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-11-25T10:18:05.172Z
Learning: Applies to **/*.{ts,tsx,mts,cts} : Treat legacy `experimentalDecorators`-style behavior as deprecated in favor of standard TC39-compliant decorators
Applied to files:
packages/editor/src/core/components/editors/document/page-renderer.tsx
📚 Learning: 2025-11-25T10:18:05.172Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-11-25T10:18:05.172Z
Learning: Applies to **/*.{ts,tsx,mts,cts} : Avoid `enum`s if the project prefers erasable syntax, using `const` objects or unions instead (TypeScript 5.8+)
Applied to files:
packages/editor/src/core/components/editors/document/page-renderer.tsx
📚 Learning: 2025-10-01T15:30:17.605Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7888
File: packages/propel/src/avatar/avatar.stories.tsx:2-3
Timestamp: 2025-10-01T15:30:17.605Z
Learning: In the makeplane/plane repository, avoid suggesting inline type imports (e.g., `import { Avatar, type TAvatarSize }`) due to bundler compatibility issues. Keep type imports and value imports as separate statements.
Applied to files:
packages/editor/src/core/components/editors/document/page-renderer.tsx
📚 Learning: 2025-11-25T10:18:05.172Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-11-25T10:18:05.172Z
Learning: Applies to **/*.{ts,tsx,mts,cts} : Use copying array methods (`toSorted`, `toSpliced`, `with`) for immutable array operations (TypeScript 5.2+)
Applied to files:
packages/editor/src/core/components/editors/document/page-renderer.tsx
📚 Learning: 2025-11-25T10:18:05.172Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-11-25T10:18:05.172Z
Learning: Applies to **/*.{ts,tsx,mts,cts} : Use constant indices to narrow object/array properties (TypeScript 5.5+)
Applied to files:
packages/editor/src/core/components/editors/document/page-renderer.tsx
📚 Learning: 2025-11-25T10:18:05.172Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-11-25T10:18:05.172Z
Learning: Applies to **/*.{ts,tsx,mts,cts} : Use `using` declarations for explicit resource management with Disposable pattern instead of manual cleanup (TypeScript 5.2+)
Applied to files:
packages/editor/src/core/components/editors/document/page-renderer.tsx
📚 Learning: 2025-10-09T20:42:31.843Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7922
File: apps/admin/app/(all)/(dashboard)/ai/form.tsx:19-19
Timestamp: 2025-10-09T20:42:31.843Z
Learning: In the makeplane/plane repository, React types are globally available through TypeScript configuration. Type annotations like React.FC, React.ReactNode, etc. can be used without explicitly importing the React namespace. The codebase uses the modern JSX transform, so React imports are not required for JSX or type references.
Applied to files:
packages/editor/src/core/components/editors/document/page-renderer.tsx
🧬 Code graph analysis (2)
packages/editor/src/core/hooks/use-collaborative-editor.ts (5)
packages/editor/src/core/types/hook.ts (2)
TCollaborativeEditorHookProps(42-63)TEditorHookProps(20-40)packages/editor/src/core/hooks/use-editor-navigation.ts (1)
useEditorNavigation(141-169)packages/editor/src/core/hooks/use-editor.ts (1)
useEditor(19-157)packages/editor/src/core/types/editor.ts (3)
EditorTitleRefApi(146-146)ICollaborativeDocumentEditorProps(187-205)IEditorProps(149-179)packages/editor/src/core/hooks/use-title-editor.ts (1)
useTitleEditor(34-82)
packages/editor/src/core/components/editors/document/page-renderer.tsx (5)
packages/editor/src/core/contexts/collaboration-context.tsx (1)
TCollabValue(5-5)packages/editor/src/core/components/editors/editor-container.tsx (1)
EditorContainer(29-179)packages/editor/src/core/components/editors/editor-content.tsx (1)
EditorContentWrapper(13-26)packages/editor/src/core/components/menus/bubble-menu/root.tsx (1)
EditorBubbleMenu(70-229)packages/editor/src/core/components/menus/block-menu.tsx (1)
BlockMenu(38-249)
🔇 Additional comments (5)
packages/editor/src/core/hooks/use-title-editor.ts (1)
62-79: LGTM on the imperative handle implementation.The
getEditorMetaDataparameter is now correctly provided with a sensible fallback. TheclearEditorandsetEditorValueimplementations properly handle the editor operations.packages/editor/src/core/hooks/use-collaborative-editor.ts (1)
196-206: LGTM on editor wiring and return shape.The effect correctly synchronizes both editors with the navigation refs when available, and the return shape properly exposes both
editorandtitleEditor.packages/editor/src/core/components/editors/document/page-renderer.tsx (3)
1-16: LGTM!The import statements correctly use
import typefor type-only imports and maintain separate statements for types and values, consistent with the project's bundler compatibility requirements.
66-83: Verify collaboration support for the title editor.The main editor receives
providerandstateprops for real-time synchronization (lines 90-91), but the title editor does not. Given that this PR focuses on realtime sync refactoring and the AI summary mentions title synchronization, please confirm whether the title editor should also receive these props for collaborative editing support.If the title editor needs collaboration support, apply this diff:
<EditorContainer editor={titleEditor} id={id + "-title"} isTouchDevice={isTouchDevice} editorContainerClassName="page-title-editor bg-transparent py-3 border-none" displayConfig={displayConfig} + provider={provider} + state={state} >
94-109: > Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
packages/editor/src/core/hooks/use-collaborative-editor.ts (1)
26-32:actionsprop is still unused in the hook body.
actions(withsignalForcedClose) is part ofUseCollaborativeEditorArgsand is destructured but never referenced. Either remove it from the type/destructuring or actually wire it into the hook where a forced-close signal should be emitted/handled (e.g., on specific collaboration state changes or cleanup).This was also flagged in a previous review and appears still unresolved.
Also applies to: 61-63
apps/live/src/extensions/title-sync.ts (2)
43-47: Fix unsafe optional chaining aroundfetchDetailsand.nameaccess
(await service.fetchDetails?.(documentName)).namewill throw whenfetchDetailsis undefined: the optional call returnsundefined,awaityieldsundefined, and accessing.namethen raises aTypeError. This is also what Biome is flagging.Split the call and guard the property access:
- // const title = await service.fe - const title = (await service.fetchDetails?.(documentName)).name; - if (title == null) return; + const details = await service.fetchDetails?.(documentName); + const title = details?.name; + if (title == null) return;
25-33:parentIdis never populated, breaking title-change broadcasts
titleObserverDataincludes an optionalparentId, butafterLoadDocumentonly storesuserId,workspaceSlug, andinstance. SinceparentIdnever exists,data.parentIdis alwaysundefined, the guard at line 114 never satisfies, and the broadcast call at line 125 would fail even if the guard passed (the function expectsdocumentNameas the second parameter, notparentId).Remove
parentIdfrom the type and fix the broadcast call to usedocumentName:- private titleObserverData: Map< - string, - { - parentId?: string | null; - userId: string; - workspaceSlug: string | null; - instance: Hocuspocus; - } - > = new Map(); + private titleObserverData: Map< + string, + { + userId: string; + workspaceSlug: string | null; + instance: Hocuspocus; + } + > = new Map(); @@ - // Broadcast to parent page if it exists - if (data?.parentId && data.workspaceSlug && data.instance) { + // Broadcast to page if it exists + if (data?.workspaceSlug && data.instance) { @@ - broadcastMessageToPage(data.instance, data.parentId, event); + broadcastMessageToPage(data.instance, documentName, event);
🧹 Nitpick comments (12)
apps/web/core/hooks/use-page-fallback.ts (3)
33-42: Clean up commented-out toast notifications.The commented-out toast calls with
console.logplaceholders leave the notification UX incomplete. Consider either uncommenting and activating the toasts, or removing the commented code if this is intentional temporary scaffolding.Also applies to: 67-71
65-65: Useunknowninstead ofanyfor error type.Modern TypeScript best practices prefer
unknownfor caught errors to enforce explicit type checking before usage.Apply this diff:
- } catch (error: any) { + } catch (error: unknown) { console.error(error); // setToast({ // type: TOAST_TYPE.ERROR, // title: "Error", - // message: `Failed to update description using backup mechanism, ${error?.message}`, + // message: `Failed to update description using backup mechanism, ${error instanceof Error ? error.message : String(error)}`, // });
28-44: Consider guarding against concurrent fallback fetches.Although the
isFetchingFallbackBinaryflag tracks fetch status, there's no guard preventing concurrent executions ofhandleUpdateDescription. Since bothuseAutoSave(line 86) and theuseEffect(line 78) can trigger this callback, overlapping calls could cause redundant fetches when disconnected.Add an early return guard:
const handleUpdateDescription = useCallback(async () => { if (!hasConnectionFailed) return; + if (isFetchingFallbackBinary) return; const editor = editorRef.current; if (!editor) return;And update the dependency array to include
isFetchingFallbackBinary:-}, [editorRef, fetchPageDescription, hasConnectionFailed, updatePageDescription]); +}, [editorRef, fetchPageDescription, hasConnectionFailed, updatePageDescription, isFetchingFallbackBinary]);apps/web/core/components/pages/editor/editor-body.tsx (4)
65-69: Avoid exposing an unusedcustomRealtimeEventHandlersprop
customRealtimeEventHandlersis added toPropsbut never read or forwarded insidePageEditorBody, so passing it from callers currently has no effect. Either wire it intouseRealtimePageEvents/ the editor as intended, or drop it fromPropsfor now to keep the public surface aligned with actual behavior.Also applies to: 85-87
132-138: Realtime page events integration is correct; consider handler stabilityHooking
useRealtimePageEventsand passingupdatePagePropertiesintoCollaborativeDocumentEditorWithRefis a clean separation of concerns. Ifhandlersis not memoized in the parent, you may want to stabilize it (e.g., viauseMemo) to avoid unnecessary resubscriptions or effect churn insideuseRealtimePageEvents, but this is an optimization, not a blocker.Also applies to: 280-280
99-99: Sync reset on page change is reasonable; confirm handler-change semanticsResetting
setSyncingStatus("syncing")and emitting a"connecting"collaboration state whenpageIdchanges makes sense to model a fresh load. This will also fire ifonCollaborationStateChange’s identity changes; if you ever pass a non‑stable callback from the parent and don’t want that to trigger a reset, consider wrapping the callback in a ref inside this component instead of keeping it in the dependency array.Also applies to: 140-148
184-208: Consider distinguishing “real error” from “still loading” for realtimeConfigIf
realtimeConfigfails to construct (e.g., badLIVE_BASE_URLor malformedwebhookConnectionParams), the catch block logs an error butisPageLoadingstays true forever, so the user just sees a loader. It may be worth introducing a separate “fatal error” state (or exposing the failure upward) so configuration issues can surface as an explicit error instead of an infinite spinner.Also applies to: 226-228
packages/editor/src/core/hooks/use-title-editor.ts (1)
16-28: Wire theplaceholderprop into the Placeholder extension (and consider deps foronUpdate).
Propsexposes aplaceholder?: string, but the hook always renders"Untitled"viaPlaceholder.configure, so callers can’t customize the title placeholder. You can keep the current default while honoring the prop:- const { - editable = true, - id, - initialValue = "", - extensions, - provider, - updatePageProperties, - titleRef, - getEditorMetaData, - } = props; + const { + editable = true, + id, + initialValue = "", + extensions, + provider, + updatePageProperties, + titleRef, + getEditorMetaData, + placeholder, + } = props; @@ - Placeholder.configure({ - placeholder: () => "Untitled", + Placeholder.configure({ + placeholder: () => placeholder ?? "Untitled", includeChildren: true, showOnlyWhenEditable: false, }),If
idorupdatePagePropertiesare ever non-stable, also consider adding them to theuseEditordeps or memoizing them upstream to avoid a staleonUpdateclosure.Also applies to: 35-44, 49-67
packages/editor/src/core/components/editors/document/collaborative-editor.tsx (1)
92-101: Avoid returningnullwhile editors initialize; consider showing a loader or skeleton instead.
if (!editor || !titleEditor) return null;results in an empty render while the editors are being created, even though you already haveisLoadingandstate.isDocReadyto drive loading UI viaPageRendererand CSS opacity. Consider rendering the same container and passingisLoading/state down (or a lightweight fallback) instead of returningnull, so users always see a loader/skeleton rather than a blank area during initialization.packages/editor/src/core/hooks/use-collaborative-editor.ts (1)
160-195: SimplifytitleEditorConfigtyping to avoid the extraascast.The inline object type for
titleEditorConfigfollowed byas Parameters<typeof useTitleEditor>[0]is a bit clunky. You can let TypeScript infer the shape directly fromuseTitleEditor’s parameter type:- const titleEditorConfig = useMemo<{ - id: string; - editable: boolean; - provider: HocuspocusProvider; - titleRef?: React.MutableRefObject<EditorTitleRefApi | null>; - updatePageProperties?: ICollaborativeDocumentEditorProps["updatePageProperties"]; - extensions: Extensions; - extendedEditorProps?: IEditorPropsExtended; - getEditorMetaData?: IEditorProps["getEditorMetaData"]; - }>( - () => ({ + const titleEditorConfig = useMemo<Parameters<typeof useTitleEditor>[0]>( + () => ({ id, editable, provider, titleRef, updatePageProperties, extensions: titleExtensions, extendedEditorProps, getEditorMetaData, }), - [provider, id, editable, titleRef, updatePageProperties, titleExtensions, extendedEditorProps, getEditorMetaData] + [provider, id, editable, titleRef, updatePageProperties, titleExtensions, extendedEditorProps, getEditorMetaData] ); - const titleEditor = useTitleEditor(titleEditorConfig as Parameters<typeof useTitleEditor>[0]); + const titleEditor = useTitleEditor(titleEditorConfig);This keeps the config type-aligned with the hook’s signature and avoids redundant casting.
apps/live/src/extensions/title-sync.ts (2)
101-132: Title extraction and update scheduling look good; optionally simplify event handlingThe extraction + scheduling path is clear and correctly uses the per‑document manager. If you only care about the latest event, you could avoid the loop and read
events[events.length - 1]directly, but the current implementation is functionally fine.
137-145: Consider wrappingforceSave()in try/catch to avoid disrupting unload
beforeUnloadDocumentawaitsupdateManager.forceSave()without a try/catch, so a rejected save could bubble out of the hook and interfere with the unload lifecycle. You already defensively clean up managers inafterUnloadDocument; mirroring your error‑handling pattern fromonLoadDocumentwould make this more robust:- const updateManager = this.titleUpdateManagers.get(documentName); - if (updateManager) { - // Force immediate save and wait for it to complete - await updateManager.forceSave(); - // Clean up the manager - this.titleUpdateManagers.delete(documentName); - } + const updateManager = this.titleUpdateManagers.get(documentName); + if (updateManager) { + try { + // Force immediate save and wait for it to complete + await updateManager.forceSave(); + } catch (error) { + logger.error( + "Error force-saving document title before unload", + new AppError(error, { context: { operation: "beforeUnloadDocument", documentName } }) + ); + } finally { + // Clean up the manager regardless of outcome + this.titleUpdateManagers.delete(documentName); + } + }Also applies to: 168-173
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/live/src/extensions/title-sync.ts(1 hunks)apps/web/core/components/pages/editor/editor-body.tsx(10 hunks)apps/web/core/hooks/use-page-fallback.ts(3 hunks)packages/editor/src/core/components/editors/document/collaborative-editor.tsx(4 hunks)packages/editor/src/core/hooks/use-collaborative-editor.ts(3 hunks)packages/editor/src/core/hooks/use-title-editor.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,mts,cts}
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/*.{ts,tsx,mts,cts}: Useconsttype parameters for more precise literal inference in TypeScript 5.0+
Use thesatisfiesoperator to validate types without widening them
Leverage inferred type predicates to reduce the need for explicitisreturn types in filter/check functions
UseNoInfer<T>utility to block inference for specific type arguments when they should be determined by other arguments
Utilize narrowing inswitch(true)blocks for control flow analysis (TypeScript 5.3+)
Rely on narrowing from direct boolean comparisons for type guards
Trust preserved narrowing in closures when variables aren't modified after the check (TypeScript 5.4+)
Use constant indices to narrow object/array properties (TypeScript 5.5+)
Use standard ECMAScript decorators (Stage 3) instead of legacyexperimentalDecorators
Useusingdeclarations for explicit resource management with Disposable pattern instead of manual cleanup (TypeScript 5.2+)
Usewith { type: "json" }for import attributes; avoid deprecatedassertsyntax (TypeScript 5.3/5.8+)
Useimport typeexplicitly when importing types to ensure they are erased during compilation, respectingverbatimModuleSyntaxflag
Use.ts,.mts,.ctsextensions inimport typestatements (TypeScript 5.2+)
Useimport type { Type } from "mod" with { "resolution-mode": "import" }for specific module resolution contexts (TypeScript 5.3+)
Use new iterator methods (map, filter, etc.) if targeting modern environments (TypeScript 5.6+)
Utilize newSetmethods likeunion,intersection, etc., when available (TypeScript 5.5+)
UseObject.groupBy/Map.groupBystandard methods for grouping instead of external libraries (TypeScript 5.4+)
UsePromise.withResolvers()for creating promises with exposed resolve/reject functions (TypeScript 5.7+)
Use copying array methods (toSorted,toSpliced,with) for immutable array operations (TypeScript 5.2+)
Avoid accessing instance fields viasuperin classes (TypeScript 5....
Files:
packages/editor/src/core/hooks/use-title-editor.tsapps/live/src/extensions/title-sync.tspackages/editor/src/core/components/editors/document/collaborative-editor.tsxpackages/editor/src/core/hooks/use-collaborative-editor.tsapps/web/core/components/pages/editor/editor-body.tsxapps/web/core/hooks/use-page-fallback.ts
🧠 Learnings (4)
📚 Learning: 2025-10-09T20:42:31.843Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7922
File: apps/admin/app/(all)/(dashboard)/ai/form.tsx:19-19
Timestamp: 2025-10-09T20:42:31.843Z
Learning: In the makeplane/plane repository, React types are globally available through TypeScript configuration. Type annotations like React.FC, React.ReactNode, etc. can be used without explicitly importing the React namespace. The codebase uses the modern JSX transform, so React imports are not required for JSX or type references.
Applied to files:
packages/editor/src/core/components/editors/document/collaborative-editor.tsxapps/web/core/components/pages/editor/editor-body.tsxapps/web/core/hooks/use-page-fallback.ts
📚 Learning: 2025-10-21T17:22:05.204Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7989
File: apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/[pageId]/page.tsx:45-46
Timestamp: 2025-10-21T17:22:05.204Z
Learning: In the makeplane/plane repository, the refactor from useParams() to params prop is specifically scoped to page.tsx and layout.tsx files in apps/web/app (Next.js App Router pattern). Other components (hooks, regular client components, utilities) should continue using the useParams() hook as that is the correct pattern for non-route components.
Applied to files:
packages/editor/src/core/components/editors/document/collaborative-editor.tsxapps/web/core/components/pages/editor/editor-body.tsxapps/web/core/hooks/use-page-fallback.ts
📚 Learning: 2025-10-01T15:30:17.605Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7888
File: packages/propel/src/avatar/avatar.stories.tsx:2-3
Timestamp: 2025-10-01T15:30:17.605Z
Learning: In the makeplane/plane repository, avoid suggesting inline type imports (e.g., `import { Avatar, type TAvatarSize }`) due to bundler compatibility issues. Keep type imports and value imports as separate statements.
Applied to files:
apps/web/core/components/pages/editor/editor-body.tsxapps/web/core/hooks/use-page-fallback.ts
📚 Learning: 2025-05-14T13:16:23.323Z
Learnt from: vamsikrishnamathala
Repo: makeplane/plane PR: 7061
File: web/core/components/workspace-notifications/root.tsx:18-18
Timestamp: 2025-05-14T13:16:23.323Z
Learning: In the Plane project codebase, the path alias `@/plane-web` resolves to the `ce` directory, making imports like `@/plane-web/hooks/use-notification-preview` correctly resolve to files in `web/ce/hooks/`.
Applied to files:
apps/web/core/components/pages/editor/editor-body.tsx
🧬 Code graph analysis (3)
packages/editor/src/core/hooks/use-title-editor.ts (3)
packages/editor/src/core/types/editor.ts (3)
EditorTitleRefApi(146-146)ICollaborativeDocumentEditorProps(187-205)IEditorProps(149-179)packages/editor/src/core/extensions/title-extension.ts (1)
TitleExtensions(6-14)packages/editor/src/core/helpers/editor-ref.ts (1)
getEditorRefHelpers(24-282)
packages/editor/src/core/components/editors/document/collaborative-editor.tsx (5)
packages/editor/src/core/types/editor.ts (1)
ICollaborativeDocumentEditorProps(187-205)packages/editor/src/core/contexts/collaboration-context.tsx (2)
useCollaboration(26-32)CollaborationProvider(14-24)packages/editor/src/core/hooks/use-collaborative-editor.ts (1)
useCollaborativeEditor(34-207)packages/editor/src/core/components/editors/document/page-renderer.tsx (1)
PageRenderer(38-116)packages/editor/src/index.ts (1)
CollaborativeDocumentEditorWithRef(3-3)
apps/web/core/hooks/use-page-fallback.ts (2)
packages/editor/src/core/types/collaboration.ts (1)
CollaborationState(29-33)packages/types/src/page/core.ts (1)
TDocumentPayload(66-70)
🪛 Biome (2.1.2)
apps/live/src/extensions/title-sync.ts
[error] 45-45: Unsafe usage of optional chaining.
If it short-circuits with 'undefined' the evaluation will throw TypeError here:
(lint/correctness/noUnsafeOptionalChaining)
⏰ 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: Build packages
🔇 Additional comments (10)
apps/web/core/hooks/use-page-fallback.ts (4)
1-2: Refactoring to collaboration state looks good.The shift from a boolean
hasConnectionFailedprop tocollaborationState: CollaborationState | nullprovides richer connection information and aligns with the broader real-time sync refactor.Also applies to: 12-17
19-26: Well-structured hook setup with derived state.Deriving
hasConnectionFailedfromcollaborationState?.stage.kindcentralizes the disconnection logic, and the new refs/state support the enhanced fallback UX.
77-84: Connection restoration logic looks good.Resetting
hasShownFallbackToastwhen the connection is restored allows the warning to be shown again if disconnection recurs, which is appropriate UX behavior.
88-88: Good API design exposing loading state.Returning
isFetchingFallbackBinaryenables consuming components to display loading indicators during fallback operations.apps/web/core/components/pages/editor/editor-body.tsx (4)
1-30: Imports and type wiring look consistentReact hooks usage matches imports, and
import typefor editor types keeps value bundles clean. No issues here.
88-89: Title ref creation and forwarding look correctCreating
titleEditorRefasuseRef<EditorTitleRefApi>(null)and passing it through astitleReftoCollaborativeDocumentEditorWithRefis consistent with a controlled-title ref API. No changes needed here.Also applies to: 267-267
165-179: Collaboration state → UI syncing mapping is clear and robustForwarding the full collaboration
stateto the parent and then mappingstage.kindinto three UI buckets (error/synced/syncing) is a good separation of concerns. The default “else → syncing” branch also degrades gracefully if new stage kinds are introduced.Also applies to: 181-181
261-267: Prop wiring intoCollaborativeDocumentEditorWithReflooks consistentPassing
titleRef,updatePageProperties, andisFetchingFallbackBinarythrough toCollaborativeDocumentEditorWithRefaligns with the new collaboration and fallback behavior introduced above. As long as the editor package exposes these props as expected in this branch, there’s nothing more to change here.Please double‑check against the current
@plane/editorAPI that these prop names and types (titleRef,updatePageProperties,isFetchingFallbackBinary) are up to date and available in the version used by this app.Also applies to: 280-291
packages/editor/src/core/components/editors/document/collaborative-editor.tsx (1)
53-84: Collaboration context wiring and ref forwarding look consistent.Using
CollaborationProvider+useCollaborationto supply a non-nullproviderintouseCollaborativeEditor, and wrapping the legacyCollaborativeDocumentEditorWithRefaround the newCollaborativeDocumentEditorkeeps the external API stable while refactoring internals. The token memoization and prop threading intoPageRendereralso look coherent with the new collaboration model.Also applies to: 135-150, 152-156
packages/editor/src/core/hooks/use-collaborative-editor.ts (1)
65-89: Collaborative + navigation wiring is coherent and memoized correctly.The composition of
SideMenuExtension,HeadingListExtension,Collaboration(with distinct fields for default vs title), and the navigation extensions, together with memoizededitorConfigand theuseEffectthat registers main/title editors withuseEditorNavigation, forms a clear, well-structured setup for collaborative editing. Dependency arrays on the memos/effect look correct for avoiding unnecessary re-instantiation while still reacting to key changes.Also applies to: 105-159, 194-201
Palanikannan1437
changed the title
pushya22
changed the title
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/live/src/extensions/title-update/title-utils.ts (1)
6-11: Duplicate function name exists in codebase with different implementations.An
extractTextFromHTMLfunction already exists inpackages/editor/src/core/helpers/yjs-utils.tsusing regex-based stripping (/<[^>]*>/g), while this new implementation uses DOMPurify viasanitizeHTML. Although both functions are in separate modules and the DOMPurify approach is more secure and handles edge cases better, having two functions with identical names but different behaviors could create confusion during maintenance.Consider either renaming this function (e.g.,
sanitizeHTMLToText) to clarify its approach, or consolidating by updating the existing function inyjs-utils.tsto use the safer DOMPurify approach for consistency across the codebase.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/live/src/extensions/title-update/title-utils.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,mts,cts}
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/*.{ts,tsx,mts,cts}: Useconsttype parameters for more precise literal inference in TypeScript 5.0+
Use thesatisfiesoperator to validate types without widening them
Leverage inferred type predicates to reduce the need for explicitisreturn types in filter/check functions
UseNoInfer<T>utility to block inference for specific type arguments when they should be determined by other arguments
Utilize narrowing inswitch(true)blocks for control flow analysis (TypeScript 5.3+)
Rely on narrowing from direct boolean comparisons for type guards
Trust preserved narrowing in closures when variables aren't modified after the check (TypeScript 5.4+)
Use constant indices to narrow object/array properties (TypeScript 5.5+)
Use standard ECMAScript decorators (Stage 3) instead of legacyexperimentalDecorators
Useusingdeclarations for explicit resource management with Disposable pattern instead of manual cleanup (TypeScript 5.2+)
Usewith { type: "json" }for import attributes; avoid deprecatedassertsyntax (TypeScript 5.3/5.8+)
Useimport typeexplicitly when importing types to ensure they are erased during compilation, respectingverbatimModuleSyntaxflag
Use.ts,.mts,.ctsextensions inimport typestatements (TypeScript 5.2+)
Useimport type { Type } from "mod" with { "resolution-mode": "import" }for specific module resolution contexts (TypeScript 5.3+)
Use new iterator methods (map, filter, etc.) if targeting modern environments (TypeScript 5.6+)
Utilize newSetmethods likeunion,intersection, etc., when available (TypeScript 5.5+)
UseObject.groupBy/Map.groupBystandard methods for grouping instead of external libraries (TypeScript 5.4+)
UsePromise.withResolvers()for creating promises with exposed resolve/reject functions (TypeScript 5.7+)
Use copying array methods (toSorted,toSpliced,with) for immutable array operations (TypeScript 5.2+)
Avoid accessing instance fields viasuperin classes (TypeScript 5....
Files:
apps/live/src/extensions/title-update/title-utils.ts
🧬 Code graph analysis (1)
apps/live/src/extensions/title-update/title-utils.ts (2)
packages/editor/src/core/helpers/yjs-utils.ts (1)
extractTextFromHTML(208-212)packages/utils/src/string.ts (1)
sanitizeHTML(122-125)
⏰ 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). (3)
- GitHub Check: check:types
- GitHub Check: check:lint
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
apps/live/src/extensions/title-update/title-utils.ts (1)
1-11: Secure implementation using DOMPurify.Using
sanitizeHTML(which wraps DOMPurify) is the correct approach for extracting text from HTML. It properly handles malformed HTML, nested tags, and potential injection attempts—unlike regex-based approaches.Regarding the past security scanner warning about "Incomplete multi-character sanitization": this appears to be a false positive. DOMPurify with
ALLOWED_TAGS: []completely strips all HTML elements through proper DOM parsing rather than string manipulation, so it won't leave partial tags like<script.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/editor/src/core/helpers/yjs-utils.ts (1)
209-214: Consolidate duplicate extractTextFromHTML function.This function is identically defined in
apps/live/src/extensions/title-update/title-utils.ts(lines 5-10). Sincetitle-utils.tsis listed as a dependent of this file, consider consolidating:Option 1 (Recommended): Keep the function here in
yjs-utils.tsand import it intitle-utils.ts:// In apps/live/src/extensions/title-update/title-utils.ts import { extractTextFromHTML } from "@plane/editor/core/helpers/yjs-utils"; // Remove the local definitionOption 2: Remove this definition and import from
title-utils.tsif this helper is only used for title operations:-export const extractTextFromHTML = (html: string): string => { - // Use sanitizeHTML to safely extract text and remove all HTML tags - // This is more secure than regex as it handles edge cases and prevents injection - // Note: sanitizeHTML trims whitespace, which is acceptable for title extraction - return sanitizeHTML(html) || ""; -};Option 1 is preferred as it centralizes the utility and improves reusability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/editor/src/core/helpers/yjs-utils.ts(7 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,mts,cts}
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/*.{ts,tsx,mts,cts}: Useconsttype parameters for more precise literal inference in TypeScript 5.0+
Use thesatisfiesoperator to validate types without widening them
Leverage inferred type predicates to reduce the need for explicitisreturn types in filter/check functions
UseNoInfer<T>utility to block inference for specific type arguments when they should be determined by other arguments
Utilize narrowing inswitch(true)blocks for control flow analysis (TypeScript 5.3+)
Rely on narrowing from direct boolean comparisons for type guards
Trust preserved narrowing in closures when variables aren't modified after the check (TypeScript 5.4+)
Use constant indices to narrow object/array properties (TypeScript 5.5+)
Use standard ECMAScript decorators (Stage 3) instead of legacyexperimentalDecorators
Useusingdeclarations for explicit resource management with Disposable pattern instead of manual cleanup (TypeScript 5.2+)
Usewith { type: "json" }for import attributes; avoid deprecatedassertsyntax (TypeScript 5.3/5.8+)
Useimport typeexplicitly when importing types to ensure they are erased during compilation, respectingverbatimModuleSyntaxflag
Use.ts,.mts,.ctsextensions inimport typestatements (TypeScript 5.2+)
Useimport type { Type } from "mod" with { "resolution-mode": "import" }for specific module resolution contexts (TypeScript 5.3+)
Use new iterator methods (map, filter, etc.) if targeting modern environments (TypeScript 5.6+)
Utilize newSetmethods likeunion,intersection, etc., when available (TypeScript 5.5+)
UseObject.groupBy/Map.groupBystandard methods for grouping instead of external libraries (TypeScript 5.4+)
UsePromise.withResolvers()for creating promises with exposed resolve/reject functions (TypeScript 5.7+)
Use copying array methods (toSorted,toSpliced,with) for immutable array operations (TypeScript 5.2+)
Avoid accessing instance fields viasuperin classes (TypeScript 5....
Files:
packages/editor/src/core/helpers/yjs-utils.ts
🧠 Learnings (3)
📚 Learning: 2025-11-25T10:18:05.172Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-11-25T10:18:05.172Z
Learning: Applies to **/*.{ts,tsx,mts,cts} : Use `.ts`, `.mts`, `.cts` extensions in `import type` statements (TypeScript 5.2+)
Applied to files:
packages/editor/src/core/helpers/yjs-utils.ts
📚 Learning: 2025-11-25T10:18:05.172Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-11-25T10:18:05.172Z
Learning: Applies to **/*.{js,mjs,cjs} : Use `import` tags in JSDoc for cleaner type imports in JavaScript files when working in a mixed codebase (TypeScript 5.5+)
Applied to files:
packages/editor/src/core/helpers/yjs-utils.ts
📚 Learning: 2025-11-25T10:18:05.172Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-11-25T10:18:05.172Z
Learning: Applies to **/*.{ts,tsx,mts,cts} : Use `with { type: "json" }` for import attributes; avoid deprecated `assert` syntax (TypeScript 5.3/5.8+)
Applied to files:
packages/editor/src/core/helpers/yjs-utils.ts
🧬 Code graph analysis (1)
packages/editor/src/core/helpers/yjs-utils.ts (5)
packages/editor/src/core/extensions/core-without-props.ts (2)
CoreEditorExtensionsWithoutProps(24-58)DocumentEditorExtensionsWithoutProps(60-60)packages/editor/src/core/extensions/title-extension.ts (1)
TitleExtensions(6-14)apps/live/src/extensions/title-update/title-update-manager.ts (1)
updateTitle(45-68)apps/live/src/extensions/title-update/title-utils.ts (1)
extractTextFromHTML(6-11)packages/utils/src/string.ts (1)
sanitizeHTML(122-125)
⏰ 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). (2)
- GitHub Check: Build packages
- GitHub Check: Analyze (javascript)
🔇 Additional comments (4)
packages/editor/src/core/helpers/yjs-utils.ts (4)
2-2: LGTM on imports.The use of
import typeforExtensionsfollows TypeScript 5.0+ best practices for type-only imports. The new imports forTitleExtensionsandsanitizeHTMLare necessary for the added functionality.Also applies to: 13-14
19-19: LGTM on the title extensions export.The export provides a clear, type-safe API for consumers needing the title-specific editor extensions.
121-129: LGTM on the function signature extension.The
updateTitleparameter and optionaltitleHTMLreturn property provide a clean API for conditional title extraction while maintaining backward compatibility.
193-196: LGTM on the function call update.Passing
falseforupdateTitleis appropriate for HTML-to-document conversion where the title hasn't been populated in the Y.Doc structure yet.
3 participants
Description
This PR fixes real-time document sync issues introduced after the Vite migration. It adds a dedicated title editor with debounced updates, ensures reliable syncing between title and body, and restores IndexedDB persistence for offline/rehydration scenarios. Collaboration behaviors have been improved with stable connection state handling, highlight/scroll-to-node support, and safer fallback fetch logic.
Type of Change
Screenshots and Media (if applicable)
N/A
Test Scenarios
References
Related: realtime sync regressions after Vite migration, collaborative editing stability reports.
Summary by CodeRabbit
New Features
Bug Fixes
Style/Assets
✏️ Tip: You can customize this high-level summary in your review settings.