-
Notifications
You must be signed in to change notification settings - Fork 41
Update Migrate Export MarkDown, Html, PDF #549
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Update Migrate Export MarkDown, Html, PDF #549
Conversation
WalkthroughFrontend export functionality is migrated from backend mutation to client-side generation. Dependencies add html2canvas, jspdf, and marked. The export hook now supports Markdown, HTML, and PDF exports, generating content in-browser, removing previous mutation usage, and introducing new handler methods with updated enum values. Changes
Sequence Diagram(s)sequenceDiagram
actor U as User
participant UI as Export UI
participant H as useFileExport Hook
participant M as marked
participant C as html2canvas
participant P as jsPDF
participant B as Browser (Download)
U->>UI: Click Export (type, content, filename)
UI->>H: handleExport(type, content, filename)
H->>H: Show "Export started" snackbar
alt Export: Markdown
H->>B: Create Blob(text/markdown) & trigger download
else Export: HTML
H->>M: Convert Markdown -> HTML
M-->>H: HTML string
H->>B: Create Blob(text/html) & trigger download
else Export: PDF
H->>H: Render HTML in off-screen container
H->>C: Capture container to canvas
C-->>H: Canvas image
H->>P: Compose PDF pages
P-->>H: PDF blob
H->>H: Cleanup off-screen container
H->>B: Trigger PDF download
end
H-->>UI: Success/Failure status
UI-->>U: Snackbar notification
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (7)
frontend/package.json (1)
54-56
: Heavy libs: prefer lazy imports to keep initial bundle small.html2canvas and jsPDF add significant weight. Import them on demand inside the export handler instead of top‑level.
I’ve included a code diff in useFileExport.ts showing how to lazy‑import both.
frontend/src/hooks/useFileExport.ts (6)
6-8
: Defer loading heavy libraries (code‑split).Top‑level imports of marked/jsPDF/html2canvas bloat the main bundle. Lazy‑load inside handlers.
Apply:
-import { marked } from "marked"; -import { jsPDF } from "jspdf"; -import html2canvas from "html2canvas"; +// Lazy-load inside handlers to reduce initial bundle sizeAnd see the handler diff below (lazy imports).
58-71
: Hide off‑screen element more safely.Use visibility:hidden to avoid accidental focus/selection and pointer events.
- element.style.position = "absolute"; - element.style.left = "-9999px"; - element.style.top = "0"; + element.style.position = "fixed"; + element.style.left = "-10000px"; + element.style.top = "0"; + element.style.visibility = "hidden"; + element.style.pointerEvents = "none";
88-103
: Avoid potential extra blank page due to rounding.Use Math.ceil and track positions cumulatively; also add a bottom margin.
- const imgWidth = 210; - const pageHeight = 297; // A4 - const imgHeight = (canvas.height * imgWidth) / canvas.width; - let heightLeft = imgHeight; - let position = 0; - pdf.addImage(imgData, "PNG", 0, position, imgWidth, imgHeight); - heightLeft -= pageHeight; - while (heightLeft > 0) { - position = heightLeft - imgHeight; - pdf.addPage(); - pdf.addImage(imgData, "PNG", 0, position, imgWidth, imgHeight); - heightLeft -= pageHeight; - } + const imgWidth = 210; + const pageHeight = 297; + const marginTop = 0, marginLeft = 0; + const imgHeight = (canvas.height * imgWidth) / canvas.width; + let position = 0; + pdf.addImage(imgData, "PNG", marginLeft, position, imgWidth, imgHeight); + let heightLeft = imgHeight - pageHeight; + while (heightLeft > -0.5) { + position -= pageHeight; + pdf.addPage(); + pdf.addImage(imgData, "PNG", marginLeft, position, imgWidth, imgHeight); + heightLeft -= pageHeight; + }
33-36
: Sanitize filename to avoid illegal characters.User titles may contain characters invalid for downloads on some platforms.
- const documentName = documentStore.data?.title || "codepair_document"; + const rawName = documentStore.data?.title || "codepair_document"; + const documentName = rawName.replace(/[<>:"/\\|?*\x00-\x1F]/g, "_").slice(0, 200);Also applies to: 115-123
134-135
: Stabilize useCallback deps.Depending on whole store objects causes frequent re‑creations. Depend only on needed scalars.
- [editorStore, documentStore, enqueueSnackbar] + [editorStore.doc, documentStore.data?.title, enqueueSnackbar]
16-22
: Return Promise from handlers to allow callers to await.Enables UI to show loading states or disable buttons during export.
-type ExportFunction = () => void; +type ExportFunction = () => Promise<void>; ... -const handleExportToPDF = () => handleExportFile(FileExtension.PDF); +const handleExportToPDF = () => handleExportFile(FileExtension.PDF);(handleExportFile is already async, so types align.)
Also applies to: 137-141
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
frontend/package.json
(4 hunks)frontend/src/hooks/useFileExport.ts
(3 hunks)
🧰 Additional context used
🪛 ast-grep (0.39.5)
frontend/src/hooks/useFileExport.ts
[warning] 58-58: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: element.innerHTML = html
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 58-58: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: element.innerHTML = html
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
🔇 Additional comments (1)
frontend/package.json (1)
43-43
: Do not remove @yorkie-js/sdk—it’s in use
The@yorkie-js/sdk
package is imported across multiple modules (utils/yorkie, store/editorSlice.ts, several hooks), so it cannot be safely removed.Likely an incorrect or invalid review comment.
"markdown-it-prism": "^2.3.0", | ||
"markdown-it-sanitizer": "^0.4.3", | ||
"markdown-it-task-checkbox": "^1.0.6", | ||
"marked": "^16.3.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Add HTML sanitization dependency (DOMPurify) to mitigate XSS.
You render marked output via innerHTML in the PDF path. Marked does not sanitize by default; sanitize before injecting.
Apply this to dependencies (pick latest stable):
"dependencies": {
+ "dompurify": "^3.0.0",
"html2canvas": "^1.4.1",
"jspdf": "^3.0.3",
"marked": "^16.3.0",
Please confirm the latest safe DOMPurify version and whether a caret range is acceptable in this repo.
🏁 Script executed:
#!/bin/bash
# Show dependencies block to verify version syntax
sed -n '1,200p' frontend/package.json
# Search for innerHTML usage in JS/TS files
rg -n 'innerHTML' -g '*.js' -g '*.ts'
# Search for marked() invocation in JS/TS files
rg -n 'marked\(' -g '*.js' -g '*.ts'
Length of output: 3604
🌐 Web query:
What is the latest stable version of DOMPurify?
💡 Result:
The latest stable DOMPurify release is 3.2.7 (released Sep 17, 2025). [1][2]
Sources:
[1] GitHub Releases — cure53/DOMPurify (3.2.7).
[2] Snyk / npm package info for dompurify (latest: 3.2.7).
Add DOMPurify v3.2.7 dependency for sanitizing marked output. You render marked output via innerHTML in frontend/src/hooks/useFileExport.ts; sanitize before injecting. Add to dependencies:
"dependencies": {
+ "dompurify": "^3.2.7",
📝 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.
"marked": "^16.3.0", | |
"dependencies": { | |
"dompurify": "^3.2.7", | |
"marked": "^16.3.0", |
🤖 Prompt for AI Agents
In frontend/package.json around line 67, add the dependency "dompurify": "3.2.7"
to dependencies and update the code that injects marked HTML
(frontend/src/hooks/useFileExport.ts) to sanitize before using innerHTML;
specifically install/add the dependency to package.json, run yarn/npm install,
import DOMPurify in useFileExport (or access window.DOMPurify) and replace
direct innerHTML assignment with innerHTML = DOMPurify.sanitize(marked(...));
ensure TypeScript imports/types are handled (install @types/dompurify if needed)
and update tests/build accordingly.
const handleExportFile = useCallback( | ||
async (exportType: string) => { | ||
try { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Tighten types: use FileExtension instead of string.
Prevents unsupported values and improves autocomplete.
- async (exportType: string) => {
+ async (exportType: FileExtension) => {
Also applies to: 42-113
🤖 Prompt for AI Agents
In frontend/src/hooks/useFileExport.ts around lines 29-31 (and also apply
changes through lines 42-113), the handleExportFile parameter is typed as a
plain string; change its type to the FileExtension union/enum used across the
codebase. Import or reference the existing FileExtension type at the top of the
file, update the handleExportFile signature to use FileExtension instead of
string, and update any local variables, calls, and switch/conditional logic in
lines 42-113 to expect FileExtension (adjust casts or validations where
necessary). Ensure any external callers pass a FileExtension value or
map/validate inputs to FileExtension before calling.
case FileExtension.HTML: | ||
{ | ||
const html = await marked(markdown); | ||
blob = new Blob([html], { type: "text/html" }); | ||
fileName = `${documentName}.html`; | ||
} | ||
break; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sanitize HTML before innerHTML to prevent XSS.
marked output is unsanitized; assigning to innerHTML is exploitable even off‑screen. Sanitize for both HTML and PDF paths.
+import DOMPurify from "dompurify";
...
- {
- const html = await marked(markdown);
- blob = new Blob([html], { type: "text/html" });
+ {
+ const { marked } = await import("marked");
+ const rawHtml = await marked(markdown);
+ const safeHtml = DOMPurify.sanitize(rawHtml);
+ blob = new Blob([safeHtml], { type: "text/html;charset=utf-8" });
fileName = `${documentName}.html`;
}
...
- {
- const html = await marked(markdown);
+ {
+ const [{ default: html2canvas }, { jsPDF }, { marked }] = await Promise.all([
+ import("html2canvas"),
+ import("jspdf"),
+ import("marked"),
+ ]);
+ const rawHtml = await marked(markdown);
+ const safeHtml = DOMPurify.sanitize(rawHtml);
const element = document.createElement("div");
- element.innerHTML = html;
+ element.innerHTML = safeHtml;
Based on static analysis hints.
Also applies to: 56-66, 58-71
🤖 Prompt for AI Agents
In frontend/src/hooks/useFileExport.ts around lines 47 to 53 (and similarly
56-66, 58-71), the HTML generated by marked is unsanitized and later assigned to
innerHTML (and used in the PDF path), leaving an XSS risk; update the code to
sanitize marked(markdown) before creating the Blob or injecting into the DOM:
import a trusted sanitizer (e.g., DOMPurify), run const cleanHtml =
DOMPurify.sanitize(html, {ALLOWED_TAGS: ...} or appropriate config), then use
cleanHtml when creating the Blob for HTML export and when passing HTML into the
PDF rendering flow; ensure the sanitizer is added to package.json, typed
correctly (or use any available DOMPurify types), and include a short comment
noting why sanitization is required.
What this PR does / why we need it:
This PR migrates the file export functionality from server-side to client-side processing. Instead of sending export requests to the backend, the frontend now handles Markdown, HTML, and PDF conversions directly in the browser using
jspdf
,html2canvas
, andmarked
libraries.Which issue(s) this PR fixes:
Fixes #443
Special notes for your reviewer:
useExportFileMutation
API calljspdf
,html2canvas
, andmarked
Does this PR introduce a user-facing change?:
pdf.pdf
md.md
html.html
Additional documentation:
Checklist:
Summary by CodeRabbit
New Features
Chores