-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
feat(component): code artifact preview #12801
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: canary
Are you sure you want to change the base?
Conversation
WalkthroughA new "code artifact preview" component was introduced, including its implementation, styling, and Storybook stories. The component supports syntax-highlighted code display, HTML preview in a sandboxed iframe, and streaming code rendering. Supporting CSS modules, a Shiki-based code highlighter, and necessary package dependencies were added or updated. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CodeArtifactPreview
participant CodeHighlight
participant ShikiHighlighter
participant Iframe
User->>CodeArtifactPreview: Render with code, language, previewable, complete
CodeArtifactPreview->>CodeHighlight: Highlight code (if in code view)
CodeHighlight->>ShikiHighlighter: Load highlighter, theme, language, highlight code
ShikiHighlighter-->>CodeHighlight: Return highlighted tokens
CodeHighlight-->>CodeArtifactPreview: Render highlighted code
CodeArtifactPreview->>Iframe: (If previewable & complete & preview selected) Render HTML preview
Iframe-->>CodeArtifactPreview: Display sandboxed HTML preview
Assessment against linked issues
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## canary #12801 +/- ##
==========================================
- Coverage 55.93% 55.62% -0.31%
==========================================
Files 2652 2652
Lines 125431 125431
Branches 19945 19881 -64
==========================================
- Hits 70160 69775 -385
- Misses 52969 53350 +381
- Partials 2302 2306 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
1b59948
to
1981bd8
Compare
1981bd8
to
e43415c
Compare
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: 4
🧹 Nitpick comments (5)
packages/frontend/component/package.json (1)
66-67
: Verify bundle impact of Shiki
shiki
adds ~1 MB of JS + a WASM blob. Consider checking the final bundle size or making this dependencypeer
to avoid duplications in host apps.packages/frontend/component/src/ui/code-artifact-preview/code-artifact-preview.tsx (2)
68-70
: DuplicateshowPreviewSwitch
computation – calculate once and pass downThe expression is calculated in both the header and the parent. Computing it once avoids divergence if the condition is ever tweaked.
-const showPreviewSwitch = previewable && complete; - <CodeArtifactPreviewHeader language={language} previewable={previewable} complete={complete} + showPreviewSwitch={showPreviewSwitch}…and make the header rely on the prop.
84-85
: Upper-casing language blindly can break multi-part identifiers
language.toUpperCase()
will convertc++
→C++
, but will also turntypescriptreact
intoTYPESCRIPTREACT
, which is less readable. Consider capitalising only the first letter or mapping common aliases to nicer labels.packages/frontend/component/src/ui/code-artifact-preview/code-artifact-preview.stories.tsx (1)
118-128
: Minor: avoid hard-coding fixed dimensions for the story container
style={{ width: '800px', height: '600px' }}
may clip on smaller viewports. Consider responsive sizing or Storybook’sparameters.layout: 'fullscreen'
instead.packages/frontend/component/src/ui/code-artifact-preview/code-artifact-preview.css.ts (1)
73-78
: Addloading: 'lazy'
to the iframe style for better performanceYou can set it directly on the element, but documenting it here reminds consumers.
export const previewIframe = style({ width: '100%', height: '100%', border: 'none', flex: 1, + selectors: { + '&[loading="lazy"]': {}, + }, });And then
<iframe loading="lazy" … />
in the JSX.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (8)
packages/frontend/component/.storybook/preview.css.ts
(1 hunks)packages/frontend/component/package.json
(1 hunks)packages/frontend/component/src/ui/code-artifact-preview/code-artifact-preview.css.ts
(1 hunks)packages/frontend/component/src/ui/code-artifact-preview/code-artifact-preview.stories.tsx
(1 hunks)packages/frontend/component/src/ui/code-artifact-preview/code-artifact-preview.tsx
(1 hunks)packages/frontend/component/src/ui/code-artifact-preview/code-highlight.css.ts
(1 hunks)packages/frontend/component/src/ui/code-artifact-preview/code-highlight.tsx
(1 hunks)packages/frontend/component/src/ui/code-artifact-preview/index.ts
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/frontend/component/src/ui/code-artifact-preview/code-highlight.css.ts (1)
packages/frontend/component/src/ui/code-artifact-preview/code-artifact-preview.css.ts (2)
container
(3-12)codeContainer
(52-56)
packages/frontend/component/src/ui/code-artifact-preview/code-artifact-preview.css.ts (1)
packages/frontend/component/src/ui/code-artifact-preview/code-highlight.css.ts (2)
container
(17-23)codeContainer
(43-51)
⏰ Context from checks skipped due to timeout of 90000ms (75)
- GitHub Check: Cloud E2E Test 3/6
- GitHub Check: Cloud E2E Test 2/6
- GitHub Check: Cloud E2E Test 6/6
- GitHub Check: Cloud E2E Test 5/6
- GitHub Check: Cloud Desktop E2E Test
- GitHub Check: Cloud E2E Test 4/6
- GitHub Check: Cloud E2E Test 1/6
- GitHub Check: Frontend Copilot E2E Test (4, 8)
- GitHub Check: Frontend Copilot E2E Test (6, 8)
- GitHub Check: Server E2E Test
- GitHub Check: Frontend Copilot E2E Test (1, 8)
- GitHub Check: Frontend Copilot E2E Test (2, 8)
- GitHub Check: Server Test (7, 8)
- GitHub Check: Server Copilot Api Test
- GitHub Check: Server Test (5, 8)
- GitHub Check: Server Test (6, 8)
- GitHub Check: Server Test (4, 8)
- GitHub Check: Check Git Status
- GitHub Check: Server Test (2, 8)
- GitHub Check: Server Test (3, 8)
- GitHub Check: Server Test (1, 8)
- GitHub Check: Server Test (0, 8)
- GitHub Check: Server Test with Elasticsearch
- GitHub Check: Unit Test (4)
- GitHub Check: Unit Test (1)
- GitHub Check: Unit Test (2)
- GitHub Check: Unit Test (5)
- GitHub Check: Unit Test (3)
- GitHub Check: test-build-mobile-app / build-android-web
- GitHub Check: test-build-mobile-app / build-ios-web
- GitHub Check: E2E Test (2)
- GitHub Check: y-octo binding test on aarch64-pc-windows-msvc
- GitHub Check: y-octo binding test on x86_64-pc-windows-msvc
- GitHub Check: E2E Mobile Test (1)
- GitHub Check: Run native tests
- GitHub Check: E2E Mobile Test (3)
- GitHub Check: E2E Mobile Test (5)
- GitHub Check: E2E Test (4)
- GitHub Check: E2E Mobile Test (4)
- GitHub Check: Typecheck
- GitHub Check: E2E Mobile Test (2)
- GitHub Check: Build AFFiNE native (x86_64-pc-windows-msvc)
- GitHub Check: E2E BlockSuite Cross Browser Test (2, webkit)
- GitHub Check: E2E Test (7)
- GitHub Check: Build AFFiNE native (aarch64-pc-windows-msvc)
- GitHub Check: E2E BlockSuite Cross Browser Test (2, chromium)
- GitHub Check: E2E BlockSuite Cross Browser Test (1, firefox)
- GitHub Check: E2E BlockSuite Cross Browser Test (1, webkit)
- GitHub Check: E2E BlockSuite Cross Browser Test (2, firefox)
- GitHub Check: E2E Test (9)
- GitHub Check: E2E Test (10)
- GitHub Check: E2E Test (5)
- GitHub Check: E2E BlockSuite Cross Browser Test (1, chromium)
- GitHub Check: E2E Test (6)
- GitHub Check: E2E Test (1)
- GitHub Check: E2E Test (3)
- GitHub Check: E2E Test (8)
- GitHub Check: E2E BlockSuite Test (8)
- GitHub Check: E2E BlockSuite Test (10)
- GitHub Check: E2E BlockSuite Test (4)
- GitHub Check: E2E BlockSuite Test (7)
- GitHub Check: E2E BlockSuite Test (9)
- GitHub Check: E2E BlockSuite Test (5)
- GitHub Check: E2E BlockSuite Test (6)
- GitHub Check: E2E BlockSuite Test (2)
- GitHub Check: E2E BlockSuite Test (1)
- GitHub Check: E2E BlockSuite Test (3)
- GitHub Check: fuzzing
- GitHub Check: Build @affine/electron renderer
- GitHub Check: loom thread test
- GitHub Check: Analyze (javascript, blocksuite)
- GitHub Check: Analyze (typescript, blocksuite)
- GitHub Check: Analyze (typescript, affine)
- GitHub Check: Analyze (javascript, affine)
- GitHub Check: Lint
🔇 Additional comments (5)
packages/frontend/component/.storybook/preview.css.ts (1)
9-10
: Change looks safeSetting
overflow: 'auto'
onbody
should not conflict with the later, more-specific.sb-main-fullscreen
rule and improves scroll behaviour in stories. No further action needed.packages/frontend/component/src/ui/code-artifact-preview/code-highlight.css.ts (1)
60-63
: Variable use is correct
opacity: startingOpacity
and the animation-duration var are both correctly wired viacreateVar
, and the fallback values are provided. Nice job.packages/frontend/component/src/ui/code-artifact-preview/index.ts (1)
1-4
: Barrel export LGTMStraight-forward re-export; no issues.
packages/frontend/component/src/ui/code-artifact-preview/code-artifact-preview.tsx (1)
49-51
: Relax the sandbox only if you really needallow-same-origin
allow-same-origin
re-grants the iframe the normal same-origin privileges for its (blob) origin.
Combined withallow-scripts
this lets arbitrary JS inside the preview poke atlocalStorage
, cookies for the blob origin, etc. If the HTML originates from untrusted user input, consider droppingallow-same-origin
(or at least gating it behind an explicit prop).packages/frontend/component/src/ui/code-artifact-preview/code-artifact-preview.css.ts (1)
30-43
: Unused.languageBadge
style – dead code?
languageBadge
is exported but not referenced in the TSX. If it’s obsolete, drop it; otherwise, wire it up to avoid shipping unused CSS.
color: token.color || undefined, | ||
fontStyle: token.fontStyle === 1 ? 'italic' : undefined, | ||
fontWeight: token.fontStyle === 2 ? 'bold' : undefined, | ||
textDecoration: token.fontStyle === 4 ? 'underline' : undefined, | ||
}} |
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
Handle combined fontStyle flags
fontStyle
is a bit-mask; equality checks miss combined cases (e.g., bold + italic).
- fontStyle: token.fontStyle === 1 ? 'italic' : undefined,
- fontWeight: token.fontStyle === 2 ? 'bold' : undefined,
- textDecoration: token.fontStyle === 4 ? 'underline' : undefined,
+ fontStyle: token.fontStyle & 1 ? 'italic' : undefined,
+ fontWeight: token.fontStyle & 2 ? 'bold' : undefined,
+ textDecoration: token.fontStyle & 4 ? 'underline' : undefined,
📝 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.
color: token.color || undefined, | |
fontStyle: token.fontStyle === 1 ? 'italic' : undefined, | |
fontWeight: token.fontStyle === 2 ? 'bold' : undefined, | |
textDecoration: token.fontStyle === 4 ? 'underline' : undefined, | |
}} | |
color: token.color || undefined, | |
fontStyle: token.fontStyle & 1 ? 'italic' : undefined, | |
fontWeight: token.fontStyle & 2 ? 'bold' : undefined, | |
textDecoration: token.fontStyle & 4 ? 'underline' : undefined, | |
}} |
🤖 Prompt for AI Agents
In packages/frontend/component/src/ui/code-artifact-preview/code-highlight.tsx
around lines 176 to 180, the fontStyle property is checked using equality which
fails to handle combined bit-mask flags like bold and italic together. Update
the checks to use bitwise AND operations to detect each style flag
independently, allowing multiple styles (italic, bold, underline) to be applied
simultaneously based on the bits set in fontStyle.
const core = await createHighlighterCore({ | ||
engine: createOnigurumaEngine(() => getWasm), | ||
}); | ||
|
||
// Load themes |
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.
Incorrect Shiki API call
codeToTokensBase
is not a public API in Shiki v3; the public method is codeToTokens
. Calling a private helper may break with minor updates.
- const tokens = highlighter.codeToTokensBase(code, {
+ const tokens = highlighter.codeToTokens(code, {
lang: language,
theme,
});
📝 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.
const core = await createHighlighterCore({ | |
engine: createOnigurumaEngine(() => getWasm), | |
}); | |
// Load themes | |
const tokens = highlighter.codeToTokens(code, { | |
lang: language, | |
theme, | |
}); |
🤖 Prompt for AI Agents
In packages/frontend/component/src/ui/code-artifact-preview/code-highlight.tsx
between lines 39 and 43, replace the usage of the private Shiki method
`codeToTokensBase` with the public API method `codeToTokens`. This involves
changing the function call to use `codeToTokens` instead, ensuring compatibility
with Shiki v3 and preventing breakage from future updates.
// Create blob URL for iframe preview | ||
const useHtmlBlobUrl = (code: string) => { | ||
const previewUrl = useMemo(() => { | ||
const blob = new Blob([code], { type: 'text/html' }); | ||
return URL.createObjectURL(blob); | ||
}, [code]); | ||
|
||
// Cleanup blob URL when component unmounts or URL changes | ||
React.useEffect(() => { | ||
return () => { | ||
if (previewUrl) { | ||
URL.revokeObjectURL(previewUrl); | ||
} | ||
}; | ||
}, [previewUrl]); | ||
|
||
return previewUrl; | ||
}; |
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
Blob URL gets recreated on every re-render – switch to useEffect
to avoid needless object churn
useMemo
will run on every render where code
changes, which is fine, but when the parent re-renders for unrelated state updates, the Blob is recreated again and immediately revoked by the old cleanup, resulting in two super-short-lived objects and extra GC work.
-const previewUrl = useMemo(() => {
- const blob = new Blob([code], { type: 'text/html' });
- return URL.createObjectURL(blob);
-}, [code]);
+const [previewUrl, setPreviewUrl] = useState<string>(() => {
+ const blob = new Blob([code], { type: 'text/html' });
+ return URL.createObjectURL(blob);
+});
+
+useEffect(() => {
+ const blob = new Blob([code], { type: 'text/html' });
+ const url = URL.createObjectURL(blob);
+ setPreviewUrl(url);
+ return () => URL.revokeObjectURL(url);
+}, [code]);
Reduces churn and ensures revocation happens exactly once per URL.
📝 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.
// Create blob URL for iframe preview | |
const useHtmlBlobUrl = (code: string) => { | |
const previewUrl = useMemo(() => { | |
const blob = new Blob([code], { type: 'text/html' }); | |
return URL.createObjectURL(blob); | |
}, [code]); | |
// Cleanup blob URL when component unmounts or URL changes | |
React.useEffect(() => { | |
return () => { | |
if (previewUrl) { | |
URL.revokeObjectURL(previewUrl); | |
} | |
}; | |
}, [previewUrl]); | |
return previewUrl; | |
}; | |
// Create blob URL for iframe preview | |
const useHtmlBlobUrl = (code: string) => { | |
const [previewUrl, setPreviewUrl] = useState<string>(() => { | |
const blob = new Blob([code], { type: 'text/html' }); | |
return URL.createObjectURL(blob); | |
}); | |
useEffect(() => { | |
const blob = new Blob([code], { type: 'text/html' }); | |
const url = URL.createObjectURL(blob); | |
setPreviewUrl(url); | |
return () => URL.revokeObjectURL(url); | |
}, [code]); | |
// Cleanup blob URL when component unmounts or URL changes | |
React.useEffect(() => { | |
return () => { | |
if (previewUrl) { | |
URL.revokeObjectURL(previewUrl); | |
} | |
}; | |
}, [previewUrl]); | |
return previewUrl; | |
}; |
🤖 Prompt for AI Agents
In
packages/frontend/component/src/ui/code-artifact-preview/code-artifact-preview.tsx
around lines 22 to 39, the Blob URL is currently created inside useMemo, which
causes it to be recreated and revoked on every re-render even if unrelated to
code changes. To fix this, move the Blob URL creation logic into a useEffect
hook that runs only when the code changes, store the URL in a state variable,
and revoke the previous URL inside the cleanup function. This ensures the Blob
URL is created and revoked exactly once per code change, reducing unnecessary
object churn and garbage collection.
function useStreamCode(code: string) { | ||
const [index, setIndex] = useState(0); | ||
|
||
useEffect(() => { | ||
setIndex(0); | ||
const interval = setInterval(() => { | ||
setIndex(prev => { | ||
const charsToAdd = Math.min( | ||
Math.floor(Math.random() * 4) + 2, | ||
code.length - prev | ||
); | ||
return prev + charsToAdd; | ||
}); | ||
}, 16); | ||
|
||
return () => clearInterval(interval); | ||
}, [code]); | ||
|
||
return code.slice(0, index); | ||
} |
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
useStreamCode
keeps the interval running forever – clear it once streaming completes
After the full string is revealed, charsToAdd
becomes 0
; the state setter still fires and the interval continues consuming CPU. Clear it when index === code.length
.
-const interval = setInterval(() => {
- setIndex(prev => {
- const charsToAdd = Math.min(
- Math.floor(Math.random() * 4) + 2,
- code.length - prev
- );
- return prev + charsToAdd;
- });
-}, 16);
+const interval = setInterval(() => {
+ setIndex(prev => {
+ if (prev >= code.length) {
+ clearInterval(interval);
+ return prev;
+ }
+ const charsToAdd = Math.min(
+ Math.floor(Math.random() * 4) + 2,
+ code.length - prev
+ );
+ return prev + charsToAdd;
+ });
+}, 16);
Prevents unnecessary work and potential memory leaks when the story tab stays open for a long time.
📝 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.
function useStreamCode(code: string) { | |
const [index, setIndex] = useState(0); | |
useEffect(() => { | |
setIndex(0); | |
const interval = setInterval(() => { | |
setIndex(prev => { | |
const charsToAdd = Math.min( | |
Math.floor(Math.random() * 4) + 2, | |
code.length - prev | |
); | |
return prev + charsToAdd; | |
}); | |
}, 16); | |
return () => clearInterval(interval); | |
}, [code]); | |
return code.slice(0, index); | |
} | |
function useStreamCode(code: string) { | |
const [index, setIndex] = useState(0); | |
useEffect(() => { | |
setIndex(0); | |
const interval = setInterval(() => { | |
setIndex(prev => { | |
if (prev >= code.length) { | |
clearInterval(interval); | |
return prev; | |
} | |
const charsToAdd = Math.min( | |
Math.floor(Math.random() * 4) + 2, | |
code.length - prev | |
); | |
return prev + charsToAdd; | |
}); | |
}, 16); | |
return () => clearInterval(interval); | |
}, [code]); | |
return code.slice(0, index); | |
} |
🤖 Prompt for AI Agents
In
packages/frontend/component/src/ui/code-artifact-preview/code-artifact-preview.stories.tsx
between lines 93 and 112, the useStreamCode hook sets an interval that never
clears after the full code string is revealed, causing unnecessary CPU usage.
Modify the interval callback to check if the updated index reaches the code
length, and if so, clear the interval to stop further updates. This ensures the
interval is properly cleaned up once streaming completes.
fix AF-2691
PR Dependency Tree
This tree was auto-generated by Charcoal
Summary by CodeRabbit
New Features
Style
Chores