fix:url truncates in docs#8468
Conversation
adwait-bruno
requested review from
bijin-bruno,
helloanoop,
lohit-bruno,
naman-bruno,
sid-bruno,
utkarsh-bruno and
vijayh-bruno
as code owners
WalkthroughMarkdown rendering now memoizes its MarkdownIt instance, applies a linkify patch that extends URLs with balanced parentheses, and keeps sanitizing rendered HTML. A new utility module implements the match extension behavior, with tests covering both helper functions. ChangesMarkdown rendering and linkify integration
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant MarkDown
participant MarkdownIt
participant DOMPurify
MarkDown->>MarkdownIt: create memoized instance
MarkDown->>MarkdownIt: render markdown
MarkdownIt-->>MarkDown: HTML
MarkDown->>DOMPurify: sanitize HTML
DOMPurify-->>MarkDown: safe HTML
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
| }; | ||
|
|
||
| return md; | ||
| } |
There was a problem hiding this comment.
Please move this to utils/url/index.js and write unit test cases for this function
adwait-bruno
force-pushed
the
fix/docs-url-truncate
branch
from
f8cc30b to
2483cc0
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/bruno-app/src/components/MarkDown/index.jsx (1)
11-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting MarkdownIt setup into a custom hook.
Instance creation (config, plugin use, and linkify patching) is business/setup logic inlined in the component body. Moving it into a small custom hook (e.g.
useMarkdownIt(allowHtml, collectionPath)) keeps the component focused on rendering/event wiring.♻️ Suggested extraction
- const md = useMemo(() => { - const instance = new MarkdownIt({ - html: allowHtml, - breaks: true, - linkify: true, - replaceLink: (link) => link.replace(/^\./, collectionPath) - }).use(MarkdownItReplaceLink); - - return patchLinkifyToExtendUrls(instance); - }, [allowHtml, collectionPath]); + const md = useMarkdownIt(allowHtml, collectionPath);// utils/markdown/useMarkdownIt.js export function useMarkdownIt(allowHtml, collectionPath) { return useMemo(() => { const instance = new MarkdownIt({ html: allowHtml, breaks: true, linkify: true, replaceLink: (link) => link.replace(/^\./, collectionPath) }).use(MarkdownItReplaceLink); return patchLinkifyToExtendUrls(instance); }, [allowHtml, collectionPath]); }As per coding guidelines, "MUST: Prefer custom hooks for business logic, data fetching, and side-effects in React."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bruno-app/src/components/MarkDown/index.jsx` around lines 11 - 20, Extract the MarkdownIt initialization from the MarkDown component into a custom hook such as useMarkdownIt, keeping the component focused on rendering. Move the existing useMemo logic, including MarkdownIt config, MarkdownItReplaceLink usage, and patchLinkifyToExtendUrls, into that hook. Preserve the allowHtml and collectionPath inputs as hook parameters and return the memoized instance for the component to consume.Source: Coding guidelines
packages/bruno-app/src/utils/linkify/index.spec.js (1)
53-115: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGood wrapper-level tests — consider adding one real-integration test.
The
patchLinkifyToExtendUrlstests mockmd.linkifyentirely, which correctly isolates the wrapper logic but never exercises the reallinkify-itAPI shape it's wrapping. Since this PR's goal is fixing truncation during actualmd.render()calls, one test using a realnew MarkdownIt({ linkify: true })instance (patched, then rendered against a URL with unbalanced parens) would guard against API drift inlinkify-it.As per coding guidelines, "Prioritise high-value tests over maximum coverage. Focus on testing behaviour that is critical, complex, or likely to break."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bruno-app/src/utils/linkify/index.spec.js` around lines 53 - 115, The current tests for patchLinkifyToExtendUrls only mock md.linkify, so they don’t validate behavior against the real markdown-it/linkify-it integration used by md.render(). Add one higher-value integration test in patchLinkifyToExtendUrls that uses a real MarkdownIt instance with linkify enabled, applies the patch, renders text containing a URL with unbalanced parentheses, and asserts the rendered link is extended correctly. Keep the existing wrapper-level tests, but use the real markdown-it API shape in the new test to protect against linkify-it drift.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/bruno-app/src/components/MarkDown/index.jsx`:
- Around line 11-20: Extract the MarkdownIt initialization from the MarkDown
component into a custom hook such as useMarkdownIt, keeping the component
focused on rendering. Move the existing useMemo logic, including MarkdownIt
config, MarkdownItReplaceLink usage, and patchLinkifyToExtendUrls, into that
hook. Preserve the allowHtml and collectionPath inputs as hook parameters and
return the memoized instance for the component to consume.
In `@packages/bruno-app/src/utils/linkify/index.spec.js`:
- Around line 53-115: The current tests for patchLinkifyToExtendUrls only mock
md.linkify, so they don’t validate behavior against the real
markdown-it/linkify-it integration used by md.render(). Add one higher-value
integration test in patchLinkifyToExtendUrls that uses a real MarkdownIt
instance with linkify enabled, applies the patch, renders text containing a URL
with unbalanced parentheses, and asserts the rendered link is extended
correctly. Keep the existing wrapper-level tests, but use the real markdown-it
API shape in the new test to protect against linkify-it drift.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6763e90a-316c-41c1-94eb-7753ed51b9da
📒 Files selected for processing (3)
packages/bruno-app/src/components/MarkDown/index.jsxpackages/bruno-app/src/utils/linkify/index.jspackages/bruno-app/src/utils/linkify/index.spec.js
2 participants
Description
JIRA
Links containing parentheses (e.g. dashboard URLs) were being cut off early by markdown-it's link detection. Reuses the existing paren-balancing fix from the CodeMirror editor (#7406) to resolve this.
Contribution Checklist:
Note: Keeping the PR small and focused helps make it easier to review and merge. If you have multiple changes you want to make, please consider submitting them as separate pull requests.
Publishing to New Package Managers
Please see here for more information.
Summary by CodeRabbit