Improve general submission process - #231
Conversation
- Introduced `DocsMarkdown.svelte` for rendering Markdown content with plugins for alerts, footnotes, and emojis. - Added cube submission guide (`cube-submission.md`) detailing eligibility, source requirements, and field definitions. - Created a general submission manual (`submission-manual.md`) outlining the review process and help resources. - Developed vendor submission guide (`vendor-submission.md`) specifying eligibility and source requirements for vendors. - Updated `package.json` to include necessary dependencies for Markdown processing. - Implemented new routes for the submission manuals in the documentation section.
❌ Deploy Preview for cubeindex failed. Why did it fail? →
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds a Markdown rendering component with heading anchors and table-of-contents navigation, three submission guide documents, and documentation routes that render the guides. ChangesDocumentation Markdown
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant DocsPage
participant DocsMarkdown
participant MarkdownIt
participant Browser
DocsPage->>DocsMarkdown: pass Markdown text
DocsMarkdown->>MarkdownIt: render text with plugins
MarkdownIt-->>DocsMarkdown: return HTML and heading tokens
DocsMarkdown->>Browser: render article and table of contents
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![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 (2)
src/lib/components/docs/DocsMarkdown.svelte (2)
94-95: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winSanitize HTML output if
textcan contain untrusted input.You added
dompurifytopackage.json, butresultis rendered as raw HTML without sanitization. If this component is ever used to render user-generated content, this poses a Cross-Site Scripting (XSS) vulnerability.If you intend to use DOMPurify here, note that it requires a DOM and will crash during SvelteKit SSR unless you use
isomorphic-dompurifyor run it purely client-side. Iftextis strictly local and trusted documentation, sanitization isn't strictly necessary, but removing the unuseddompurifydependency frompackage.jsonis advised.🤖 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 `@src/lib/components/docs/DocsMarkdown.svelte` around lines 94 - 95, Sanitize the HTML assigned to result before the {`@html`} render in DocsMarkdown, using an SSR-compatible sanitizer such as isomorphic-dompurify so SvelteKit server rendering does not crash; if text is guaranteed trusted local documentation instead, remove the unused dompurify dependency and keep the raw rendering contract explicit.
36-83: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winInitialize
MarkdownItoutside the reactive block.Instantiating
MarkdownItand re-registering all plugins inside Svelte's$derived.bycallback causes the entire parser to be rebuilt every timetextchanges.Consider initializing the
MarkdownItinstance outside the$derivedblock and using a local array variable to collect thetableOfContentsduring the.render()pass. This significantly improves performance during reactive updates.⚡ Proposed performance optimization
- const rendered = $derived.by(() => { - const tableOfContents: TocItem[] = []; - - const md = MarkdownIt({ + let currentToc: TocItem[] = []; + + const md = MarkdownIt({ html: false, linkify: true, }) .use(alert) .use(markdownItAnchor, { level: [2, 3], permalink: markdownItAnchor.permalink.linkInsideHeader({ symbol: "#", placement: "after", class: "header-anchor", ariaHidden: true, }), callback(token, info) { const level = Number(token.tag.slice(1)); if (level !== 2 && level !== 3) { return; } const inlineToken = token.children?.find( (child) => child.type === "inline", ); const title = inlineToken?.children ? getHeadingText(inlineToken.children) : info.title; - tableOfContents.push({ + currentToc.push({ id: info.slug, title: title || info.slug, level, }); }, }) .use(footnote) .use(fullEmoji); + + const rendered = $derived.by(() => { + currentToc = []; + const html = md.render(text); return { - html: md.render(text), - tableOfContents, + html, + tableOfContents: [...currentToc], }; });🤖 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 `@src/lib/components/docs/DocsMarkdown.svelte` around lines 36 - 83, Move the MarkdownIt construction and plugin registration out of the rendered $derived.by callback into a stable instance, while preserving the existing anchor callback behavior. Keep tableOfContents as a fresh local array for each reactive render and have the callback populate that array during md.render(text), so only rendering and TOC collection rerun when text changes.
🤖 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.
Inline comments:
In `@src/lib/components/docs/DocsMarkdown.svelte`:
- Line 9: Update the side-effect import in DocsMarkdown.svelte to reference the
package’s resolvable CSS entry, using the .css extension if that file exists;
otherwise add an ambient declaration for the extensionless
`@mdit/plugin-alert/style` module. Do not suppress the error with TypeScript
ignore directives.
---
Nitpick comments:
In `@src/lib/components/docs/DocsMarkdown.svelte`:
- Around line 94-95: Sanitize the HTML assigned to result before the {`@html`}
render in DocsMarkdown, using an SSR-compatible sanitizer such as
isomorphic-dompurify so SvelteKit server rendering does not crash; if text is
guaranteed trusted local documentation instead, remove the unused dompurify
dependency and keep the raw rendering contract explicit.
- Around line 36-83: Move the MarkdownIt construction and plugin registration
out of the rendered $derived.by callback into a stable instance, while
preserving the existing anchor callback behavior. Keep tableOfContents as a
fresh local array for each reactive render and have the callback populate that
array during md.render(text), so only rendering and TOC collection rerun when
text changes.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: baf7a8eb-1983-4aeb-9bda-c6ecbd907f29
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (9)
package.jsonsrc/lib/components/docs/DocsMarkdown.sveltesrc/lib/content/guides/cube-submission.mdsrc/lib/content/guides/submission-manual.mdsrc/lib/content/guides/vendor-submission.mdsrc/routes/(docs)/docs/+layout.sveltesrc/routes/(docs)/docs/submission-manual/+page.sveltesrc/routes/(docs)/docs/submission-manual/cubes/+page.sveltesrc/routes/(docs)/docs/submission-manual/vendors/+page.svelte
| import { footnote } from "@mdit/plugin-footnote"; | ||
| import { fullEmoji } from "@mdit/plugin-emoji"; | ||
|
|
||
| import "@mdit/plugin-alert/style"; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Resolve the missing type declarations for this side-effect import.
The pipeline is failing (svelte-check) because TypeScript cannot resolve this module. If the package provides a CSS file, append the .css extension to resolve the import:
import "`@mdit/plugin-alert/style.css`";Alternatively, if the extensionless import is correct, you must declare it in an ambient .d.ts file (e.g., declare module '@mdit/plugin-alert/style';) to satisfy the TypeScript compiler. As per coding guidelines, do not use @ts-ignore or @ts-expect-error.
🧰 Tools
🪛 GitHub Actions: Pull Request Check / 0_check.txt
[error] 9-9: svelte-check (TypeScript): Cannot find module or type declarations for side-effect import of '@mdit/plugin-alert/style'.
🪛 GitHub Actions: Pull Request Check / check
[error] 9-9: svelte-check (TypeScript): Cannot find module or type declarations for side-effect import of '@mdit/plugin-alert/style'. (ts) import "@mdit/plugin-alert/style";
🤖 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 `@src/lib/components/docs/DocsMarkdown.svelte` at line 9, Update the
side-effect import in DocsMarkdown.svelte to reference the package’s resolvable
CSS entry, using the .css extension if that file exists; otherwise add an
ambient declaration for the extensionless `@mdit/plugin-alert/style` module. Do
not suppress the error with TypeScript ignore directives.
Sources: Coding guidelines, Pipeline failures
…nd form validation
… and clean up unused props
…or improved vendor display
…error handling in cube submission process
…ing and vendor logo upload
1 participant
Summary by CodeRabbit
New Features
Documentation