fix: handle relative image paths in markdown renderer#95
fix: handle relative image paths in markdown renderer#95sejalkutriyar wants to merge 1 commit intoStabilityNexus:mainfrom
Conversation
WalkthroughUpdated the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/markdown-renderer.tsx`:
- Line 67: The optional chaining on resolvedSrc is redundant because the
preceding type guard (typeof src === "string") ensures resolvedSrc is a string;
update the conditional in the markdown-renderer (the if that currently uses
resolvedSrc?.startsWith("/") && !resolvedSrc.startsWith(BASE_PATH)) to remove
the ?. so it reads resolvedSrc.startsWith("/") &&
!resolvedSrc.startsWith(BASE_PATH), keeping the same logic and using the
existing resolvedSrc identifier.
- Around line 62-64: The current check in markdown-renderer.tsx only replaces
the first "../" (resolvedSrc = src.replace("../", "/")) so deeper relative paths
like "../../images" or "../foo/../images" won't be normalized; update the logic
that sets resolvedSrc (using the src variable) to strip or collapse all leading
"../" segments (e.g., use a regex that matches one-or-more leading "../"
segments or loop while src.startsWith("../") to repeatedly remove them) so
resolvedSrc becomes a proper rooted path for any depth of relative path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c86ba8d5-744f-4895-9052-adb8977c137d
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (1)
lib/markdown-renderer.tsx
| if (src.startsWith("../")) { | ||
| resolvedSrc = src.replace("../", "/") | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider handling deeper relative paths for future-proofing.
String.replace() only replaces the first occurrence. If a markdown file ever uses ../../images/ or paths like ../foo/../images/, only the first ../ would be replaced. While context confirms current files use single-level ../, a regex or loop would be more robust.
🔧 Optional: Handle multiple relative path segments
// Handle relative paths like ../images/ - assumes images are in public/images/
- if (src.startsWith("../")) {
- resolvedSrc = src.replace("../", "/")
+ // Normalize by collapsing leading ../ sequences to root-relative
+ while (resolvedSrc.startsWith("../")) {
+ resolvedSrc = resolvedSrc.slice(3) // Remove leading "../"
}
+ if (resolvedSrc !== src) {
+ resolvedSrc = "/" + resolvedSrc
+ }Alternatively, a simpler regex approach:
- if (src.startsWith("../")) {
- resolvedSrc = src.replace("../", "/")
- }
+ // Strip all leading ../ and prepend /
+ resolvedSrc = src.replace(/^(\.\.\/)+/, "/")📝 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.
| if (src.startsWith("../")) { | |
| resolvedSrc = src.replace("../", "/") | |
| } | |
| // Strip all leading ../ and prepend / | |
| resolvedSrc = src.replace(/^(\.\.\/)+/, "/") |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/markdown-renderer.tsx` around lines 62 - 64, The current check in
markdown-renderer.tsx only replaces the first "../" (resolvedSrc =
src.replace("../", "/")) so deeper relative paths like "../../images" or
"../foo/../images" won't be normalized; update the logic that sets resolvedSrc
(using the src variable) to strip or collapse all leading "../" segments (e.g.,
use a regex that matches one-or-more leading "../" segments or loop while
src.startsWith("../") to repeatedly remove them) so resolvedSrc becomes a proper
rooted path for any depth of relative path.
| } | ||
|
|
||
| // Prepend BASE_PATH if it's a root-relative path | ||
| if (resolvedSrc?.startsWith("/") && !resolvedSrc.startsWith(BASE_PATH)) { |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Optional chaining is redundant after the type guard.
After the typeof src === "string" check on line 60, resolvedSrc is guaranteed to be a string, making the ?. unnecessary. This is a minor nitpick and doesn't affect functionality.
🧹 Remove redundant optional chaining
- if (resolvedSrc?.startsWith("/") && !resolvedSrc.startsWith(BASE_PATH)) {
+ if (resolvedSrc.startsWith("/") && !resolvedSrc.startsWith(BASE_PATH)) {📝 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.
| if (resolvedSrc?.startsWith("/") && !resolvedSrc.startsWith(BASE_PATH)) { | |
| if (resolvedSrc.startsWith("/") && !resolvedSrc.startsWith(BASE_PATH)) { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/markdown-renderer.tsx` at line 67, The optional chaining on resolvedSrc
is redundant because the preceding type guard (typeof src === "string") ensures
resolvedSrc is a string; update the conditional in the markdown-renderer (the if
that currently uses resolvedSrc?.startsWith("/") &&
!resolvedSrc.startsWith(BASE_PATH)) to remove the ?. so it reads
resolvedSrc.startsWith("/") && !resolvedSrc.startsWith(BASE_PATH), keeping the
same logic and using the existing resolvedSrc identifier.
1 participant
Addressed Issues:
Fixes #(TODO:issue number)
Fixes #89
Screenshots/Recordings:
TODO: If applicable, add screenshots or recordings that demonstrate the interface before and after the changes.
before fix
after fix
Additional Notes:
The markdown articles in this repository use relative paths (e.g.,
../images/) to reference assets. This PR updateslib/markdown-renderer.tsxto correctly resolve these paths to the root-relative/images/directory, ensuring images render correctly when viewed on the/a/[slug]route. It also ensures that any configuredBASE_PATHis respected.AI Usage Disclosure:
We encourage contributors to use AI tools responsibly when creating Pull Requests. While AI can be a valuable aid, it is essential to ensure that your contributions meet the task requirements, build successfully, include relevant tests, and pass all linters. Submissions that do not meet these standards may be closed without warning to maintain the quality and integrity of the project. Please take the time to understand the changes you are proposing and their impact. AI slop is strongly discouraged and may lead to banning and blocking. Do not spam our repos with AI slop.
Check one of the checkboxes below:
I have used the following AI models and tools: TODO
I have used chatgpt to identify the markdown rendering bug
Checklist
Summary by CodeRabbit