Add local post editing#4
Conversation
- TipTap-based rich text editor at /edit/[slug] - Matches site typography (Canela fonts, 72ch width, 200% line-height) - MDX components shown as read-only placeholder blocks - Save with Cmd+S, writes back to .mdx files - Conflict detection when file is edited externally (VS Code) - Dev-mode only (disabled in production) New files: - src/pages/edit/[...slug].astro - editor page - src/components/editor/PostEditor.tsx - TipTap editor - src/components/editor/editor-styles.css - prose styling - src/pages/api/save-post.ts - save API with conflict handling Config changes: - astro.config.mjs: output: "server" for dynamic routes - Added prerender: true to static pages to maintain SSG 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
There was a problem hiding this comment.
Pull request overview
This pull request adds a local post editing feature that enables in-browser editing of blog posts in development mode. The implementation introduces a dynamic server-rendered editing interface while maintaining static generation for production builds.
Key changes:
- New browser-based MDX editor with live component rendering for dev environment
- API endpoints for saving files and rendering components (dev-only)
- Custom build script to exclude dev-only pages from static builds
- Configuration shift to server output mode with selective prerendering
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 20 comments.
Show a summary per file
| File | Description |
|---|---|
src/scripts/build-static.js |
Build script that temporarily removes dev-only pages before static build |
src/pages/edit/[...slug].astro |
Editor page that displays post metadata and embeds the React editor |
src/pages/api/save-post.ts |
API endpoint for saving post content with conflict detection |
src/pages/api/render-component.ts |
API endpoint that renders component placeholders from MDX code |
src/components/editor/PostEditor.tsx |
React component implementing the MDX editor with segment-based parsing |
src/components/editor/editor-styles.css |
Styling for the editor interface matching blog aesthetics |
src/layouts/PostLayout.astro |
Added Edit button for posts in development mode |
astro.config.mjs |
Changed to server output mode to enable dynamic routes |
package.json |
Added TipTap dependencies and modified build/preview commands |
src/pages/[...slug].astro |
Added prerender flag for static generation |
src/pages/topics/[topic].astro |
Added prerender flag for static generation |
src/pages/og/[...slug].png.ts |
Added prerender flag for static generation |
src/pages/now-[slug].astro |
Added prerender flag for static generation |
src/components/mdx/Draft.astro |
Icon name capitalization change |
src/content/essays/growing-a-human.mdx |
Added trailing newline |
src/content/data/webmentions.json |
Updated timestamp |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "@tiptap/extension-placeholder": "^3.14.0", | ||
| "@tiptap/pm": "^3.14.0", | ||
| "@tiptap/react": "^3.14.0", | ||
| "@tiptap/starter-kit": "^3.14.0", |
There was a problem hiding this comment.
The TipTap packages (@tiptap/extension-placeholder, @tiptap/pm, @tiptap/react, @tiptap/starter-kit) are added as dependencies but don't appear to be used anywhere in the codebase. These packages add significant bundle size. If they were intended for future use, consider moving them to devDependencies or removing them until they're actually needed.
| "@tiptap/extension-placeholder": "^3.14.0", | |
| "@tiptap/pm": "^3.14.0", | |
| "@tiptap/react": "^3.14.0", | |
| "@tiptap/starter-kit": "^3.14.0", |
| site: "https://maggieappleton.com", | ||
| // Server mode with static as default - allows /edit/* pages to be dynamic | ||
| // Most pages prerender by default; specific pages opt-out with prerender = false | ||
| output: "server", |
There was a problem hiding this comment.
The comment states this is for dev-only pages, but setting output to "server" affects the entire site's build strategy. This means all pages now require a Node.js server in production unless explicitly marked with prerender = true. Ensure this aligns with your deployment infrastructure, and consider documenting the deployment requirements (Node.js server vs static hosting).
| output: "server", | |
| output: "hybrid", |
| } | ||
| } catch (error) { | ||
| // File might not exist, which is fine for new files | ||
| console.error("Error checking file stats:", error); |
There was a problem hiding this comment.
The error handling here silently continues when file stat checking fails, which could mask real errors. While the comment says "File might not exist, which is fine for new files", this endpoint is editing existing files based on the context. Consider logging this error or being more specific about which errors are acceptable (e.g., ENOENT for new files only).
| console.error("Error checking file stats:", error); | |
| const err = error as NodeJS.ErrnoException; | |
| if (err.code !== "ENOENT") { | |
| console.error("Error checking file stats:", error); | |
| throw error; | |
| } |
| function resolveImagePath(src: string): string { | ||
| if (src.startsWith("/images/")) { | ||
| // In dev mode, images in src/images/ need to be served via /@fs/ | ||
| // The absolute path to the project root | ||
| const projectRoot = process.cwd(); | ||
| return `/@fs${projectRoot}/src${src}`; | ||
| } | ||
| if (src.startsWith("../../images/")) { | ||
| const projectRoot = process.cwd(); | ||
| return `/@fs${projectRoot}/src/images/${src.replace("../../images/", "")}`; | ||
| } | ||
| // Remote URLs or already absolute paths | ||
| return src; | ||
| } |
There was a problem hiding this comment.
The resolveImagePath function constructs file system paths but returns them as URLs for the dev server. However, it doesn't validate that the path actually exists or is within allowed directories. An attacker could potentially craft image paths that access files outside the images directory. Add validation to ensure resolved paths stay within the expected directory structure.
| // Remove output: "server" line | ||
| const modified = content.replace(/\s*\/\/.*\n\s*\/\/.*\n\s*output:\s*["']server["'],?\n?/g, '\n'); |
There was a problem hiding this comment.
The regex pattern used here is fragile and could break with different formatting. The pattern /\s*\/\/.*\n\s*\/\/.*\n\s*output:\s*["']server["'],?\n?/g makes assumptions about comment formatting (two comment lines) and exact whitespace. If the Astro config comments change, this will fail silently. Consider a more robust approach, such as parsing the file as JavaScript/ESM or using a more flexible regex that doesn't rely on specific comment formatting.
| // Remove output: "server" line | |
| const modified = content.replace(/\s*\/\/.*\n\s*\/\/.*\n\s*output:\s*["']server["'],?\n?/g, '\n'); | |
| // Remove lines that set output: "server" (or 'server'), independent of surrounding comments/whitespace | |
| const modified = content.replace(/^\s*output:\s*["']server["']\s*,?\s*$/gm, ''); |
| // Check if dist folder has content | ||
| const distPath = path.join(projectRoot, 'dist'); | ||
| if (fs.existsSync(distPath) && fs.readdirSync(distPath).length > 10) { |
There was a problem hiding this comment.
Using an arbitrary threshold of "more than 10 files" to determine build success is unreliable. This approach could give false positives (build partially succeeded but important files are missing) or false negatives (small site with few files that actually built successfully). Consider checking for specific required files (like index.html) or examining the build error more carefully instead.
| // Check if dist folder has content | |
| const distPath = path.join(projectRoot, 'dist'); | |
| if (fs.existsSync(distPath) && fs.readdirSync(distPath).length > 10) { | |
| // Check if dist has a primary output file | |
| const distPath = path.join(projectRoot, 'dist'); | |
| const indexHtmlPath = path.join(distPath, 'index.html'); | |
| if (fs.existsSync(indexHtmlPath)) { |
|
|
||
| while (j < lines.length && !foundEnd) { | ||
| componentContent += "\n" + lines[j]; | ||
|
|
||
| if (inOpeningTag) { | ||
| // Still looking for the end of the opening tag | ||
| // It could end with > (content follows) or /> (self-closing) | ||
| if (lines[j].trim().endsWith("/>")) { | ||
| // Self-closing tag complete | ||
| foundEnd = true; | ||
| } else if (lines[j].includes(">")) { | ||
| // Opening tag closed, now look for closing tag | ||
| inOpeningTag = false; | ||
| // But also check if closing tag is on same line | ||
| if (lines[j].includes(`</${componentName}>`)) { | ||
| foundEnd = true; | ||
| } | ||
| } | ||
| } else { | ||
| // Looking for closing tag </ComponentName> | ||
| // Simple approach: just find the closing tag (doesn't handle deep nesting of same component) | ||
| if (lines[j].includes(`</${componentName}>`)) { | ||
| foundEnd = true; | ||
| } |
There was a problem hiding this comment.
The component parsing logic doesn't properly handle nested components of the same type. For example, if you have nested <SimpleCard> components, the parser will incorrectly match the first closing tag it encounters. This could lead to components being split incorrectly or content being lost. Consider implementing a proper tag matching algorithm that counts opening and closing tags.
| while (j < lines.length && !foundEnd) { | |
| componentContent += "\n" + lines[j]; | |
| if (inOpeningTag) { | |
| // Still looking for the end of the opening tag | |
| // It could end with > (content follows) or /> (self-closing) | |
| if (lines[j].trim().endsWith("/>")) { | |
| // Self-closing tag complete | |
| foundEnd = true; | |
| } else if (lines[j].includes(">")) { | |
| // Opening tag closed, now look for closing tag | |
| inOpeningTag = false; | |
| // But also check if closing tag is on same line | |
| if (lines[j].includes(`</${componentName}>`)) { | |
| foundEnd = true; | |
| } | |
| } | |
| } else { | |
| // Looking for closing tag </ComponentName> | |
| // Simple approach: just find the closing tag (doesn't handle deep nesting of same component) | |
| if (lines[j].includes(`</${componentName}>`)) { | |
| foundEnd = true; | |
| } | |
| let nestingLevel = 0; // Tracks nested components of the same type | |
| while (j < lines.length && !foundEnd) { | |
| const currentLine = lines[j]; | |
| componentContent += "\n" + currentLine; | |
| if (inOpeningTag) { | |
| // Still looking for the end of the opening tag | |
| // It could end with > (content follows) or /> (self-closing) | |
| if (currentLine.trim().endsWith("/>")) { | |
| // Self-closing tag complete | |
| foundEnd = true; | |
| } else if (currentLine.includes(">")) { | |
| // Opening tag closed, now look for closing tag | |
| inOpeningTag = false; | |
| // But also check if closing tag is on same line | |
| if (currentLine.includes(`</${componentName}>`)) { | |
| // We will let the nesting-aware logic below determine | |
| // whether this is the actual end or a nested close | |
| } | |
| } | |
| } | |
| if (!inOpeningTag && !foundEnd) { | |
| // Looking for closing tag </ComponentName> with nesting awareness | |
| const openTagRegex = new RegExp(`<${componentName}(\\s|>|/)`, "g"); | |
| const closeTagRegex = new RegExp(`</${componentName}>`, "g"); | |
| const openMatches = currentLine.match(openTagRegex); | |
| const closeMatches = currentLine.match(closeTagRegex); | |
| const opensOnLine = openMatches ? openMatches.length : 0; | |
| const closesOnLine = closeMatches ? closeMatches.length : 0; | |
| // Count nested openings (these start at j >= i + 1, so they are inner components) | |
| if (opensOnLine > 0) { | |
| nestingLevel += opensOnLine; | |
| } | |
| // Process each closing tag; when nestingLevel is zero, the closing tag | |
| // corresponds to the current outer component. | |
| if (closesOnLine > 0) { | |
| for (let k = 0; k < closesOnLine; k++) { | |
| if (nestingLevel === 0) { | |
| foundEnd = true; | |
| break; | |
| } else { | |
| nestingLevel--; | |
| } | |
| } | |
| } |
| e.preventDefault(); | ||
| save(); |
There was a problem hiding this comment.
The keyboard shortcut handler for Cmd/Ctrl+S doesn't prevent the default browser save behavior in all cases. While preventDefault is called, some browsers might still show the save dialog if the event bubbles. Consider also calling stopPropagation() and returning false to ensure the browser's save behavior is fully suppressed.
| e.preventDefault(); | |
| save(); | |
| e.preventDefault(); | |
| e.stopPropagation(); | |
| save(); | |
| return false; |
| .replace(/</g, "<") | ||
| .replace(/>/g, ">") | ||
| .replace(/"/g, """) | ||
| .replace(/'/g, "'"); |
There was a problem hiding this comment.
The escapeHtml function doesn't escape backticks or curly braces, which could be problematic when the escaped content is used in template literals or JSX-like contexts. Consider adding cases for backtick (`) and curly braces ({ }) to prevent potential injection issues.
| .replace(/'/g, "'"); | |
| .replace(/'/g, "'") | |
| .replace(/`/g, "`") | |
| .replace(/{/g, "{") | |
| .replace(/}/g, "}"); |
| const currentSlug = Astro.url.pathname.replace(/^\/|\/$/g, ""); | ||
|
|
||
| // Check if we're in development mode | ||
| const isDev = import.meta.env.DEV; |
There was a problem hiding this comment.
The variable name 'isDev' could be more descriptive. While it's clear in context, 'isDevMode' or 'isDevelopment' would be more explicit and follow common naming conventions. This improves code readability, especially for developers unfamiliar with the codebase.
| const isDev = import.meta.env.DEV; | |
| const isDevelopment = import.meta.env.DEV; |
No description provided.