[WIKI-181] refactor: file plugins and types#7074
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 11 minutes and 1 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 (6)
WalkthroughThis update introduces a centralized Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Editor
participant DropHandlerPlugin
participant FilePlugins
participant UtilityExtension
participant FileHandler
User->>Editor: Uploads or drops a file
Editor->>DropHandlerPlugin: Handles paste/drop event
DropHandlerPlugin->>FilePlugins: Validates and processes file
FilePlugins->>UtilityExtension: Updates upload status
FilePlugins->>FileHandler: Calls delete/restore/upload as needed
FileHandler-->>UtilityExtension: Updates assetsUploadStatus
UtilityExtension-->>Editor: Exposes updated storage and status
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Pull Request Linked with Plane Work Items
Comment Automatically Generated by Plane |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
packages/editor/src/core/hooks/use-file-upload.ts (1)
77-77:⚠️ Potential issueMissing dependency in callback dependency array
The
uploadFilecallback dependency array does not include the newonInvalidFilefunction. This could lead to stale closures where the callback uses an outdated version of the error handler.- [acceptedMimeTypes, editorCommand, handleProgressStatus, loadFileFromFileSystem, maxFileSize, onUpload] + [acceptedMimeTypes, editorCommand, handleProgressStatus, loadFileFromFileSystem, maxFileSize, onInvalidFile, onUpload]
🧹 Nitpick comments (3)
packages/editor/src/core/extensions/custom-image/components/image-uploader.tsx (1)
10-10: Consider creating a helper function for error alertsBoth error handlers use the same implementation
(_error, message) => alert(message). Consider extracting this to a helper function to maintain DRY principles and enable future enhancements to error presentation.+ // handlers + const handleInvalidFile = useCallback((_error: any, message: string) => alert(message), []); + // hooks const { isUploading: isImageBeingUploaded, uploadFile } = useUploader({Then use this handler in both locations:
const { draggedInside, onDrop, onDragEnter, onDragLeave } = useDropZone({ acceptedMimeTypes: ACCEPTED_IMAGE_MIME_TYPES, editor, maxFileSize, - onInvalidFile: (_error, message) => alert(message), + onInvalidFile: handleInvalidFile, pos: getPos(), type: "image", uploader: uploadFile, });And:
await uploadFirstFileAndInsertRemaining({ acceptedMimeTypes: ACCEPTED_IMAGE_MIME_TYPES, editor, filesList, maxFileSize, - onInvalidFile: (_error, message) => alert(message), + onInvalidFile: handleInvalidFile, pos: getPos(), type: "image", uploader: uploadFile, });packages/editor/src/core/plugins/file/restore.ts (1)
35-36: Special case for imageComponent nodes seems to contradict file-agnostic goal.This conditional logic specifically checks for "imageComponent" node type, which contradicts the goal of creating a file-agnostic plugin. Consider refactoring this to use a more general approach or making the special behavior configurable.
- if (nodeType === "imageComponent" && !node.attrs.src?.startsWith("http")) return; + // Pass a function parameter to determine when to skip processing certain URLs + if (typeof skipRestoreCondition === 'function' && skipRestoreCondition(nodeType, node.attrs.src)) return;packages/editor/src/core/hooks/use-file-upload.ts (1)
179-181: Consider enhancing error reporting for invalid filesWhile the warning about invalid files is logged to the console, consider also passing this information through the error handler to provide better feedback to the user interface.
if (filteredFiles.length !== filesList.length) { console.warn("Some files were invalid and have been ignored."); + onInvalidFile(EFileError.GENERAL, "Some files were invalid and have been ignored."); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (19)
packages/editor/src/ce/types/storage.ts(1 hunks)packages/editor/src/core/extensions/custom-image/components/image-uploader.tsx(2 hunks)packages/editor/src/core/extensions/custom-image/custom-image.ts(3 hunks)packages/editor/src/core/extensions/image/extension.tsx(2 hunks)packages/editor/src/core/extensions/image/image-component-without-props.tsx(1 hunks)packages/editor/src/core/helpers/file.ts(1 hunks)packages/editor/src/core/hooks/use-editor.ts(1 hunks)packages/editor/src/core/hooks/use-file-upload.ts(7 hunks)packages/editor/src/core/hooks/use-read-only-editor.ts(1 hunks)packages/editor/src/core/plugins/file/delete.ts(1 hunks)packages/editor/src/core/plugins/file/restore.ts(1 hunks)packages/editor/src/core/plugins/file/types.ts(1 hunks)packages/editor/src/core/plugins/image/delete-image.ts(0 hunks)packages/editor/src/core/plugins/image/index.ts(0 hunks)packages/editor/src/core/plugins/image/restore-image.ts(0 hunks)packages/editor/src/core/plugins/image/types/image-node.ts(0 hunks)packages/editor/src/core/plugins/image/types/index.ts(0 hunks)packages/editor/src/core/types/config.ts(1 hunks)packages/editor/src/core/types/image.ts(0 hunks)
💤 Files with no reviewable changes (6)
- packages/editor/src/core/plugins/image/types/index.ts
- packages/editor/src/core/plugins/image/index.ts
- packages/editor/src/core/plugins/image/types/image-node.ts
- packages/editor/src/core/plugins/image/restore-image.ts
- packages/editor/src/core/types/image.ts
- packages/editor/src/core/plugins/image/delete-image.ts
🧰 Additional context used
🧬 Code Graph Analysis (3)
packages/editor/src/core/extensions/image/extension.tsx (2)
packages/editor/src/core/plugins/file/delete.ts (1)
TrackFileDeletionPlugin(8-56)packages/editor/src/core/plugins/file/restore.ts (1)
TrackFileRestorationPlugin(8-57)
packages/editor/src/core/plugins/file/restore.ts (2)
packages/editor/src/core/types/config.ts (1)
TFileHandler(6-18)packages/editor/src/core/plugins/file/types.ts (1)
TFileNode(3-8)
packages/editor/src/core/plugins/file/delete.ts (2)
packages/editor/src/core/types/config.ts (1)
TFileHandler(6-18)packages/editor/src/core/plugins/file/types.ts (1)
TFileNode(3-8)
🔇 Additional comments (29)
packages/editor/src/ce/types/storage.ts (1)
4-4: Import path updated correctlyThe import path for
ImageExtensionStoragehas been updated from"@/plugins/image"to"@/extensions/image", which aligns with the refactoring effort to move functionality from image-specific plugins to more general file handling extensions.packages/editor/src/core/extensions/image/image-component-without-props.tsx (1)
3-4: Import source refactored appropriatelyThe import path for
ImageExtensionStoragehas been updated from an absolute import to a relative local import, and the comment has been updated accordingly. This change correctly reflects the new structure whereImageExtensionStorageis now defined locally in the extension directory rather than in the plugins directory.packages/editor/src/core/extensions/custom-image/components/image-uploader.tsx (2)
88-88: Error handling implementation looks goodAdding an
onInvalidFilecallback for theuseDropZonehook implements the standardized error handling mechanism for file validation. This separates error reporting logic from UI presentation, improving code organization.
127-127: Consistent error handling implementationThe
onInvalidFilecallback is consistently applied here for file input changes, matching the approach used with drag-and-drop. This ensures uniform error reporting throughout the component.packages/editor/src/core/plugins/file/types.ts (1)
1-8: Good generic file node type definitionThe new
TFileNodetype is well-defined and appropriately captures the essential attributes needed for handling files generically in the editor. This supports the transition from image-specific to file-generic handling in the codebase.The type extends
ProseMirrorNodeand requires both asrcandidin its attributes, which are necessary properties for file identification and reference.packages/editor/src/core/hooks/use-read-only-editor.ts (1)
78-78: Updated meta key for file deletion trackingThe meta key has been updated from "skipImageDeletion" to "skipFileDeletion" to align with the more generalized file handling approach introduced in this refactoring.
packages/editor/src/core/hooks/use-editor.ts (1)
146-146: Updated meta key for file deletion trackingThe meta key has been renamed from "skipImageDeletion" to "skipFileDeletion" to be consistent with the new file-centric approach. This change ensures the editor properly communicates with the new file deletion tracking plugin.
packages/editor/src/core/extensions/custom-image/custom-image.ts (3)
14-15: Replaced image-specific plugins with generalized file pluginsThe imports have been updated to use the generalized file tracking plugins instead of image-specific ones, promoting a more versatile approach to file handling.
108-109: Updated plugin initialization with storage key parameterThe image-specific plugins have been replaced with generalized file plugins, which now require an additional parameter "deletedImageSet" to specify the storage key for tracking deleted files.
156-157: Added error handling to file validationAn
onErrorcallback has been added to theisFileValidfunction to provide standardized error handling for file validation failures.packages/editor/src/core/extensions/image/extension.tsx (3)
8-9: Replaced image-specific plugins with generalized file pluginsThe imports have been updated to use the generalized file tracking plugins instead of image-specific ones, consistent with the broader refactoring approach.
13-16: Added local ImageExtensionStorage type definitionA locally defined
ImageExtensionStoragetype has been added to replace the previously imported type from the removed image plugin types. This type includes the necessary storage properties for tracking deleted files and upload status.
36-37: Updated plugin initialization with storage key parameterThe plugins are now initialized with the "deletedImageSet" parameter to specify which storage map to use for tracking deleted files, aligning with the new generalized file tracking approach.
packages/editor/src/core/types/config.ts (3)
3-3: Function signature directly defined instead of import.The
restorefunction signature is now directly defined in the interface rather than imported from an image-specific module. This is a good change that supports the transition from image-specific to generic file handling.
9-10: Generalized file handlers replacing image-specific types.The
deleteanduploadfunctions are now directly defined with clear signatures rather than importing image-specific types. This change creates a more flexible, file-agnostic interface that can handle multiple file types beyond just images.
14-14: Documentation improvement for maxFileSize.The documentation example now clearly shows how to calculate the size in bytes, which improves developer experience.
packages/editor/src/core/plugins/file/restore.ts (2)
8-14: Well-structured plugin factory with good parameter naming.The
TrackFileRestorationPluginfactory function is well-designed with clear parameters:
editor: The TipTap editor instancerestoreHandler: The function that handles file restorationnodeType: The type of node to trackfileSetName: The storage key for tracking deleted filesThis generalized approach allows the plugin to work with different types of file nodes.
39-53: Good error handling in asynchronous file restoration.The code properly handles file restoration with appropriate error catching and storage updates. The plugin correctly:
- Checks if a file was previously tracked and marked as deleted
- Attempts restoration of deleted files
- Updates the tracking status upon successful restoration
- Catches and logs any errors during the restoration process
packages/editor/src/core/helpers/file.ts (3)
1-5: Good introduction of typed file error enum.The
EFileErrorenum improves type safety and creates a standardized way to communicate file validation errors throughout the application. The error types are clear and self-explanatory.
11-11: Adding error callback improves error handling design.The addition of an
onErrorcallback to theTArgstype is an excellent change. It:
- Decouples validation logic from UI presentation
- Allows for different error handling strategies in different contexts
- Makes the validation function more reusable and testable
15-15: Improved error handling by using the callback.The code now correctly uses the
onErrorcallback instead of direct alerts, providing:
- More flexibility in how errors are presented to users
- Better separation of concerns (validation logic vs. UI feedback)
- Consistent error reporting across the application
This is a significant improvement in the error handling design.
Also applies to: 18-18, 23-23, 28-31
packages/editor/src/core/plugins/file/delete.ts (4)
8-14: Well-structured plugin factory for file deletion tracking.The
TrackFileDeletionPluginis well-designed with clear parameters and purpose. It generalizes deletion tracking from images to arbitrary file types, allowing for more flexible file handling in the editor.
24-28: Good transaction filtering logic.The plugin appropriately filters transactions to:
- Skip transactions with the "skipFileDeletion" meta flag
- Ignore transactions that don't change the document
This prevents unnecessary processing and respects deliberate skipping of deletion, e.g., when clearing editor content programmatically.
32-40: Efficient file removal detection algorithm.The code uses an efficient approach to detect removed files by:
- Collecting all current file sources in the new state
- Comparing old state nodes against this collection
- Identifying nodes that no longer exist in the new state
This approach correctly handles both direct node deletion and node replacement scenarios.
42-51: File deletion with good error handling.The code properly:
- Updates the tracking status in editor storage
- Asynchronously calls the delete handler
- Handles and logs potential errors during deletion
The error handling is appropriate, and the asynchronous processing doesn't block the editor.
packages/editor/src/core/hooks/use-file-upload.ts (4)
16-16: Good addition of typed error handling!Adding the
onInvalidFilecallback with theEFileErrorenum type improves error handling by providing structured feedback when file validation fails. This makes the API more robust and user-friendly.
27-27: Well implemented error callback in validation logicThe error callback is correctly passed to the
isFileValidfunction, providing a clean way to handle validation errors while maintaining separation of concerns.Also applies to: 41-41
87-87: Correctly propagated error handler through dropzone functionalityThe error handler is properly passed through the dropzone hook and its dependency array is correctly updated to include the new parameter.
Also applies to: 94-94, 130-130, 136-137
155-155: Consistent error handling for multiple file uploadsThe error handler implementation is consistent throughout the file, including in the multiple file upload function. This ensures uniform error handling regardless of how files are uploaded.
Also applies to: 163-163, 173-173
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
packages/editor/src/core/extensions/custom-image/components/image-node.tsx (1)
78-88: 💡 Verification agent🧩 Analysis chain
Check for proper error handling in file upload validation.
Since the PR includes improved invalid file error handling, ensure that the
CustomImageUploadercomponent properly handles the case when a file exceedsmaxFileSize. Consider checking for proper integration with the improved error handling mechanisms.
🏁 Script executed:
#!/bin/bash # Check how CustomImageUploader handles file validation errors # Find the CustomImageUploader definition echo "Looking for CustomImageUploader implementation:" rg -A 10 "export (const|function) CustomImageUploader" --type=typescript # Check for onInvalidFile usage in the component echo "Checking for error handling implementation:" rg "onInvalidFile" packages/editor/src/core/extensions/custom-imageLength of output: 653
Improve invalid-file error handling in CustomImageUploader
The
CustomImageUploaderinner component (packages/editor/src/core/extensions/custom-image/components/image-uploader.tsx) currently catches file-validation errors viaonInvalidFile: (_error, message) => alert(message),to display an alert. To hook into the new invalid-file error handling you’ve added, please:
- Replace the hard-coded
alert(message)with the editor’s notification/toast API or by forwarding to a passed-in callback.- Expose an optional
onInvalidFile?: (error: Error, message: string) => voidprop inCustomImageUploaderPropsand use it as the handler.- In
packages/editor/src/core/extensions/custom-image/components/image-node.tsx, pass your improved handler (e.g. a toast or the existingfailedToLoadImage) via this newonInvalidFileprop so that files exceedingmaxFileSizesurface correctly in the UI.Example diff for image-uploader.tsx:
--- a/packages/editor/src/core/extensions/custom-image/components/image-uploader.tsx +++ b/packages/editor/src/core/extensions/custom-image/components/image-uploader.tsx @@ - onInvalidFile: (_error, message) => alert(message), + onInvalidFile: (error, message) => { + // forward to consumer or fallback to toast + props.onInvalidFile?.(error, message) || showErrorToast(message) + },And in image-node.tsx:
--- a/packages/editor/src/core/extensions/custom-image/components/image-node.tsx +++ b/packages/editor/src/core/extensions/custom-image/components/image-node.tsx @@ - <CustomImageUploader + <CustomImageUploader + onInvalidFile={failedToLoadImage} editor={editor} failedToLoadImage={failedToLoadImage} … />This ensures that oversize-file errors use your new error-handling flows rather than browser alerts.
🧹 Nitpick comments (6)
packages/editor/src/core/extensions/issue-embed/issue-embed-without-props.ts (2)
35-37: Consider updating parseHTML tag selector.While you've updated the extension name to use the constant
CORE_EXTENSIONS.WORK_ITEM_EMBED, the HTML tag selector in theparseHTMLmethod still uses the hardcoded string"issue-embed-component". This might lead to inconsistency if the tag name needs to change in the future.parseHTML() { return [ { - tag: "issue-embed-component", + tag: "issue-embed-component", // Consider using a constant here as well }, ]; },
40-42: Consider updating renderHTML output tag name.Similar to the parseHTML method, the renderHTML method still uses the hardcoded string
"issue-embed-component". It would be more consistent to also use a constant for this tag name.renderHTML({ HTMLAttributes }) { - return ["issue-embed-component", mergeAttributes(HTMLAttributes)]; + return ["issue-embed-component", mergeAttributes(HTMLAttributes)]; // Consider using a constant here as well },packages/editor/src/core/extensions/custom-image/components/upload-status.tsx (1)
14-64: Consider renaming component to match generalized file handling.Since the refactoring aims to generalize image-specific file handling to support arbitrary files, consider renaming the component from
ImageUploadStatusto something more generic likeFileUploadStatus. This would better align with the architectural changes.-export const ImageUploadStatus: React.FC<Props> = (props) => { +export const FileUploadStatus: React.FC<Props> = (props) => { // Rest of the component implementation };packages/editor/src/core/extensions/extensions.tsx (1)
155-161: Consider updating shouldHidePlaceholder check.The
shouldHidePlaceholdercondition still checks foreditor.isActive("imageComponent")using a string literal rather than the constant. Consider updating this for consistency with the rest of the refactoring.const shouldHidePlaceholder = editor.isActive("table") || editor.isActive("codeBlock") || editor.isActive("image") || - editor.isActive("imageComponent"); + editor.isActive(CORE_EXTENSIONS.CUSTOM_IMAGE);packages/editor/src/core/plugins/file/delete.ts (1)
12-12: Consider using a more specific plugin key nameThe current plugin key name "delete-utility" is somewhat ambiguous. A more specific name like "file-deletion-tracker" could better describe the plugin's purpose.
- key: new PluginKey("delete-utility"), + key: new PluginKey("file-deletion-tracker"),packages/editor/src/core/extensions/utility.ts (1)
26-26: Consider using the CORE_EXTENSIONS constant for extension nameFor consistency with other extensions like CUSTOM_IMAGE, consider using the CORE_EXTENSIONS constant for the extension name rather than a string literal.
- name: "utility", + name: CORE_EXTENSIONS.UTILITY,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (35)
packages/editor/src/ce/constants/utility.ts(1 hunks)packages/editor/src/ce/types/storage.ts(1 hunks)packages/editor/src/core/components/editors/editor-container.tsx(2 hunks)packages/editor/src/core/constants/extension.ts(1 hunks)packages/editor/src/core/extensions/callout/extension-config.ts(2 hunks)packages/editor/src/core/extensions/code-inline/index.tsx(2 hunks)packages/editor/src/core/extensions/code/code-block.ts(3 hunks)packages/editor/src/core/extensions/custom-code-inline.ts(0 hunks)packages/editor/src/core/extensions/custom-color.ts(2 hunks)packages/editor/src/core/extensions/custom-image/components/image-node.tsx(2 hunks)packages/editor/src/core/extensions/custom-image/components/image-uploader.tsx(3 hunks)packages/editor/src/core/extensions/custom-image/components/upload-status.tsx(2 hunks)packages/editor/src/core/extensions/custom-image/custom-image.ts(3 hunks)packages/editor/src/core/extensions/custom-image/read-only-custom-image.ts(2 hunks)packages/editor/src/core/extensions/custom-link/extension.tsx(2 hunks)packages/editor/src/core/extensions/enter-key-extension.tsx(1 hunks)packages/editor/src/core/extensions/extensions.tsx(4 hunks)packages/editor/src/core/extensions/headers.ts(2 hunks)packages/editor/src/core/extensions/horizontal-rule.ts(2 hunks)packages/editor/src/core/extensions/image/extension.tsx(1 hunks)packages/editor/src/core/extensions/image/image-component-without-props.tsx(1 hunks)packages/editor/src/core/extensions/index.ts(1 hunks)packages/editor/src/core/extensions/issue-embed/issue-embed-without-props.ts(1 hunks)packages/editor/src/core/extensions/side-menu.tsx(2 hunks)packages/editor/src/core/extensions/slash-commands/root.tsx(2 hunks)packages/editor/src/core/extensions/table/table-cell/table-cell.ts(1 hunks)packages/editor/src/core/extensions/table/table-header/table-header.ts(1 hunks)packages/editor/src/core/extensions/table/table-row/table-row.ts(1 hunks)packages/editor/src/core/extensions/table/table/table.ts(3 hunks)packages/editor/src/core/extensions/typography/index.ts(2 hunks)packages/editor/src/core/extensions/utility.ts(1 hunks)packages/editor/src/core/hooks/use-editor.ts(6 hunks)packages/editor/src/core/plugins/file/delete.ts(1 hunks)packages/editor/src/core/plugins/file/restore.ts(1 hunks)web/core/store/pages/base-page.ts(0 hunks)
💤 Files with no reviewable changes (2)
- web/core/store/pages/base-page.ts
- packages/editor/src/core/extensions/custom-code-inline.ts
✅ Files skipped from review due to trivial changes (16)
- packages/editor/src/core/extensions/custom-link/extension.tsx
- packages/editor/src/core/extensions/side-menu.tsx
- packages/editor/src/core/extensions/table/table-row/table-row.ts
- packages/editor/src/core/extensions/index.ts
- packages/editor/src/core/extensions/headers.ts
- packages/editor/src/core/extensions/slash-commands/root.tsx
- packages/editor/src/ce/constants/utility.ts
- packages/editor/src/core/extensions/table/table-header/table-header.ts
- packages/editor/src/core/extensions/custom-image/read-only-custom-image.ts
- packages/editor/src/core/extensions/code/code-block.ts
- packages/editor/src/core/extensions/typography/index.ts
- packages/editor/src/core/components/editors/editor-container.tsx
- packages/editor/src/core/extensions/table/table/table.ts
- packages/editor/src/core/extensions/image/extension.tsx
- packages/editor/src/core/constants/extension.ts
- packages/editor/src/core/extensions/table/table-cell/table-cell.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/editor/src/core/extensions/custom-image/components/image-uploader.tsx
- packages/editor/src/core/extensions/image/image-component-without-props.tsx
- packages/editor/src/ce/types/storage.ts
- packages/editor/src/core/plugins/file/restore.ts
- packages/editor/src/core/hooks/use-editor.ts
🧰 Additional context used
🧬 Code Graph Analysis (6)
packages/editor/src/core/extensions/custom-image/components/upload-status.tsx (1)
packages/editor/src/core/helpers/get-extension-storage.ts (1)
getExtensionStorage(5-8)
packages/editor/src/core/extensions/custom-image/components/image-node.tsx (1)
packages/editor/src/core/helpers/get-extension-storage.ts (1)
getExtensionStorage(5-8)
packages/editor/src/core/extensions/enter-key-extension.tsx (1)
packages/editor/src/core/helpers/get-extension-storage.ts (1)
getExtensionStorage(5-8)
packages/editor/src/core/extensions/extensions.tsx (2)
packages/editor/src/core/helpers/get-extension-storage.ts (1)
getExtensionStorage(5-8)packages/editor/src/core/extensions/utility.ts (1)
UtilityExtension(22-51)
packages/editor/src/core/extensions/utility.ts (3)
packages/editor/src/core/types/config.ts (1)
TFileHandler(6-18)packages/editor/src/core/plugins/file/delete.ts (1)
TrackFileDeletionPlugin(10-65)packages/editor/src/core/plugins/file/restore.ts (1)
TrackFileRestorationPlugin(12-70)
packages/editor/src/core/plugins/file/delete.ts (3)
packages/editor/src/core/types/config.ts (1)
TFileHandler(6-18)packages/editor/src/ce/constants/utility.ts (1)
NODE_FILE_MAP(3-14)packages/editor/src/core/plugins/file/types.ts (1)
TFileNode(3-8)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (35)
packages/editor/src/core/extensions/enter-key-extension.tsx (4)
2-5: Well-structured import organization.The code introduces new imports for constants and helpers, following a clean separation pattern with comment dividers. This improves code organization and readability.
9-9: Good use of constants instead of string literals.Replacing the hardcoded string
"enterKey"with theCORE_EXTENSIONS.ENTER_KEYconstant improves maintainability and prevents typos. This aligns with the broader refactoring mentioned in the summary to centralize extension names.
14-15: Improved extension storage access.Using the
getExtensionStoragehelper function to access the mention extension's state is a significant improvement over direct storage access. This approach:
- Decouples the extension from internal storage structures
- Provides type safety
- Follows the centralized pattern established in the codebase
16-16: Clean optional callback invocation.The use of optional chaining for the
onEnterKeyPresscallback is more concise and safer than the previous implementation, eliminating the need for an explicit null check.packages/editor/src/core/extensions/issue-embed/issue-embed-without-props.ts (1)
2-3: Good refactor using centralized extension constants.The change from hardcoded string literal
"issue-embed-component"toCORE_EXTENSIONS.WORK_ITEM_EMBEDimproves maintainability and consistency. This aligns with the broader refactoring effort across the codebase to use centralized extension naming.Also applies to: 7-7
packages/editor/src/core/extensions/custom-image/components/upload-status.tsx (2)
4-7: Good refactor to use centralized storage access pattern.The change from direct storage access (
editor.storage.imageComponent?.assetsUploadStatus[nodeId]) to using thegetExtensionStoragehelper with theCORE_EXTENSIONS.UTILITYconstant is a solid improvement. This centralizes file upload state management and makes the code more consistent and type-safe.Also applies to: 23-23
21-24:Details
✅ Verification successful
Verify nullable/undefined handling in the selector.
The selector checks for optional chaining on both
assetsUploadStatusand the indexed property[nodeId], which is good. However, ensure that theUTILITYextension is always initialized before this component renders to avoid potential undefined errors.
🏁 Script executed:
#!/bin/bash # Check if UtilityExtension is always added to extensions array before ImageUploadStatus is used # Search for ImageUploadStatus imports and usages echo "Searching for ImageUploadStatus usages:" rg -A 3 "ImageUploadStatus" --type=typescript # Check if UtilityExtension is added before ImageUploadStatus is used echo "Checking extension initialization order:" rg "UtilityExtension" packages/editor/src/core/extensions/extensions.tsxLength of output: 388
UtilityExtension always loaded; selector’s optional chaining suffices
TheUtilityExtensionis included by default inpackages/editor/src/core/extensions/extensions.tsx, so thegetExtensionStorage(editor, CORE_EXTENSIONS.UTILITY)call will never be uninitialized, and the optional chaining onassetsUploadStatus?.[nodeId]safely returnsundefinedif no status exists. No further action required.packages/editor/src/core/extensions/custom-image/components/image-node.tsx (1)
3-8: Good implementation of centralized extension storage access.Using
getExtensionStorage(editor, CORE_EXTENSIONS.CUSTOM_IMAGE).maxFileSizeinstead of direct property access improves type safety and maintains consistency with the codebase-wide refactoring pattern.Also applies to: 83-83
packages/editor/src/core/extensions/extensions.tsx (4)
10-11: Good update to support the utility extension.The addition of imports for
CORE_EXTENSIONS,getExtensionStorage, andUtilityExtensionproperly supports the refactoring to centralize file handling and extension naming.Also applies to: 34-34, 38-38
152-153: Good implementation of centralized upload progress status check.Replacing direct storage access with
getExtensionStorage(editor, CORE_EXTENSIONS.UTILITY)?.uploadInProgressaligns with the pattern of using centralized storage access and standardized extension naming.
175-175: Good integration of the new UtilityExtension.Adding
UtilityExtension(fileHandler)to the extensions array properly integrates the new utility extension that centralizes file deletion, restoration, and upload status handling.
9-46:Details
❓ Verification inconclusive
Verify file handler propagation.
The
fileHandleris passed to multiple extensions:UtilityExtension,CoreEditorAdditionalExtensions,ImageExtension, andCustomImageExtension. Ensure that the file handler's properties are used consistently across these extensions to avoid potential integration issues.Also applies to: 177-192
🏁 Script executed:
#!/bin/bash # Check fileHandler usage across extensions # Find fileHandler interface definition echo "Looking for TFileHandler definition:" rg "type TFileHandler" --type=typescript -A 10 # Check how fileHandler is used in each extension echo "Checking usage in UtilityExtension:" rg -A 5 "UtilityExtension\(.*\)" --type=typescript echo "Checking usage in ImageExtension:" rg -A 5 "ImageExtension\(.*\)" --type=typescript echo "Checking usage in CustomImageExtension:" rg -A 5 "CustomImageExtension\(.*\)" --type=typescriptLength of output: 679
#!/bin/bash
set -eecho "Finding TFileHandler definition:"
rg "type TFileHandler" -n -A5echo -e "\nSearching for fileHandler usage across entire codebase:"
rg "fileHandler" -n -C3echo -e "\nLocating the extensions.tsx file:"
EXT_FILE=$(fd extensions.tsx | head -n 1)
echo "Found extensions file at: $EXT_FILE"echo -e "\nShowing first 200 lines of the extensions file:"
sed -n '1,200p' "$EXT_FILE"echo -e "\nfileHandler usage within $EXT_FILE:"
rg "fileHandler" -n -C3 "$EXT_FILE"
#!/bin/bash
set -eecho "1. Locating TFileHandler definition:"
rg "type TFileHandler" -n -A5echo -e "\n2. Finding all references to fileHandler:"
rg "fileHandler" -n -C3echo -e "\n3. Inspecting UtilityExtension usage:"
rg "UtilityExtension" -n -C5echo -e "\n4. Inspecting ImageExtension usage:"
rg "ImageExtension" -n -C5echo -e "\n5. Inspecting CustomImageExtension usage:"
rg "CustomImageExtension" -n -C5echo -e "\n6. Inspecting CoreEditorAdditionalExtensions invocation:"
rg "CoreEditorAdditionalExtensions" -n -C5
#!/bin/bash
set -eecho "1. Locate TFileHandler definition:"
rg "type TFileHandler" -n -C5 --glob ".ts"echo -e "\n2. Find all fileHandler references:"
rg "fileHandler" -n -C3 --glob ".ts"echo -e "\n3. Inspect UtilityExtension declaration/usage:"
rg -n -C5 "UtilityExtension" --glob ".ts"echo -e "\n4. Inspect ImageExtension declaration/usage:"
rg -n -C5 "ImageExtension" --glob ".ts"echo -e "\n5. Inspect CustomImageExtension declaration/usage:"
rg -n -C5 "CustomImageExtension" --glob ".ts"echo -e "\n6. Inspect CoreEditorAdditionalExtensions invocation:"
rg -n -C5 "CoreEditorAdditionalExtensions" --glob ".ts"
Verify consistent
fileHandlerintegration across extensionsEnsure that the
fileHandleryou’re passing into the following extensions all share the same expected shape and use its methods/properties consistently. Mismatches can lead to runtime errors or unexpected behavior:
- packages/editor/src/core/extensions/extensions.tsx (lines 9–46 and 177–192): where you pass
fileHandlerinto
UtilityExtensionCoreEditorAdditionalExtensionsImageExtensionCustomImageExtension- TFileHandler type (in your
@/typesfolder): confirm that all extensions consume the same required/optional properties and method signatures.- Each extension’s constructor or factory must correctly declare and use
fileHandler; verify there are no missing null checks or renamed properties.Please review these locations and update any mismatched signatures or usages to maintain consistent integration.
packages/editor/src/core/extensions/horizontal-rule.ts (1)
3-4: Good refactoring to use constants instead of string literalsUsing
CORE_EXTENSIONS.HORIZONTAL_RULEconstant instead of hardcoded string literals improves code maintainability and helps prevent typos. This change is part of the broader effort to standardize extension naming across the editor.Also applies to: 12-12, 22-22
packages/editor/src/core/extensions/custom-color.ts (1)
4-4: Good adoption of constant-based naming patternUsing
CORE_EXTENSIONS.CUSTOM_COLORconstant instead of hardcoded string literals for both the command interface namespace and extension name improves consistency and maintainability.Also applies to: 8-8, 38-38
packages/editor/src/core/extensions/code-inline/index.tsx (1)
2-3: Proper standardization of extension namingReplacing hardcoded strings with
CORE_EXTENSIONS.CODE_INLINEaligns with the refactoring goals and maintains consistency with other extensions. The comment indicating the constants import is a good practice for code organization.Also applies to: 11-11, 32-32
packages/editor/src/core/extensions/callout/extension-config.ts (2)
4-5: Consistent application of extension naming patternGood refactoring to use
CORE_EXTENSIONS.CALLOUTconstant, maintaining consistency with other extensions.Also applies to: 14-14, 21-21
3-3:Details
✅ Verification successful
Verify the necessity of the NodeType import
The addition of
import { Node as NodeType } from "@tiptap/pm/model";seems unrelated to the main refactoring focus. This import appears to be used on line 41, but it might have already been available through another import.Please verify if this import is actually needed or if it was accidentally added during the refactoring.
🏁 Script executed:
#!/bin/bash # Checking for existing imports of Node as NodeType in the previous version git show HEAD:packages/editor/src/core/extensions/callout/extension-config.ts | grep -A 5 "import.*Node" # Checking for usage of NodeType in the file grep -n "NodeType" packages/editor/src/core/extensions/callout/extension-config.tsLength of output: 641
No action needed:
NodeTypeimport is required
Theimport { Node as NodeType } from "@tiptap/pm/model";statement was present in the previous version (line 3) and is used in theserializemethod (line 41). It should remain as-is.packages/editor/src/core/plugins/file/delete.ts (6)
10-11: Well-structured plugin factory functionThe plugin factory function accepts an editor instance and a deletion handler function, with proper typing using the
TFileHandler["delete"]type. This allows for type-safe function composition.
14-16: Good use of type definition for file sourcesThe type definition for
newFileSourcesproperly describes an object with string keys mapping to either a Set of strings or undefined, which makes the code more maintainable and self-documenting.
19-29: Efficient file source collectionThe implementation efficiently collects sources of all file nodes in the new document state using a single traversal, and properly groups them by node type. The use of Sets for storing file sources is appropriate for the subsequent comparison operations.
31-33: Good transaction filtering with skip mechanismThe implementation includes a mechanism to skip file deletion tracking for certain transactions, which is useful for programmatic operations like clearing the editor. This prevents unnecessary file deletion operations during bulk content changes.
38-47: Comprehensive file node detectionThe code carefully checks each node in the old state against the new state to determine if a file has been removed. The validation against
NODE_FILE_MAPensures only supported file node types are processed.
49-60: Proper error handling for file deletionThe implementation includes appropriate error handling during the file deletion process, ensuring that errors won't break the editor's operation. The error logging helps with debugging potential issues.
One minor suggestion: consider adding more context to the error message, such as including the file source (truncated if too long) to help with debugging.
- console.error("Error deleting file via delete utility plugin:", error); + console.error(`Error deleting file via delete utility plugin for src "${src.substring(0, 50)}${src.length > 50 ? '...' : ''}":`, error);packages/editor/src/core/extensions/utility.ts (6)
9-15: Well-defined command interface extensionThe extension properly declares a new command
updateAssetsUploadStatusin the TipTap Commands interface, allowing for type-safe command usage throughout the editor.
17-20: Clear storage interface definitionThe
UtilityExtensionStorageinterface clearly defines the storage structure for the extension, making the code more maintainable and providing proper typing for storage access.
22-24: Good use of destructuring for propsThe code properly destructures the required handlers and state from the props, making the implementation cleaner and more readable.
28-34: Comprehensive plugin integrationThe extension effectively integrates three plugins: file deletion tracking, file restoration tracking, and code mark handling. This consolidation centralizes related functionality into a single extension, improving code organization.
36-41: Simple and clear storage initializationThe storage initialization is straightforward and properly sets initial values for tracking asset upload status and the upload progress flag.
43-49: Concise command implementationThe
updateAssetsUploadStatuscommand is implemented in a clear and concise manner, directly updating the extension's storage with the provided status.packages/editor/src/core/extensions/custom-image/custom-image.ts (6)
7-7: Good use of centralized constantsThe addition of the CORE_EXTENSIONS import centralizes extension naming, which improves maintainability and reduces the risk of inconsistent naming across the codebase.
25-25: Consistent use of extension name constantsReplacing the hardcoded string "imageComponent" with CORE_EXTENSIONS.CUSTOM_IMAGE in the commands interface makes the code more maintainable and less prone to errors from inconsistent naming.
34-35: Improved helper function with extension constantThe refactored
getImageComponentImageFileMapfunction now uses the CORE_EXTENSIONS constant, ensuring consistency with other parts of the codebase.
54-54: Consistent extension namingThe use of CORE_EXTENSIONS.CUSTOM_IMAGE for the extension name ensures consistency throughout the codebase.
141-141: Enhanced error handling with user feedbackThe addition of the onError callback in the file validation function improves the user experience by providing feedback when file validation fails.
However, consider replacing the direct
alert()call with a more flexible callback that allows for different UI implementations:- onError: (_error, message) => alert(message), + onError: (_error, message) => props.onValidationError?.(message) || alert(message),This would require adding an optional
onValidationErrorproperty to theInsertImageComponentPropsinterface.
34-126:Details
✅ Verification successful
Verify that deletion tracking still works correctly after removal of image-specific plugins
The code no longer includes image-specific deletion plugins, as this functionality has been moved to the new UtilityExtension. Make sure that the
deletedImageSetstorage property is still being properly updated by the new centralized plugins.
🏁 Script executed:
#!/bin/bash # Check how deletedImageSet is used in the codebase after the refactor # First, find all usages of deletedImageSet in the codebase echo "Checking usages of deletedImageSet:" rg "deletedImageSet" --type ts # Then, check the NODE_FILE_MAP constant to see how file types are mapped to storage sets echo "\nChecking NODE_FILE_MAP definition:" rg -A 15 "NODE_FILE_MAP" --type tsLength of output: 12520
Deletion tracking verified
The centralized file deletion plugin (
TrackFileDeletionPlugin) uses theNODE_FILE_MAPto map both theimageandimageComponentnode types to thedeletedImageSetstorage key. During itsappendTransaction, it correctly calls:editor.storage[nodeType][nodeFileSetDetails.fileSetName]?.set(src, true);for all matching node types—including your custom image extension—so no further updates to
CustomImageExtensionare required.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
packages/editor/src/core/plugins/drop.ts (2)
111-113: Consider enhancing error handlingWhile errors are caught and logged to the console, there's no user-facing error notification or feedback. This might lead to a confusing user experience if file insertion fails silently.
Consider adding a callback parameter for error reporting to provide user feedback:
type InsertFilesSafelyArgs = { editor: Editor; event: "insert" | "drop"; files: File[]; initialPos: number; type?: Extract<TEditorCommands, "attachment" | "image">; + onError?: (error: Error) => void; }; export const insertFilesSafely = async (args: InsertFilesSafelyArgs) => { const { editor, event, files, initialPos, type, onError } = args; // ... try { // ... } catch (error) { console.error(`Error while ${event}ing file:`, error); + onError?.(error instanceof Error ? error : new Error(String(error))); } // ... };
116-116: Simple position increment might not account for content sizeThe position is incremented by just 1 after each file insertion, which may not account for the actual size of the inserted content in the document structure.
While this works as a basic approach, consider if a more sophisticated position calculation might be needed for cases where large components are inserted:
// Move to the next position - pos += 1; + // Increment by at least 1, or consider calculating the actual size of the inserted content + pos += 1;packages/editor/src/core/plugins/markdown-clipboard.ts (4)
18-27: Consider improving type definitions inprocessTableContent.The function parameters could benefit from more specific type annotations. The
tableNodeparameter accepts bothNode | Fragment, but the operations performed suggest it should specifically be a table node.- const processTableContent = (tableNode: Node | Fragment) => { + const processTableContent = (tableNode: Node | Fragment): string => {
39-59: Complex traversal logic could benefit from documentation.The
traverseToParentOfLeaffunction performs complex node traversal logic that could be difficult to understand for future maintainers. Consider adding JSDoc comments explaining the purpose and algorithm.+ /** + * Traverses down the node tree to find the parent of the leaf node. + * Special handling for list items and multi-child nodes. + * @param node The starting node + * @param parent The parent fragment or node + * @param depth The current depth in the tree + * @returns The appropriate parent node or fragment to serialize + */ const traverseToParentOfLeaf = (node: Node | null, parent: Fragment | Node, depth: number): Node | Fragment => {
67-69: Use optional chaining for better readability.The condition chain can be simplified using optional chaining syntax.
- while (currentNode && currentNode.content && currentNode.childCount === 1 && currentNode.firstChild) { + while (currentNode?.content && currentNode.childCount === 1 && currentNode.firstChild) {🧰 Tools
🪛 Biome (1.9.4)
[error] 67-67: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
61-75: Consider splitting the content serialization logic for better readability.The serialization logic would be clearer if extracted into a separate helper function, particularly the node traversal and text node handling section.
+ /** + * Finds the appropriate node to serialize when dealing with a single child + * @param content The slice content to process + * @param openStart The openStart value from the slice + * @returns Either the text content or serialized markdown + */ + const serializeSingleChild = (content: Fragment, openStart: number, serializer: any): string => { + const targetNode = traverseToParentOfLeaf(content.firstChild, content, openStart); + + let currentNode = targetNode; + while (currentNode?.content && currentNode.childCount === 1 && currentNode.firstChild) { + currentNode = currentNode.firstChild; + } + if (currentNode instanceof Node && currentNode.isText) { + return currentNode.text || ''; + } + + return serializer.serialize(targetNode); + }; if (slice.content.childCount > 1) { return markdownSerializer.serialize(slice.content); } else { - const targetNode = traverseToParentOfLeaf(slice.content.firstChild, slice.content, slice.openStart); - - let currentNode = targetNode; - while (currentNode && currentNode.content && currentNode.childCount === 1 && currentNode.firstChild) { - currentNode = currentNode.firstChild; - } - if (currentNode instanceof Node && currentNode.isText) { - return currentNode.text; - } - - return markdownSerializer.serialize(targetNode); + return serializeSingleChild(slice.content, slice.openStart, markdownSerializer); }🧰 Tools
🪛 Biome (1.9.4)
[error] 67-67: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (9)
packages/editor/src/core/extensions/clipboard.ts(0 hunks)packages/editor/src/core/extensions/drop.ts(0 hunks)packages/editor/src/core/extensions/extensions.tsx(4 hunks)packages/editor/src/core/extensions/index.ts(1 hunks)packages/editor/src/core/extensions/read-only-extensions.tsx(2 hunks)packages/editor/src/core/extensions/utility.ts(1 hunks)packages/editor/src/core/hooks/use-file-upload.ts(7 hunks)packages/editor/src/core/plugins/drop.ts(1 hunks)packages/editor/src/core/plugins/markdown-clipboard.ts(1 hunks)
💤 Files with no reviewable changes (2)
- packages/editor/src/core/extensions/clipboard.ts
- packages/editor/src/core/extensions/drop.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/editor/src/core/extensions/read-only-extensions.tsx
- packages/editor/src/core/extensions/index.ts
- packages/editor/src/core/extensions/extensions.tsx
- packages/editor/src/core/extensions/utility.ts
- packages/editor/src/core/hooks/use-file-upload.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/editor/src/core/plugins/drop.ts (2)
packages/editor/src/core/constants/config.ts (2)
ACCEPTED_IMAGE_MIME_TYPES(11-11)ACCEPTED_ATTACHMENT_MIME_TYPES(13-62)packages/editor/src/core/types/editor.ts (1)
TEditorCommands(24-49)
🪛 Biome (1.9.4)
packages/editor/src/core/plugins/markdown-clipboard.ts
[error] 67-67: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (8)
packages/editor/src/core/plugins/drop.ts (5)
1-7: Good clean imports with appropriate organizationThe imports are well-organized with clear categorization using comments, bringing in exactly what's needed for the plugin functionality. The separation of constants and types aids readability.
8-73: Well-structured drop and paste handler implementationThe DropHandlerPlugin implements both drop and paste event handling with proper validation and filtering of files. The logic follows best practices by:
- Checking editor.isEditable before processing
- Preventing default browser behavior when handling files
- Filtering files against accepted MIME types
- Using appropriate positioning logic for paste vs. drop
75-81: Well-typed arguments using TypeScript features appropriatelyGood use of TypeScript's Extract utility type to constrain the optional type parameter to only valid values ("attachment" | "image") from TEditorCommands. The type definition is comprehensive and properly documents the expected parameters.
109-110: Missing implementation for attachment file typeThere's an empty conditional block for handling attachment file types, suggesting this functionality is not yet implemented.
Is this intentional or should there be an implementation for attachment files similar to the image implementation? If this is part of planned incremental development, consider adding a TODO comment to make it clear that this is a pending implementation.
} else if (fileType === "attachment") { + // TODO: Implement attachment insertion }
83-118: Solid implementation of file insertion with safety checksThe insertFilesSafely function is well-implemented with:
- Position clamping to prevent out-of-bounds insertion
- Type checking and validation
- Error handling to prevent crashes
- Sequential insertion with position management
This makes the file handling robust against unexpected inputs or states.
packages/editor/src/core/plugins/markdown-clipboard.ts (3)
1-8: Well-structured plugin initialization.The plugin initialization is clean and follows best practices for ProseMirror plugins by using a unique PluginKey.
9-16: Clipboard serialization logic looks good.The clipboard text serializer appropriately handles node selection cases by checking
openStartandopenEndvalues, which correctly determines if the entire node was selected.
29-37: Edge case handling for tables looks good.The special handling for single row/cell versus multiple rows/cells is a nice touch for better user experience when copying table content.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/editor/src/core/helpers/image-helpers.ts (2)
18-18: Type assertion may not be type-safeThe type assertion
node.type.name as CORE_EXTENSIONSassumes thatnode.type.nameis always a value from theCORE_EXTENSIONSenum, which might not be guaranteed. Consider using a type guard or checking if the value exists in the enum.- if ([CORE_EXTENSIONS.IMAGE, CORE_EXTENSIONS.CUSTOM_IMAGE].includes(node.type.name as CORE_EXTENSIONS)) { + const nodeTypeName = node.type.name; + if (nodeTypeName === CORE_EXTENSIONS.IMAGE || nodeTypeName === CORE_EXTENSIONS.CUSTOM_IMAGE) {
15-32: Error handling approach is good, but consider additional improvementsThe function properly catches and logs errors during image restoration, but it could be enhanced to provide more contextual information and structured error handling.
Consider enhancing error logging to include the specific image source that failed:
try { await restoreImageFn(src); } catch (error) { - console.error("Error restoring image: ", error); + console.error(`Error restoring image from source "${src}":`, error); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (6)
packages/editor/src/core/extensions/custom-image/custom-image.ts(3 hunks)packages/editor/src/core/extensions/image/extension.tsx(1 hunks)packages/editor/src/core/extensions/utility.ts(1 hunks)packages/editor/src/core/helpers/image-helpers.ts(1 hunks)packages/editor/src/core/plugins/file/delete.ts(1 hunks)packages/editor/src/core/plugins/file/restore.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/editor/src/core/plugins/file/restore.ts
- packages/editor/src/core/plugins/file/delete.ts
- packages/editor/src/core/extensions/image/extension.tsx
- packages/editor/src/core/extensions/custom-image/custom-image.ts
- packages/editor/src/core/extensions/utility.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/editor/src/core/helpers/image-helpers.ts (1)
packages/editor/src/core/types/config.ts (1)
TFileHandler(6-18)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (3)
packages/editor/src/core/helpers/image-helpers.ts (3)
7-14: Good documentation practiceThe code is well-documented with clear explanation of its purpose and why it should be preserved. This is excellent for maintainability and helps future developers understand the rationale behind this implementation.
1-5: Clean imports structureThe imports are well-organized and properly categorized with comments. This makes the code more readable and maintainable.
16-23: Efficient use of Set for duplicate preventionUsing a Set to collect image sources ensures that duplicate images aren't processed multiple times, which is an efficient approach.
Palanikannan1437
force-pushed
the
refactor/file-plugins
branch
from
9db9ddd to
e059bcc
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
packages/editor/src/core/extensions/work-item-embed/widget-node.tsx (2)
20-20: Avoid usinganytype.Using
anyforissuePropsloses type safety. Consider using a more specific type from TipTap or creating a custom interface.- return ReactNodeViewRenderer((issueProps: any) => ( + return ReactNodeViewRenderer((issueProps: { node: { attrs: { entity_identifier: string, project_identifier?: string, workspace_identifier?: string } } }) => (
22-26: Terminology inconsistency between props and component name.The extension is named
WorkItemEmbedbut the props still use "issue" terminology. Consider aligning the naming for consistency.{props.widgetCallback({ - issueId: issueProps.node.attrs.entity_identifier, - projectId: issueProps.node.attrs.project_identifier, - workspaceSlug: issueProps.node.attrs.workspace_identifier, + workItemId: issueProps.node.attrs.entity_identifier, + projectId: issueProps.node.attrs.project_identifier, + workspaceSlug: issueProps.node.attrs.workspace_identifier, })}You would also need to update the Props type definition accordingly.
packages/editor/src/core/extensions/work-item-embed/extension-config.ts (2)
32-38: Tag name inconsistency with extension name.The extension is named
WORK_ITEM_EMBEDbut usesissue-embed-componentas the HTML tag. Consider updating the tag name to match the new terminology.parseHTML() { return [ { - tag: "issue-embed-component", + tag: "work-item-embed-component", }, ]; },Remember to also update the
renderHTMLmethod below to use the same tag name.
40-42: Tag name inconsistency with extension name.The
renderHTMLmethod should be updated to match the tag name suggested in theparseHTMLmethod.renderHTML({ HTMLAttributes }) { - return ["issue-embed-component", mergeAttributes(HTMLAttributes)]; + return ["work-item-embed-component", mergeAttributes(HTMLAttributes)]; },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
packages/editor/src/core/components/editors/document/collaborative-editor.tsx(2 hunks)packages/editor/src/core/components/editors/document/read-only-editor.tsx(2 hunks)packages/editor/src/core/extensions/core-without-props.ts(2 hunks)packages/editor/src/core/extensions/index.ts(2 hunks)packages/editor/src/core/extensions/issue-embed/index.ts(0 hunks)packages/editor/src/core/extensions/issue-embed/issue-embed-without-props.ts(0 hunks)packages/editor/src/core/extensions/issue-embed/widget-node.tsx(0 hunks)packages/editor/src/core/extensions/work-item-embed/extension-config.ts(1 hunks)packages/editor/src/core/extensions/work-item-embed/index.ts(1 hunks)packages/editor/src/core/extensions/work-item-embed/widget-node.tsx(1 hunks)
💤 Files with no reviewable changes (3)
- packages/editor/src/core/extensions/issue-embed/index.ts
- packages/editor/src/core/extensions/issue-embed/issue-embed-without-props.ts
- packages/editor/src/core/extensions/issue-embed/widget-node.tsx
✅ Files skipped from review due to trivial changes (4)
- packages/editor/src/core/extensions/work-item-embed/index.ts
- packages/editor/src/core/components/editors/document/collaborative-editor.tsx
- packages/editor/src/core/components/editors/document/read-only-editor.tsx
- packages/editor/src/core/extensions/index.ts
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/editor/src/core/extensions/work-item-embed/widget-node.tsx (1)
packages/editor/src/core/extensions/work-item-embed/extension-config.ts (1)
WorkItemEmbedExtensionConfig(5-43)
packages/editor/src/core/extensions/core-without-props.ts (1)
packages/editor/src/core/extensions/work-item-embed/extension-config.ts (1)
WorkItemEmbedExtensionConfig(5-43)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (6)
packages/editor/src/core/extensions/core-without-props.ts (3)
8-10: Improved import organization.The separation of imports into categories (plane editor imports vs extensions) improves code readability and maintainability. This clean separation makes it easier to understand the dependency structure.
23-23: Appropriate replacement of issue embed with work item embed.The import of
WorkItemEmbedExtensionConfigcorrectly replaces the previousIssueWidgetWithoutPropsas part of the broader refactoring to generalize file handling in the editor.
107-107:Details
✅ Verification successful
Verify behavior change in DocumentEditorExtensionsWithoutProps.
The previous implementation likely invoked a function (e.g.,
IssueWidgetWithoutProps()), while the new version directly references the extension config object. Ensure this change doesn't affect the editor initialization.
🏁 Script executed:
#!/bin/bash # Look for references to DocumentEditorExtensionsWithoutProps to ensure consumers handle the new non-function format rg "DocumentEditorExtensionsWithoutProps" --type ts -A 2 -B 2Length of output: 1805
Confirm Array-Only Export Doesn’t Break Initialization
I checked where
DocumentEditorExtensionsWithoutPropsis consumed—both in
packages/editor/src/core/helpers/yjs-utils.tsandlive/src/core/helpers/page.tsThey merely spread this array alongside
CoreEditorExtensionsWithoutPropsintoDOCUMENT_EDITOR_EXTENSIONS, which is then passed unchanged intogetSchemaand the editor setup. No code assumes it’s a function or invokes it.• No invocation sites for
DocumentEditorExtensionsWithoutProps
• All downstream usages spread it as an array of config objectsSince Tiptap’s API expects extension config objects (not factory functions) here, the change is safe.
packages/editor/src/core/extensions/work-item-embed/widget-node.tsx (1)
5-15: Well-defined props interface.The Props interface clearly defines the contract for the extension, specifying the parameters required by the widgetCallback function. This provides good documentation for consumers of the component.
packages/editor/src/core/extensions/work-item-embed/extension-config.ts (2)
5-11: Well-structured node extension configuration.The extension is properly configured as a block-level, atomic, selectable, and draggable node, which is appropriate for an embedded entity in a rich text editor.
12-30: Comprehensive attribute definitions.The attributes cover all necessary data for identifying and rendering an embedded work item, including entity, project, and workspace identifiers.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/editor/src/core/plugins/markdown-clipboard.ts (2)
31-39: Consider handling empty tables more explicitly.The code differentiates between single-row/cell and multi-row/cell tables, but doesn't explicitly handle empty tables. While the current implementation would likely work, explicit handling could improve maintainability.
if (isTableRow) { const rowsCount = slice.content?.childCount || 0; const cellsCount = slice.content?.firstChild?.content?.childCount || 0; + // If table is empty, return an empty string + if (rowsCount === 0) { + return ""; + } if (rowsCount === 1 || cellsCount === 1) { return processTableContent(slice.content); } else { return markdownSerializer.serialize(slice.content); } }
22-28: Optional chaining could improve robustness in table processing.Using optional chaining in the table processing logic would make the code more resilient to unexpected content structures.
- tableNode.content?.forEach?.((tableRowNode: Node | Fragment) => { - tableRowNode.content?.forEach?.((cell: Node) => { + tableNode.content?.forEach?.((tableRowNode: Node | Fragment) => { + tableRowNode.content?.forEach?.((cell: Node) => { const cellContent = cell.content ? markdownSerializer.serialize(cell.content) : ""; result += cellContent + "\n"; }); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
packages/editor/src/core/components/editors/editor-container.tsx(3 hunks)packages/editor/src/core/components/menus/block-menu.tsx(2 hunks)packages/editor/src/core/components/menus/bubble-menu/link-selector.tsx(2 hunks)packages/editor/src/core/components/menus/bubble-menu/root.tsx(2 hunks)packages/editor/src/core/components/menus/menu-items.ts(4 hunks)packages/editor/src/core/extensions/code/code-block.ts(5 hunks)packages/editor/src/core/extensions/custom-image/components/image-uploader.tsx(5 hunks)packages/editor/src/core/extensions/custom-link/extension.tsx(3 hunks)packages/editor/src/core/extensions/custom-link/helpers/clickHandler.ts(1 hunks)packages/editor/src/core/extensions/custom-list-keymap/list-helpers.ts(3 hunks)packages/editor/src/core/extensions/custom-list-keymap/list-keymap.ts(3 hunks)packages/editor/src/core/extensions/enter-key-extension.tsx(1 hunks)packages/editor/src/core/extensions/extensions.tsx(4 hunks)packages/editor/src/core/extensions/keymap.tsx(2 hunks)packages/editor/src/core/extensions/mentions/mention-node-view.tsx(1 hunks)packages/editor/src/core/extensions/quote.tsx(2 hunks)packages/editor/src/core/extensions/slash-commands/root.tsx(3 hunks)packages/editor/src/core/extensions/table/table/table-controls.ts(3 hunks)packages/editor/src/core/extensions/table/table/table-view.tsx(2 hunks)packages/editor/src/core/extensions/table/table/table.ts(4 hunks)packages/editor/src/core/extensions/table/table/utilities/delete-table-when-all-cells-selected.ts(2 hunks)packages/editor/src/core/extensions/table/table/utilities/insert-line-above-table-action.ts(2 hunks)packages/editor/src/core/extensions/table/table/utilities/insert-line-below-table-action.ts(2 hunks)packages/editor/src/core/extensions/work-item-embed/extension.tsx(1 hunks)packages/editor/src/core/extensions/work-item-embed/index.ts(1 hunks)packages/editor/src/core/helpers/common.ts(2 hunks)packages/editor/src/core/helpers/editor-commands.ts(2 hunks)packages/editor/src/core/helpers/insert-empty-paragraph-at-node-boundary.ts(3 hunks)packages/editor/src/core/hooks/use-editor.ts(7 hunks)packages/editor/src/core/plugins/drag-handle.ts(8 hunks)packages/editor/src/core/plugins/markdown-clipboard.ts(1 hunks)
✅ Files skipped from review due to trivial changes (14)
- packages/editor/src/core/extensions/quote.tsx
- packages/editor/src/core/extensions/custom-list-keymap/list-keymap.ts
- packages/editor/src/core/extensions/table/table/table-view.tsx
- packages/editor/src/core/extensions/keymap.tsx
- packages/editor/src/core/extensions/table/table/utilities/insert-line-above-table-action.ts
- packages/editor/src/core/extensions/custom-link/helpers/clickHandler.ts
- packages/editor/src/core/components/menus/menu-items.ts
- packages/editor/src/core/extensions/table/table/utilities/insert-line-below-table-action.ts
- packages/editor/src/core/extensions/table/table/table-controls.ts
- packages/editor/src/core/extensions/table/table/utilities/delete-table-when-all-cells-selected.ts
- packages/editor/src/core/helpers/insert-empty-paragraph-at-node-boundary.ts
- packages/editor/src/core/extensions/work-item-embed/extension.tsx
- packages/editor/src/core/extensions/custom-list-keymap/list-helpers.ts
- packages/editor/src/core/helpers/editor-commands.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- packages/editor/src/core/extensions/work-item-embed/index.ts
- packages/editor/src/core/extensions/extensions.tsx
- packages/editor/src/core/components/editors/editor-container.tsx
- packages/editor/src/core/extensions/enter-key-extension.tsx
- packages/editor/src/core/extensions/custom-image/components/image-uploader.tsx
- packages/editor/src/core/extensions/slash-commands/root.tsx
- packages/editor/src/core/extensions/custom-link/extension.tsx
- packages/editor/src/core/extensions/table/table/table.ts
- packages/editor/src/core/extensions/code/code-block.ts
- packages/editor/src/core/hooks/use-editor.ts
🧰 Additional context used
🪛 Biome (1.9.4)
packages/editor/src/core/plugins/markdown-clipboard.ts
[error] 69-69: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (23)
packages/editor/src/core/extensions/mentions/mention-node-view.tsx (1)
21-21: Well done adding a fallback for entity_identifierThis defensive programming approach prevents potential
undefinedvalues from being passed torenderComponent, making the code more robust and consistent with howentity_nameis already handled on line 22.packages/editor/src/core/components/menus/bubble-menu/root.tsx (2)
21-21: Good addition of CORE_EXTENSIONS importAdding this import supports the refactoring of hardcoded string literals to centralized constants.
94-95: Good replacement of string literals with constantsReplacing hardcoded strings with
CORE_EXTENSIONSconstants improves maintainability and reduces the risk of typos when checking for active node types.packages/editor/src/core/components/menus/bubble-menu/link-selector.tsx (2)
4-7: Good organization of imports with proper commentsThe clear separation of imports with appropriate comments improves code readability.
48-48: Good replacement of string literal with CORE_EXTENSIONS constantUsing
CORE_EXTENSIONS.CUSTOM_LINKinstead of the hardcoded string "link" improves code maintainability and consistency across the codebase.packages/editor/src/core/helpers/common.ts (2)
2-5: Good organization of imports with proper commentsThe clear separation of imports with appropriate comments improves code readability.
96-96: Good replacement of string literal with CORE_EXTENSIONS constantUsing
CORE_EXTENSIONS.PARAGRAPHinstead of the hardcoded string "paragraph" improves consistency and maintainability, aligning with the broader refactoring effort.packages/editor/src/core/components/menus/block-menu.tsx (2)
3-6: Import organization improvementThe imports have been reorganized with a clear separation for constants, and the
CORE_EXTENSIONSimport has been added to support the string literal replacements.
107-108: Improved maintainability with CORE_EXTENSIONS constantsReplacing hardcoded string literals (
"image"and"imageComponent") withCORE_EXTENSIONS.IMAGEandCORE_EXTENSIONS.CUSTOM_IMAGEconstants improves code maintainability and ensures consistency across the codebase.packages/editor/src/core/plugins/drag-handle.ts (9)
5-6: Import organization improvementAdded import for
CORE_EXTENSIONSconstants, which supports the broader refactoring goal of replacing string literals with centralized constants.
137-137: Enhanced type safetyExplicitly typing
scrollAnimationFrameasnumber | nullimproves type safety and makes the variable's purpose clearer.
147-150: Improved error handlingThe safe destructuring of
handleNodeSelectionresult with fallback handling enhances robustness by preventing potential errors when the function returns undefined.
304-304: Consistent use of CORE_EXTENSIONS constantsReplacing hardcoded string
"listItem"withCORE_EXTENSIONS.LIST_ITEMconstant improves code maintainability.
312-312: Consistent use of CORE_EXTENSIONS constantsReplacing hardcoded string
"listItem"withCORE_EXTENSIONS.LIST_ITEMconstant improves code maintainability.
355-356: Variable naming consistencyRenamed
sublisttosubListfor better adherence to camelCase naming convention.
393-395: Improved type safety with CORE_EXTENSIONSUsing
CORE_EXTENSIONSenum for type checking and explicitly casting withas CORE_EXTENSIONSenhances type safety.
413-415: Improved type safety with CORE_EXTENSIONSUsing
CORE_EXTENSIONSenum for type checking and explicitly casting withas CORE_EXTENSIONSenhances type safety.
422-422: Defensive programming improvementAdded a null check for
event.dataTransferbefore accessing its properties, preventing potential runtime errors.packages/editor/src/core/plugins/markdown-clipboard.ts (5)
1-6: LGTM! Good use of constants for extension types.The imports are clean and well-organized. Using the centralized
CORE_EXTENSIONSconstant instead of hardcoded string literals is a great practice that improves maintainability.
12-14: LGTM! The code correctly retrieves the markdown serializer and checks for table rows.The implementation properly accesses the markdown serializer from editor storage and uses the
CORE_EXTENSIONSconstant for type name comparison.
20-29: LGTM! Table content processing helper function is well-structured.The function neatly handles table content by iterating through rows and cells, serializing each cell's content and joining them with newlines.
41-61: LGTM! The traversal logic handles nested structures effectively.The traversal function correctly navigates through node hierarchies, with special handling for list items. This ensures proper context is maintained when serializing to markdown.
7-80: LGTM! Well-structured clipboard serialization plugin.The plugin effectively handles various edge cases for clipboard serialization, including tables, list items, and text nodes. The logic is well-organized with distinct handling for different scenarios.
🧰 Tools
🪛 Biome (1.9.4)
[error] 69-69: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/editor/src/core/components/links/link-edit-view.tsx (1)
54-56: Good error handling improvement!Adding error handling to the link removal cleanup logic prevents silent failures and improves debugging. This aligns well with the PR's goal of improving error handling throughout the codebase.
Consider enhancing the error message with more context information for better debugging:
- console.error("Error removing link", e); + console.error("Error removing link during component cleanup", e);packages/editor/src/core/extensions/table/table/table-view.tsx (1)
33-36: Proper naming convention applied for colWidth attribute.The attribute was correctly renamed from
colwidthtocolWidthto maintain consistent camelCase naming conventions.However, there's a potential improvement on line 36:
- const hasWidth = overrideCol === col ? overrideValue : colWidth && colWidth[j]; + const hasWidth = overrideCol === col ? overrideValue : colWidth?.[j];Using optional chaining (
?.) instead of the&&operator would be more concise and is the modern approach for safely accessing potentially undefined properties.🧰 Tools
🪛 Biome (1.9.4)
[error] 36-36: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (33)
packages/editor/src/ce/extensions/document-extensions.tsx(0 hunks)packages/editor/src/core/components/editors/link-view-container.tsx(1 hunks)packages/editor/src/core/components/links/link-edit-view.tsx(1 hunks)packages/editor/src/core/components/menus/bubble-menu/node-selector.tsx(1 hunks)packages/editor/src/core/extensions/callout/block.tsx(1 hunks)packages/editor/src/core/extensions/callout/logo-selector.tsx(1 hunks)packages/editor/src/core/extensions/callout/types.ts(1 hunks)packages/editor/src/core/extensions/callout/utils.ts(4 hunks)packages/editor/src/core/extensions/code/code-block-node-view.tsx(2 hunks)packages/editor/src/core/extensions/code/code-block.ts(5 hunks)packages/editor/src/core/extensions/code/lowlight-plugin.ts(1 hunks)packages/editor/src/core/extensions/enter-key.tsx(1 hunks)packages/editor/src/core/extensions/headers.ts(2 hunks)packages/editor/src/core/extensions/image/extension.tsx(1 hunks)packages/editor/src/core/extensions/index.ts(2 hunks)packages/editor/src/core/extensions/keymap.tsx(3 hunks)packages/editor/src/core/extensions/mentions/mentions-list-dropdown.tsx(3 hunks)packages/editor/src/core/extensions/mentions/utils.ts(2 hunks)packages/editor/src/core/extensions/slash-commands/command-items-list.tsx(1 hunks)packages/editor/src/core/extensions/slash-commands/command-menu.tsx(2 hunks)packages/editor/src/core/extensions/slash-commands/root.tsx(4 hunks)packages/editor/src/core/extensions/table/table-cell.ts(1 hunks)packages/editor/src/core/extensions/table/table-cell/index.ts(0 hunks)packages/editor/src/core/extensions/table/table-header.ts(1 hunks)packages/editor/src/core/extensions/table/table-header/index.ts(0 hunks)packages/editor/src/core/extensions/table/table-row.ts(1 hunks)packages/editor/src/core/extensions/table/table-row/index.ts(0 hunks)packages/editor/src/core/extensions/table/table/table-controls.ts(4 hunks)packages/editor/src/core/extensions/table/table/table-view.tsx(4 hunks)packages/editor/src/core/extensions/table/table/table.ts(5 hunks)packages/editor/src/core/helpers/common.ts(4 hunks)packages/editor/src/core/helpers/editor-commands.ts(3 hunks)packages/editor/src/core/hooks/use-collaborative-editor.ts(2 hunks)
💤 Files with no reviewable changes (4)
- packages/editor/src/core/extensions/table/table-row/index.ts
- packages/editor/src/ce/extensions/document-extensions.tsx
- packages/editor/src/core/extensions/table/table-header/index.ts
- packages/editor/src/core/extensions/table/table-cell/index.ts
✅ Files skipped from review due to trivial changes (11)
- packages/editor/src/core/extensions/callout/logo-selector.tsx
- packages/editor/src/core/extensions/code/lowlight-plugin.ts
- packages/editor/src/core/extensions/callout/block.tsx
- packages/editor/src/core/components/editors/link-view-container.tsx
- packages/editor/src/core/components/menus/bubble-menu/node-selector.tsx
- packages/editor/src/core/extensions/mentions/utils.ts
- packages/editor/src/core/extensions/table/table-header.ts
- packages/editor/src/core/extensions/slash-commands/command-items-list.tsx
- packages/editor/src/core/extensions/table/table-cell.ts
- packages/editor/src/core/extensions/table/table-row.ts
- packages/editor/src/core/extensions/index.ts
🚧 Files skipped from review as they are similar to previous changes (9)
- packages/editor/src/core/helpers/common.ts
- packages/editor/src/core/extensions/headers.ts
- packages/editor/src/core/extensions/keymap.tsx
- packages/editor/src/core/helpers/editor-commands.ts
- packages/editor/src/core/extensions/table/table/table-controls.ts
- packages/editor/src/core/extensions/table/table/table.ts
- packages/editor/src/core/extensions/image/extension.tsx
- packages/editor/src/core/extensions/slash-commands/root.tsx
- packages/editor/src/core/extensions/code/code-block.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/editor/src/core/extensions/enter-key.tsx (1)
packages/editor/src/core/helpers/get-extension-storage.ts (1)
getExtensionStorage(5-8)
🪛 Biome (1.9.4)
packages/editor/src/core/extensions/table/table/table-view.tsx
[error] 36-36: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (20)
packages/editor/src/core/extensions/code/code-block-node-view.tsx (2)
8-8: Import organization looks good.The
useStateimport is now properly organized with other React imports. This maintains consistency with standard React import conventions.
30-30: Good simplification of the catch block.Removing the unused error parameter in the catch block improves code cleanliness while maintaining the same error handling functionality.
packages/editor/src/core/extensions/callout/types.ts (1)
23-23: Good type improvementChanging
BACKGROUNDattribute fromstringtostring | undefinedimproves type safety by explicitly indicating that this value can be undefined. This aligns with other attribute definitions in the file that already follow this pattern, creating more consistent typing throughout the codebase.Make sure that any components or functions using this attribute handle the potential undefined value appropriately with proper null checks.
packages/editor/src/core/hooks/use-collaborative-editor.ts (2)
3-3: Import ordering adjustment looks good.The updated import order places the React hooks together, which improves code organization and readability.
79-79: Good type safety improvement with boolean coercion.Using
!!editableexplicitly coerces the value to a boolean, which ensures type consistency when passing the editable state to the editor. This is a good practice that prevents potential type-related issues and aligns with the broader refactoring goals for improved type safety.packages/editor/src/core/extensions/mentions/mentions-list-dropdown.tsx (2)
64-66: Added null check to prevent setting state with undefined valuesGood defensive programming practice! This conditional check ensures
setSelectedIndexis only called whennewIndexis truthy, preventing potential React state updates with undefined/null values.
84-86: Added null check to prevent setting state with undefined valuesNice improvement! Adding this conditional guard ensures that
setSectionsis only called with a validsectionsResponse, preventing potential state updates with null/undefined values and enhancing component stability.packages/editor/src/core/extensions/slash-commands/command-menu.tsx (1)
106-108: Added null check to prevent setting state with undefined valuesGood defensive programming! This conditional check ensures that
setSelectedIndexis only called whennewIndexexists, preventing potential React state updates with undefined/null values. This matches the pattern applied in other components like MentionsListDropdown.packages/editor/src/core/extensions/enter-key.tsx (4)
2-5: Good addition of imports for centralized constants and helpers.The use of a centralized CORE_EXTENSIONS enum and a getExtensionStorage helper function improves code maintainability and consistency across the codebase.
9-9: Excellent replacement of hardcoded string with constant.Replacing the hardcoded string "enterKey" with CORE_EXTENSIONS.ENTER_KEY reduces the risk of typos and makes future refactoring easier.
14-16: Improved mention detection and callback handling.The changes here provide two improvements:
- Using getExtensionStorage to access mention state instead of direct storage access
- Using optional chaining for the callback invocation, which is more concise
This follows better TypeScript practices and improves consistency with the rest of the codebase.
24-25: Good use of constants for list and task item extensions.Replacing hardcoded strings with constants from CORE_EXTENSIONS ensures type safety and consistency across the codebase.
packages/editor/src/core/extensions/callout/utils.ts (4)
14-16: Good improvement: Changed default values from null to undefined.Changing the default values for
data-icon-coloranddata-icon-namefromnulltoundefinedis more consistent with TypeScript best practices. This helps with type safety and better represents the absence of an optional value.
18-18: Good improvement: Changed default value from null to undefined.Changing the default value for
data-backgroundfromnulltoundefinedmaintains consistency with the other attribute changes and follows TypeScript best practices.
34-34: Good defensive programming: Added null handling for localStorage.The addition of the null coalescing operator (
??) with an empty string fallback ensures thatsanitizeHTMLalways receives a string input instead of potentially receivingnullwhen the item doesn't exist in localStorage. This is a good defensive programming practice.
71-71: Good defensive programming: Added null handling for localStorage.Similar to the other localStorage change, adding the null coalescing operator (
??) here ensuressanitizeHTMLreceives a valid string input rather thannull. This prevents potential issues with the sanitization function.packages/editor/src/core/extensions/table/table/table-view.tsx (4)
2-9: Good standardization of imports and constants.The code now uses proper imports with specific type references and introduces the centralized
CORE_EXTENSIONSenum, aligning with the project's refactoring goals to replace hardcoded string literals throughout the codebase.
88-92: Good use of constants instead of string literals.Replacing the hardcoded string
"tableCell"withCORE_EXTENSIONS.TABLE_CELLimproves code maintainability and reduces the risk of typos.
107-109: Proper use of CORE_EXTENSIONS constant for table row.The hardcoded string
"tableRow"was correctly replaced withCORE_EXTENSIONS.TABLE_ROW, maintaining consistency with the refactoring approach.
112-114: Consistent use of constants for node type checking.The code properly uses
CORE_EXTENSIONS.TABLE_ROWto maintain consistency with the refactoring to use constants instead of string literals.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
packages/editor/src/core/extensions/read-only-extensions.ts (1)
27-28: Confirm side-effects of importingUtilityExtensionin read-only context
UtilityExtensionbundles several ProseMirror plugins (file plugins, markdown-clipboard, codemark, drop-handler).
In a read-only editor most of these bring little value but still attach DOM listeners and increase bundle size.Consider exposing a lightweight variant (
UtilityExtension.readOnly) that:
- skips
DropHandlerPlugin& the file-delete plugin whenisEditable === false,- retains only the restore-image logic that you genuinely need here.
This trims unnecessary work during render and avoids surprises if any of those plugins emit transactions in the future.
packages/editor/src/core/extensions/enter-key.ts (2)
14-17: Handle absent Mention extension more explicitly
getExtensionStorage(...).mentionsOpenis safely optional-chained, but the intent is clearer if the fallback is explicit:-const isMentionOpen = getExtensionStorage(this.editor, CORE_EXTENSIONS.MENTION)?.mentionsOpen; +const { mentionsOpen = false } = + getExtensionStorage(this.editor, CORE_EXTENSIONS.MENTION) ?? {}; +const isMentionOpen = mentionsOpen;This avoids the intermediate
undefinedvalue and communicates the expected boolean type.
24-26: EnsuresplitListItemreceives aNodeType
commands.splitListItemultimately expects aNodeType; passing a string relies on an internal cast.
Now that you already depend onCORE_EXTENSIONS, consider:() => commands.splitListItem(this.editor.schema.nodes[CORE_EXTENSIONS.LIST_ITEM]),This keeps typings strict and future-proof if tiptap changes its overloads.
packages/editor/src/core/extensions/extensions.ts (2)
36-37: Missing tree-shaking forgetExtensionStorage
getExtensionStorageis only referenced inside the placeholder callback.
If the callback is removed by consumers (e.g. viaPlaceholder.configure({})override in downstream code) the import still ends up in the bundle.Not critical, but wrapping the helper usage in a small inline util (or lazy-importing inside the callback) can shave a few bytes.
146-150: Guard placeholder logic against undefined storage
getExtensionStorage(editor, CORE_EXTENSIONS.UTILITY)?.uploadInProgressworks, but when the extension is disabled viadisabledExtensionsthe constant still resolves to"utility"; the helper will returnundefined.That’s fine, yet subsequent refactors might access deeper props.
Consider normalising:const { uploadInProgress = false } = getExtensionStorage(editor, CORE_EXTENSIONS.UTILITY) ?? {};This guarantees a boolean and keeps TypeScript happy.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
packages/editor/src/core/constants/extension.ts(1 hunks)packages/editor/src/core/extensions/enter-key.ts(1 hunks)packages/editor/src/core/extensions/extensions.ts(4 hunks)packages/editor/src/core/extensions/headings-list.ts(2 hunks)packages/editor/src/core/extensions/index.ts(1 hunks)packages/editor/src/core/extensions/keymap.ts(3 hunks)packages/editor/src/core/extensions/quote.ts(2 hunks)packages/editor/src/core/extensions/read-only-extensions.ts(2 hunks)packages/editor/src/core/extensions/side-menu.ts(2 hunks)
✅ Files skipped from review due to trivial changes (4)
- packages/editor/src/core/extensions/side-menu.ts
- packages/editor/src/core/extensions/quote.ts
- packages/editor/src/core/extensions/headings-list.ts
- packages/editor/src/core/extensions/keymap.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/editor/src/core/extensions/index.ts
- packages/editor/src/core/constants/extension.ts
🧰 Additional context used
🧬 Code Graph Analysis (3)
packages/editor/src/core/extensions/read-only-extensions.ts (1)
packages/editor/src/core/extensions/utility.ts (1)
UtilityExtension(31-71)
packages/editor/src/core/extensions/enter-key.ts (1)
packages/editor/src/core/helpers/get-extension-storage.ts (1)
getExtensionStorage(5-8)
packages/editor/src/core/extensions/extensions.ts (2)
packages/editor/src/core/helpers/get-extension-storage.ts (1)
getExtensionStorage(5-8)packages/editor/src/core/extensions/utility.ts (1)
UtilityExtension(31-71)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: CodeQL
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (4)
packages/editor/src/core/extensions/read-only-extensions.ts (1)
129-132: Pass a stablefileHandlerreference
UtilityExtensionstores a reference tofileHandlerinside its storage.
If the parent component ever re-creates the handler object, TipTap will keep the stale instance because extensions are not recreated after the editor mounts.If the handler can change during the lifetime of the editor, pass only the immutable subset (e.g.
restore,assetsUploadStatus) or memoise the object before feeding it to the editor.
Otherwise, document that the handler must be stable.packages/editor/src/core/extensions/enter-key.ts (1)
2-6: 👍 Clear separation of concerns with constants & helperReplacing magic strings with
CORE_EXTENSIONSconstants and usinggetExtensionStoragemakes the codebase safer against future renames.packages/editor/src/core/extensions/extensions.ts (2)
10-12: Constants import improves resilienceGood move replacing string literals with
CORE_EXTENSIONS; this reduces the risk of typos across large files.
171-174: Order ofUtilityExtensioncan influence other extensions
UtilityExtensionhaspriority: 1000, so TipTap places it before most core extensions no matter the array order.
However, extensions added later in the array may rely on custom plugins injected byUtilityExtension(e.g. clipboard transformer). Placing it earlier in the list improves DX when scanning the file, but you could also drop it to the top for clarity.No action required—just pointing out the implicit behaviour.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (5)
live/package.json(1 hunks)packages/decorators/package.json(1 hunks)packages/hooks/package.json(1 hunks)packages/ui/package.json(1 hunks)packages/utils/package.json(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- packages/decorators/package.json
- live/package.json
- packages/utils/package.json
- packages/ui/package.json
Palanikannan1437
force-pushed
the
refactor/file-plugins
branch
from
036798f to
c5da685
Compare
Palanikannan1437
force-pushed
the
refactor/file-plugins
branch
from
c5da685 to
c44a209
Compare
Palanikannan1437
force-pushed
the
refactor/file-plugins
branch
from
e4f22f6 to
ed6d258
Compare
Palanikannan1437
force-pushed
the
refactor/file-plugins
branch
from
ed6d258 to
46b951c
Compare
Palanikannan1437
force-pushed
the
refactor/file-plugins
branch
from
954dae2 to
7fcb798
Compare
* refactor: file plugins and types * refactor: image extension storage types * chore: update meta tag name * chore: extension fileset storage key * fix: build errors * refactor: utility extension * refactor: file plugins * chore: remove standalone plugin extensions * chore: refactoring out onCreate into a common utility * refactor: work item embed extension * chore: use extension enums * fix: errors and warnings * refactor: rename extension files * fix: tsup reloading issue * fix: image upload types and heading types * fix: file plugin object reference * fix: iseditable is hard coded * fix: image extension names * fix: collaborative editor editable value * chore: add constants for editor meta as well --------- Co-authored-by: Palanikannan M <akashmalinimurugu@gmail.com>
3 participants
Description
This PR refactors the following-
Type of Change
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Refactor
Chores
tsuppackage versions to fixed 8.3.0 across multiple packages for consistency.