[WIKI-788] regression: copy markdown option #8200
Conversation
aaryan610
added
🐛bug
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
WalkthroughThe changes refactor how Markdown is generated in the editor. Instead of retrieving Markdown directly from storage, the implementation now converts HTML to Markdown using a utility function and editor metadata. The ref helpers and use-editor hook are updated to propagate metadata extraction logic throughout the flow. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR fixes a regression in the "Copy markdown" option by implementing new markdown conversion rules. The change migrates from the old TipTap markdown storage approach to a new unified conversion method that uses metadata extraction.
Key Changes:
- Pass
getEditorMetaDatafunction through the editor ref helpers chain - Replace the legacy markdown storage retrieval with the new
convertHTMLToMarkdownutility - Ensure consistency with the clipboard markdown conversion pattern
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/editor/src/core/hooks/use-editor.ts | Passes getEditorMetaData to getEditorRefHelpers and adds it to the dependency array of useImperativeHandle |
| packages/editor/src/core/helpers/editor-ref.ts | Updates getMarkDown() to use convertHTMLToMarkdown with metadata extraction, replacing the old markdown storage approach |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/editor/src/core/helpers/editor-ref.ts (1)
13-13: Consider usingimport typefor type-only imports.Per coding guidelines, type-only imports should use
import typefor proper tree-shaking and to respectverbatimModuleSyntax:Apply this diff:
-import type { EditorRefApi, IEditorProps, TEditorCommands } from "@/types"; +import type { EditorRefApi, TEditorCommands } from "@/types"; +import type { IEditorProps } from "@/types";Or consolidate all as type imports (if not already):
-import type { EditorRefApi, IEditorProps, TEditorCommands } from "@/types"; +import type { EditorRefApi, IEditorProps, TEditorCommands } from "@/types";Note: The current import is already using
import type, so this is already correct. The comment above is only applicable if any of these were regular imports.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/editor/src/core/helpers/editor-ref.ts(2 hunks)packages/editor/src/core/hooks/use-editor.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,mts,cts}
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/*.{ts,tsx,mts,cts}: Useconsttype parameters for more precise literal inference in TypeScript 5.0+
Use thesatisfiesoperator to validate types without widening them
Leverage inferred type predicates to reduce the need for explicitisreturn types in filter/check functions
UseNoInfer<T>utility to block inference for specific type arguments when they should be determined by other arguments
Utilize narrowing inswitch(true)blocks for control flow analysis (TypeScript 5.3+)
Rely on narrowing from direct boolean comparisons for type guards
Trust preserved narrowing in closures when variables aren't modified after the check (TypeScript 5.4+)
Use constant indices to narrow object/array properties (TypeScript 5.5+)
Use standard ECMAScript decorators (Stage 3) instead of legacyexperimentalDecorators
Useusingdeclarations for explicit resource management with Disposable pattern instead of manual cleanup (TypeScript 5.2+)
Usewith { type: "json" }for import attributes; avoid deprecatedassertsyntax (TypeScript 5.3/5.8+)
Useimport typeexplicitly when importing types to ensure they are erased during compilation, respectingverbatimModuleSyntaxflag
Use.ts,.mts,.ctsextensions inimport typestatements (TypeScript 5.2+)
Useimport type { Type } from "mod" with { "resolution-mode": "import" }for specific module resolution contexts (TypeScript 5.3+)
Use new iterator methods (map, filter, etc.) if targeting modern environments (TypeScript 5.6+)
Utilize newSetmethods likeunion,intersection, etc., when available (TypeScript 5.5+)
UseObject.groupBy/Map.groupBystandard methods for grouping instead of external libraries (TypeScript 5.4+)
UsePromise.withResolvers()for creating promises with exposed resolve/reject functions (TypeScript 5.7+)
Use copying array methods (toSorted,toSpliced,with) for immutable array operations (TypeScript 5.2+)
Avoid accessing instance fields viasuperin classes (TypeScript 5....
Files:
packages/editor/src/core/hooks/use-editor.tspackages/editor/src/core/helpers/editor-ref.ts
🧠 Learnings (1)
📚 Learning: 2025-11-25T10:18:05.172Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-11-25T10:18:05.172Z
Learning: Applies to **/*.{js,mjs,cjs} : Use `import` tags in JSDoc for cleaner type imports in JavaScript files when working in a mixed codebase (TypeScript 5.5+)
Applied to files:
packages/editor/src/core/helpers/editor-ref.ts
🧬 Code graph analysis (2)
packages/editor/src/core/hooks/use-editor.ts (1)
packages/editor/src/core/helpers/editor-ref.ts (1)
getEditorRefHelpers(24-261)
packages/editor/src/core/helpers/editor-ref.ts (2)
packages/editor/src/core/types/editor.ts (2)
IEditorProps(142-171)EditorRefApi(100-139)packages/utils/src/editor/markdown-parser/root.ts (1)
convertHTMLToMarkdown(112-143)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: CodeQL analysis (javascript-typescript)
- GitHub Check: Agent
- GitHub Check: Analyze (javascript)
- GitHub Check: Build and lint web apps
🔇 Additional comments (5)
packages/editor/src/core/helpers/editor-ref.ts (4)
5-6: LGTM!The import of
convertHTMLToMarkdownis correctly added to support the new markdown conversion flow.
13-13: LGTM!The addition of
IEditorPropsto the type imports is correct and follows TypeScript best practices by usingimport type.
19-22: LGTM!The type extension using
Pick<IEditorProps, "getEditorMetaData">is a clean and type-safe approach to ensure the function signature matches the interface definition.
25-25: LGTM!The destructuring correctly extracts
getEditorMetaDataalongside the existing properties.packages/editor/src/core/hooks/use-editor.ts (1)
135-144:getEditorMetaDatais already memoized in parent components viauseCallback; the dependency array addition is correct and poses no performance concerns.The verification confirms that
getEditorMetaDatais properly wrapped inuseCallbackin the parent components (bothapps/web/core/hooks/use-parse-editor-content.tsandapps/space/core/hooks/use-parse-editor-content.ts), ensuring it remains stable across renders. The addition ofgetEditorMetaDatato theuseImperativeHandledependency array on line 143 is therefore correct per React's rules of hooks, and the ref helpers will only be recreated when necessary. No performance issues arise from this change.
| getMarkDown: () => { | ||
| const markdownOutput = editor?.storage?.markdown?.getMarkdown?.() ?? ""; | ||
| return markdownOutput; | ||
| if (!editor) return ""; | ||
| const editorHTML = editor.getHTML(); | ||
| const metaData = getEditorMetaData(editorHTML); | ||
| // convert to markdown | ||
| const markdown = convertHTMLToMarkdown({ | ||
| description_html: editorHTML, | ||
| metaData, | ||
| }); | ||
| return markdown; | ||
| }, |
There was a problem hiding this comment.
Add error handling for the markdown conversion flow.
The refactored getMarkDown implementation doesn't handle potential errors from getEditorMetaData or convertHTMLToMarkdown. If either function throws, it could break the copy markdown functionality and impact user experience.
Apply this diff to add defensive error handling:
getMarkDown: () => {
if (!editor) return "";
- const editorHTML = editor.getHTML();
- const metaData = getEditorMetaData(editorHTML);
- // convert to markdown
- const markdown = convertHTMLToMarkdown({
- description_html: editorHTML,
- metaData,
- });
- return markdown;
+ try {
+ const editorHTML = editor.getHTML();
+ const metaData = getEditorMetaData(editorHTML);
+ // convert to markdown
+ const markdown = convertHTMLToMarkdown({
+ description_html: editorHTML,
+ metaData,
+ });
+ return markdown;
+ } catch (error) {
+ console.error("Error converting HTML to markdown:", error);
+ return "";
+ }
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| getMarkDown: () => { | |
| const markdownOutput = editor?.storage?.markdown?.getMarkdown?.() ?? ""; | |
| return markdownOutput; | |
| if (!editor) return ""; | |
| const editorHTML = editor.getHTML(); | |
| const metaData = getEditorMetaData(editorHTML); | |
| // convert to markdown | |
| const markdown = convertHTMLToMarkdown({ | |
| description_html: editorHTML, | |
| metaData, | |
| }); | |
| return markdown; | |
| }, | |
| getMarkDown: () => { | |
| if (!editor) return ""; | |
| try { | |
| const editorHTML = editor.getHTML(); | |
| const metaData = getEditorMetaData(editorHTML); | |
| // convert to markdown | |
| const markdown = convertHTMLToMarkdown({ | |
| description_html: editorHTML, | |
| metaData, | |
| }); | |
| return markdown; | |
| } catch (error) { | |
| console.error("Error converting HTML to markdown:", error); | |
| return ""; | |
| } | |
| }, |
🤖 Prompt for AI Agents
In packages/editor/src/core/helpers/editor-ref.ts around lines 81 to 91, the
getMarkDown function currently calls getEditorMetaData and convertHTMLToMarkdown
without protection; wrap the conversion flow in a try/catch, catch any errors
from metadata extraction or markdown conversion, log the error with the project
logger (or console.error if no logger is available), and return an empty string
(or a safe fallback) so the UI doesn't break; ensure the catch block avoids
rethrowing and keeps the function deterministic.
Description
This PR implements the new markdown rules in the
Copy markdownoption.Type of Change
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.