Feature/preview external #225
Conversation
WalkthroughAdds a BlockViewer layout and a DynamicWebsitePreview with iframe URL/loading/error flows and optional cross-origin theming; introduces hooks and types for iframe theming and website preview, a public live-preview embed script, an error boundary, UI tab changes, SocialLink API update, and CodeBlock theming consolidation. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Toolbar
participant Preview as DynamicWebsitePreview
participant WP as useWebsitePreview
participant Iframe as <iframe>
participant TI as useIframeThemeInjector
participant Embed as live-preview.js
User->>Toolbar: enter URL / choose viewport / load
Toolbar->>WP: setInputUrl() / loadUrl()
WP->>Iframe: set src (normalized, cache-bust)
Iframe-->>WP: load / error events -> update state
WP->>Preview: update loading/error/ui
Iframe->>TI: onLoad init
alt allowCrossOrigin
TI->>Iframe: postMessage PING / CHECK_SHADCN / THEME_UPDATE
Iframe->>Embed: embed script receives messages
Embed-->>TI: PONG / SHADCN_STATUS / THEME_APPLIED
TI-->>Preview: status updates
else same-origin
TI->>Iframe: apply theme directly
TI-->>Preview: status supported
end
sequenceDiagram
autonumber
participant ParentApp
participant Iframe as <iframe> (page)
participant Embed as live-preview.js
ParentApp->>Iframe: load page with embed script
Embed-->>ParentApp: EMBED_LOADED
ParentApp->>Embed: CHECK_SHADCN
Embed-->>ParentApp: SHADCN_STATUS
ParentApp->>Embed: THEME_UPDATE
Embed->>Iframe: apply theme & fonts
Embed-->>ParentApp: THEME_APPLIED
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
components/block-viewer.tsx(1 hunks)components/dynamic-website-preview.tsx(1 hunks)components/editor/theme-preview-panel.tsx(5 hunks)components/error-boundary.tsx(1 hunks)components/examples/custom/index.tsx(1 hunks)components/social-link.tsx(1 hunks)hooks/use-iframe-theme-injector.ts(1 hunks)hooks/use-website-preview.ts(1 hunks)public/live-preview-embed-script.js(1 hunks)types/live-preview-embed.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
types/live-preview-embed.ts (1)
types/editor.ts (1)
ThemeEditorState(18-27)
hooks/use-iframe-theme-injector.ts (4)
store/editor-store.ts (1)
useEditorStore(34-231)types/live-preview-embed.ts (3)
IframeStatus(35-42)EmbedMessage(25-33)MESSAGE(4-13)utils/apply-theme.ts (1)
applyThemeToElement(54-69)public/live-preview-embed-script.js (1)
handleMessage(156-195)
components/examples/custom/index.tsx (1)
components/dynamic-website-preview.tsx (1)
DynamicWebsitePreview(53-80)
components/editor/theme-preview-panel.tsx (1)
components/examples/custom/index.tsx (1)
CustomDemo(3-5)
components/block-viewer.tsx (3)
lib/utils.ts (1)
cn(6-8)components/tooltip-wrapper.tsx (1)
TooltipWrapper(7-35)components/error-boundary.tsx (1)
ComponentErrorBoundary(4-40)
components/social-link.tsx (1)
lib/utils.ts (1)
cn(6-8)
components/dynamic-website-preview.tsx (9)
components/block-viewer.tsx (3)
BlockViewerProvider(28-43)BlockViewerToolbar(69-123)BlockViewerDisplay(125-176)lib/utils.ts (1)
cn(6-8)hooks/use-website-preview.ts (1)
useWebsitePreview(50-146)hooks/use-iframe-theme-injector.ts (1)
useIframeThemeInjector(18-175)components/copy-button.tsx (1)
CopyButton(18-35)components/editor/ai/loading-logo.tsx (1)
LoadingLogo(4-20)types/live-preview-embed.ts (1)
IframeStatus(35-42)components/horizontal-scroll-area.tsx (1)
HorizontalScrollArea(9-73)components/social-link.tsx (1)
SocialLink(8-23)
🪛 Biome (2.1.2)
hooks/use-iframe-theme-injector.ts
[error] 120-120: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 133-133: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
public/live-preview-embed-script.js
[error] 177-177: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Vercel Agent Review
🔇 Additional comments (6)
components/social-link.tsx (5)
1-2: LGTM: Imports are clean and tree-shakeable.The imports follow best practices with named imports from lucide-react for optimal tree-shaking.
Based on learnings: If your project uses React 19, be aware that lucide-react may have peer dependency warnings or TypeScript type issues until maintainers publish updated releases. The runtime API is stable, but you may need to address typing errors if they arise.
4-5: LGTM: Interface design is clean and backward-compatible.Extending
React.ComponentProps<"a">provides all standard anchor attributes while the optionalshowIconprop maintains backward compatibility with existing usage.
8-8: LGTM: Function signature is correct.The
showIconparameter with a default value offalseensures backward compatibility.
14-17: LGTM: className composition follows best practices.Using the
cnutility ensures proper Tailwind class merging and allows consumer-provided classes to override base styles correctly.
20-20: Verify the group-hover class usage.The icon uses
group-hover/link:rotate-45, but the parent<a>element doesn't have agrouporgroup/linkclass. This hover effect won't work unless consumers add the group class via theclassNameprop.Consider either:
- Adding the group class to the base styles in line 15, or
- Documenting that consumers should include the group class when using
showIconApply this diff if you want the hover effect to work by default:
className={cn( - "text-foreground/60 hover:text-foreground inline-flex w-fit items-center transition-colors", + "group/link text-foreground/60 hover:text-foreground inline-flex w-fit items-center transition-colors", className )}types/live-preview-embed.ts (1)
4-33: Typed message contract looks solid.The literal map plus discriminated union give downstream handlers exhaustive, type-safe coverage of every embed message variant.
| const resizablePanelRef = React.useRef<ImperativePanelHandle>(null); | ||
| const [toggleValue, setToggleValue] = React.useState("100"); |
There was a problem hiding this comment.
Fix the ref typing to accept null.
React.useRef<ImperativePanelHandle>(null) fails under strictNullChecks because null isn’t assignable to that generic. Widen the type so the provider compiles.
- const resizablePanelRef = React.useRef<ImperativePanelHandle>(null);
+ const resizablePanelRef = React.useRef<ImperativePanelHandle | null>(null);📝 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 resizablePanelRef = React.useRef<ImperativePanelHandle>(null); | |
| const [toggleValue, setToggleValue] = React.useState("100"); | |
| const resizablePanelRef = React.useRef<ImperativePanelHandle | null>(null); | |
| const [toggleValue, setToggleValue] = React.useState("100"); |
🤖 Prompt for AI Agents
In components/block-viewer.tsx around lines 29 to 30, the useRef is typed as
React.useRef<ImperativePanelHandle>(null) which fails under strictNullChecks;
change the ref type to allow null (e.g. use React.useRef<ImperativePanelHandle |
null>(null) or the equivalent MutableRefObject type) so the initial null value
is assignable and the provider compiles.
| constructor(props: { children: React.ReactNode; name: string; fallback: React.ReactNode }) { | ||
| super(props); | ||
| this.state = { hasError: false }; | ||
| } |
There was a problem hiding this comment.
Constructor makes optional prop mandatory
The constructor parameter marks fallback as required even though the component’s declared props make it optional. This forces callers to supply fallback and will trigger a TypeScript error anywhere it’s omitted while the component’s type says it’s optional. Update the constructor signature to accept an optional fallback to match the class props.
- constructor(props: { children: React.ReactNode; name: string; fallback: React.ReactNode }) {
+ constructor(props: { children: React.ReactNode; name: string; fallback?: React.ReactNode }) {📝 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.
| constructor(props: { children: React.ReactNode; name: string; fallback: React.ReactNode }) { | |
| super(props); | |
| this.state = { hasError: false }; | |
| } | |
| constructor(props: { children: React.ReactNode; name: string; fallback?: React.ReactNode }) { | |
| super(props); | |
| this.state = { hasError: false }; | |
| } |
🤖 Prompt for AI Agents
In components/error-boundary.tsx around lines 8 to 11, the constructor currently
types props so that fallback is required, which contradicts the component's
declared props where fallback is optional; update the constructor parameter to
match by marking fallback optional (e.g., fallback?: React.ReactNode) while
keeping children and name as before, then call super(props) and initialize state
as before.
| case MESSAGE.SHADCN_STATUS: | ||
| const { supported } = message.payload; | ||
| if (supported) { | ||
| setStatus("supported"); | ||
| setThemeInjectionError(null); | ||
| } else { | ||
| setStatus("unsupported"); | ||
| setThemeInjectionError( | ||
| "Live theme preview requires shadcn/ui setup. Please make sure that the basic shadcn/ui variables are configured correctly." | ||
| ); | ||
| } | ||
| break; | ||
|
|
||
| case MESSAGE.EMBED_ERROR: | ||
| const { error } = message.payload; | ||
| console.error("Tweakcn Embed: Error from iframe:", error); | ||
| setStatus("error"); | ||
| setThemeInjectionError(error); | ||
| break; |
There was a problem hiding this comment.
Wrap switch-case declarations in blocks
noSwitchDeclarations is firing because const declarations in these cases aren’t scoped. Without braces they leak into the shared switch block and will fail lint. Wrap each case body in braces so the declarations stay local and the lint error clears.
- case MESSAGE.SHADCN_STATUS:
- const { supported } = message.payload;
+ case MESSAGE.SHADCN_STATUS: {
+ const { supported } = message.payload;
if (supported) {
setStatus("supported");
setThemeInjectionError(null);
} else {
setStatus("unsupported");
setThemeInjectionError(
"Live theme preview requires shadcn/ui setup. Please make sure that the basic shadcn/ui variables are configured correctly."
);
}
- break;
-
- case MESSAGE.EMBED_ERROR:
- const { error } = message.payload;
+ break;
+ }
+
+ case MESSAGE.EMBED_ERROR: {
+ const { error } = message.payload;
console.error("Tweakcn Embed: Error from iframe:", error);
setStatus("error");
setThemeInjectionError(error);
- break;
+ break;
+ }📝 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.
| case MESSAGE.SHADCN_STATUS: | |
| const { supported } = message.payload; | |
| if (supported) { | |
| setStatus("supported"); | |
| setThemeInjectionError(null); | |
| } else { | |
| setStatus("unsupported"); | |
| setThemeInjectionError( | |
| "Live theme preview requires shadcn/ui setup. Please make sure that the basic shadcn/ui variables are configured correctly." | |
| ); | |
| } | |
| break; | |
| case MESSAGE.EMBED_ERROR: | |
| const { error } = message.payload; | |
| console.error("Tweakcn Embed: Error from iframe:", error); | |
| setStatus("error"); | |
| setThemeInjectionError(error); | |
| break; | |
| case MESSAGE.SHADCN_STATUS: { | |
| const { supported } = message.payload; | |
| if (supported) { | |
| setStatus("supported"); | |
| setThemeInjectionError(null); | |
| } else { | |
| setStatus("unsupported"); | |
| setThemeInjectionError( | |
| "Live theme preview requires shadcn/ui setup. Please make sure that the basic shadcn/ui variables are configured correctly." | |
| ); | |
| } | |
| break; | |
| } | |
| case MESSAGE.EMBED_ERROR: { | |
| const { error } = message.payload; | |
| console.error("Tweakcn Embed: Error from iframe:", error); | |
| setStatus("error"); | |
| setThemeInjectionError(error); | |
| break; | |
| } |
🧰 Tools
🪛 Biome (2.1.2)
[error] 120-120: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 133-133: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🤖 Prompt for AI Agents
In hooks/use-iframe-theme-injector.ts around lines 119 to 137, the const
declarations inside the MESSAGE.SHADCN_STATUS and MESSAGE.EMBED_ERROR cases are
not block-scoped and trigger the noSwitchDeclarations lint rule; wrap the body
of each case in braces (e.g., case MESSAGE.SHADCN_STATUS: { ... break; } and
case MESSAGE.EMBED_ERROR: { ... break; }) so the const bindings are scoped to
each case and the lint error is resolved.
hooks/use-website-preview.ts
Outdated
| const clearLoadingTimeout = () => { | ||
| if (loadingTimeoutRef.current) { | ||
| clearTimeout(loadingTimeoutRef.current); | ||
| loadingTimeoutRef.current = null; | ||
| } | ||
| }; | ||
|
|
||
| const handleIframeLoad = useCallback(() => { | ||
| clearLoadingTimeout(); | ||
| dispatch({ type: "SET_LOAD_SUCCESS" }); | ||
| }, []); | ||
|
|
||
| const handleIframeError = useCallback(() => { | ||
| clearLoadingTimeout(); | ||
| dispatch({ | ||
| type: "SET_LOAD_ERROR", | ||
| payload: | ||
| "Failed to load website. This could be due to CORS restrictions or the site blocking iframes.", | ||
| }); | ||
| }, []); | ||
|
|
||
| useEffect(() => { | ||
| if (state.isLoading && state.currentUrl) { | ||
| clearLoadingTimeout(); | ||
| loadingTimeoutRef.current = setTimeout(() => { | ||
| dispatch({ | ||
| type: "SET_LOAD_ERROR", | ||
| payload: "Loading timeout - the website may be taking too long to respond", | ||
| }); | ||
| loadingTimeoutRef.current = null; | ||
| }, LOADING_TIMEOUT_MS); | ||
| return clearLoadingTimeout; | ||
| } | ||
| }, [state.isLoading, state.currentUrl]); |
There was a problem hiding this comment.
Fix missing hook dependencies for timeout cleanup.
clearLoadingTimeout is recreated every render but used inside this effect and both iframe callbacks without being in their dependency arrays, so the default react-hooks/exhaustive-deps rule will fail CI. Wrap the helper in useCallback and reference it from the effect/callbacks to keep lint happy and always call the latest cleanup.
- const clearLoadingTimeout = () => {
+ const clearLoadingTimeout = useCallback(() => {
if (loadingTimeoutRef.current) {
clearTimeout(loadingTimeoutRef.current);
loadingTimeoutRef.current = null;
}
- };
+ }, []);
- const handleIframeLoad = useCallback(() => {
+ const handleIframeLoad = useCallback(() => {
clearLoadingTimeout();
dispatch({ type: "SET_LOAD_SUCCESS" });
- }, []);
+ }, [clearLoadingTimeout]);
- const handleIframeError = useCallback(() => {
+ const handleIframeError = useCallback(() => {
clearLoadingTimeout();
dispatch({
type: "SET_LOAD_ERROR",
payload:
"Failed to load website. This could be due to CORS restrictions or the site blocking iframes.",
});
- }, []);
+ }, [clearLoadingTimeout]);
- useEffect(() => {
+ useEffect(() => {
if (state.isLoading && state.currentUrl) {
clearLoadingTimeout();
loadingTimeoutRef.current = setTimeout(() => {
dispatch({
type: "SET_LOAD_ERROR",
payload: "Loading timeout - the website may be taking too long to respond",
});
loadingTimeoutRef.current = null;
}, LOADING_TIMEOUT_MS);
return clearLoadingTimeout;
}
- }, [state.isLoading, state.currentUrl]);
+ }, [state.isLoading, state.currentUrl, clearLoadingTimeout]);📝 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 clearLoadingTimeout = () => { | |
| if (loadingTimeoutRef.current) { | |
| clearTimeout(loadingTimeoutRef.current); | |
| loadingTimeoutRef.current = null; | |
| } | |
| }; | |
| const handleIframeLoad = useCallback(() => { | |
| clearLoadingTimeout(); | |
| dispatch({ type: "SET_LOAD_SUCCESS" }); | |
| }, []); | |
| const handleIframeError = useCallback(() => { | |
| clearLoadingTimeout(); | |
| dispatch({ | |
| type: "SET_LOAD_ERROR", | |
| payload: | |
| "Failed to load website. This could be due to CORS restrictions or the site blocking iframes.", | |
| }); | |
| }, []); | |
| useEffect(() => { | |
| if (state.isLoading && state.currentUrl) { | |
| clearLoadingTimeout(); | |
| loadingTimeoutRef.current = setTimeout(() => { | |
| dispatch({ | |
| type: "SET_LOAD_ERROR", | |
| payload: "Loading timeout - the website may be taking too long to respond", | |
| }); | |
| loadingTimeoutRef.current = null; | |
| }, LOADING_TIMEOUT_MS); | |
| return clearLoadingTimeout; | |
| } | |
| }, [state.isLoading, state.currentUrl]); | |
| const clearLoadingTimeout = useCallback(() => { | |
| if (loadingTimeoutRef.current) { | |
| clearTimeout(loadingTimeoutRef.current); | |
| loadingTimeoutRef.current = null; | |
| } | |
| }, []); | |
| const handleIframeLoad = useCallback(() => { | |
| clearLoadingTimeout(); | |
| dispatch({ type: "SET_LOAD_SUCCESS" }); | |
| }, [clearLoadingTimeout]); | |
| const handleIframeError = useCallback(() => { | |
| clearLoadingTimeout(); | |
| dispatch({ | |
| type: "SET_LOAD_ERROR", | |
| payload: | |
| "Failed to load website. This could be due to CORS restrictions or the site blocking iframes.", | |
| }); | |
| }, [clearLoadingTimeout]); | |
| useEffect(() => { | |
| if (state.isLoading && state.currentUrl) { | |
| clearLoadingTimeout(); | |
| loadingTimeoutRef.current = setTimeout(() => { | |
| dispatch({ | |
| type: "SET_LOAD_ERROR", | |
| payload: "Loading timeout - the website may be taking too long to respond", | |
| }); | |
| loadingTimeoutRef.current = null; | |
| }, LOADING_TIMEOUT_MS); | |
| return clearLoadingTimeout; | |
| } | |
| }, [state.isLoading, state.currentUrl, clearLoadingTimeout]); |
🤖 Prompt for AI Agents
In hooks/use-website-preview.ts around lines 55 to 88, the clearLoadingTimeout
helper is recreated each render and used inside effect and iframe callbacks
which causes missing-react-hook dependency lint failures; wrap
clearLoadingTimeout in useCallback (using loadingTimeoutRef which is stable, so
an empty deps array is fine) and then add clearLoadingTimeout to the dependency
arrays of handleIframeLoad, handleIframeError, and the useEffect so the
callbacks/effect always reference the latest memoized cleanup function and the
linter stops failing.
| case TWEAKCN_MESSAGE.PING: | ||
| sendMessageToParent({ type: TWEAKCN_MESSAGE.PONG }); | ||
| break; | ||
|
|
||
| case TWEAKCN_MESSAGE.CHECK_SHADCN: | ||
| const supportInfo = checkShadcnSupport(); | ||
| sendMessageToParent({ | ||
| type: TWEAKCN_MESSAGE.SHADCN_STATUS, | ||
| payload: supportInfo, | ||
| }); | ||
| break; | ||
|
|
There was a problem hiding this comment.
Wrap this switch case body in a block
Declaring const supportInfo directly under case CHECK_SHADCN violates the noSwitchDeclarations rule (Biome raises an error) because the declaration is visible to other cases. Wrap the case body in braces so the declaration stays scoped and lint passes.
- case TWEAKCN_MESSAGE.CHECK_SHADCN:
- const supportInfo = checkShadcnSupport();
- sendMessageToParent({
- type: TWEAKCN_MESSAGE.SHADCN_STATUS,
- payload: supportInfo,
- });
- break;
+ case TWEAKCN_MESSAGE.CHECK_SHADCN: {
+ const supportInfo = checkShadcnSupport();
+ sendMessageToParent({
+ type: TWEAKCN_MESSAGE.SHADCN_STATUS,
+ payload: supportInfo,
+ });
+ break;
+ }📝 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.
| case TWEAKCN_MESSAGE.PING: | |
| sendMessageToParent({ type: TWEAKCN_MESSAGE.PONG }); | |
| break; | |
| case TWEAKCN_MESSAGE.CHECK_SHADCN: | |
| const supportInfo = checkShadcnSupport(); | |
| sendMessageToParent({ | |
| type: TWEAKCN_MESSAGE.SHADCN_STATUS, | |
| payload: supportInfo, | |
| }); | |
| break; | |
| case TWEAKCN_MESSAGE.PING: | |
| sendMessageToParent({ type: TWEAKCN_MESSAGE.PONG }); | |
| break; | |
| case TWEAKCN_MESSAGE.CHECK_SHADCN: { | |
| const supportInfo = checkShadcnSupport(); | |
| sendMessageToParent({ | |
| type: TWEAKCN_MESSAGE.SHADCN_STATUS, | |
| payload: supportInfo, | |
| }); | |
| break; | |
| } |
🧰 Tools
🪛 Biome (2.1.2)
[error] 177-177: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🤖 Prompt for AI Agents
In public/live-preview-embed-script.js around lines 172 to 183, the case
TWEAKCN_MESSAGE.CHECK_SHADCN declares const supportInfo directly under the case
which violates noSwitchDeclarations; wrap the entire case body in a block (add {
... } after the case label), place the const supportInfo and sendMessageToParent
call inside that block, and keep the break inside the block so the declaration
is scoped to the case and the linter error is resolved.
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
| { children: React.ReactNode; name: string; fallback?: React.ReactNode }, | ||
| { hasError: boolean } | ||
| > { | ||
| constructor(props: { children: React.ReactNode; name: string; fallback: React.ReactNode }) { |
There was a problem hiding this comment.
| constructor(props: { children: React.ReactNode; name: string; fallback: React.ReactNode }) { | |
| constructor(props: { children: React.ReactNode; name: string; fallback?: React.ReactNode }) { |
Constructor parameter type doesn't match component props interface, making the fallback prop required when it should be optional.
View Details
Analysis
Constructor parameter type mismatch in ComponentErrorBoundary makes fallback required when it should be optional
What fails: ComponentErrorBoundary constructor expects fallback: React.ReactNode (required) but component props interface declares fallback?: React.ReactNode (optional)
How to reproduce:
const props = { children: null, name: "test" }; // No fallback prop
new ComponentErrorBoundary(props); // TypeScript errorResult: TypeScript error Property 'fallback' is optional in type 'ComponentProps' but required in type '{ children: ReactNode; name: string; fallback: ReactNode; }'
Expected: Should compile successfully since fallback is declared optional in the component props interface per React TypeScript patterns
* feat: Add Block Viewer for Preview Panel * feat: support theme injection for external websites
* feat: Add support for loading Google Fonts in the embed script. * feat: Add error state and UI feedback
jnsahaj
force-pushed
the
feature/preview-external
branch
from
94e0e9e to
f029535
Compare
|
Deployment failed with the following error: View Documentation: https://vercel.com/docs/accounts/team-members-and-roles |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
public/live-preview-embed-script.js (1)
205-212: Fix Biome noSwitchDeclarations: wrap case body in a blockDeclare
supportInfoinside a block to scope it to the case and satisfy the linter.- case TWEAKCN_MESSAGE.CHECK_SHADCN: - const supportInfo = checkShadcnSupport(); - sendMessageToParent({ - type: TWEAKCN_MESSAGE.SHADCN_STATUS, - payload: supportInfo, - }); - break; + case TWEAKCN_MESSAGE.CHECK_SHADCN: { + const supportInfo = checkShadcnSupport(); + sendMessageToParent({ + type: TWEAKCN_MESSAGE.SHADCN_STATUS, + payload: supportInfo, + }); + break; + }
🧹 Nitpick comments (2)
public/live-preview-embed-script.js (2)
134-143: Load fonts from the active (light/dark) style setCurrently fonts are derived from lightStyles only. Use the active mode to derive font families.
- // Apply dark mode overrides - const darkStyles = themeStyles.dark; + // Apply dark mode overrides + const darkStyles = themeStyles.dark || {}; if (mode === "dark" && darkStyles) { for (const [key, value] of Object.entries(darkStyles)) { applyStyleProperty(root, key, value); } } - loadThemeFonts(root, lightStyles); + // Load fonts from the effective style set + const fontStyles = mode === "dark" ? { ...lightStyles, ...darkStyles } : lightStyles; + loadThemeFonts(root, fontStyles);
24-30: Return missing SHADCN variables to aid diagnosticsExpose which CSS vars are missing to help the host fix theme issues.
const hasSupport = REQUIRED_SHADCN_VARS.every( (v) => rootStyles.getPropertyValue(v).trim() !== "" ); - return { supported: hasSupport }; + const missing = hasSupport + ? [] + : REQUIRED_SHADCN_VARS.filter((v) => rootStyles.getPropertyValue(v).trim() === ""); + return { supported: hasSupport, missing };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
public/live-preview-embed-script.js(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
public/live-preview-embed-script.js (3)
utils/fonts/index.ts (1)
extractFontFamily(188-200)utils/fonts/google-fonts.ts (2)
buildFontCssUrl(34-38)loadGoogleFont(42-54)utils/theme-fonts.ts (1)
fonts(37-68)
🪛 Biome (2.1.2)
public/live-preview-embed-script.js
[error] 206-206: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Vercel Agent Review
| // ----- MESSAGE SENDING ----- | ||
| function sendMessageToParent(message) { | ||
| if (window.parent && window.parent !== window) { | ||
| try { | ||
| window.parent.postMessage(message, "*"); | ||
| } catch (error) { | ||
| console.warn("Tweakcn Embed: Failed to send message to parent:", error); | ||
| } | ||
| } | ||
| }; | ||
|
|
There was a problem hiding this comment.
Restrict postMessage targetOrigin to validated parent origin
Using "*" is permissive. Capture the allowed origin from validated messages and use it for all replies.
+// Validated parent origin will be set after the first allowed message
+let PARENT_ORIGIN = "*";
function sendMessageToParent(message) {
if (window.parent && window.parent !== window) {
try {
- window.parent.postMessage(message, "*");
+ window.parent.postMessage(message, PARENT_ORIGIN);
} catch (error) {
console.warn("Tweakcn Embed: Failed to send message to parent:", error);
}
}
};- if (!ALLOWED_ORIGINS.includes(event.origin)){
+ if (!ALLOWED_ORIGINS.includes(event.origin)){
sendMessageToParent({ type: TWEAKCN_MESSAGE.EMBED_ERROR, payload: { error: "Origin not allowed. Preview failed to establish the connection with tweakcn." } });
return;
- } ;
+ } ;
+ // Lock in the validated parent origin for subsequent replies
+ PARENT_ORIGIN = event.origin;Note: Consider making ALLOWED_ORIGINS configurable (e.g., via script data attribute) for non-prod environments.
Also applies to: 191-198
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
hooks/use-website-preview.ts (1)
55-88: Stabilize timeout cleanup helpers (lint + runtime safety)
clearLoadingTimeoutchanges every render but the callbacks/effect omit it from deps. This still triggersreact-hooks/exhaustive-depsand risks stale closures. Wrap it inuseCallbackand add it to the dependent hooks as previously suggested.- const clearLoadingTimeout = () => { + const clearLoadingTimeout = useCallback(() => { if (loadingTimeoutRef.current) { clearTimeout(loadingTimeoutRef.current); loadingTimeoutRef.current = null; } - }; + }, []); - const handleIframeLoad = useCallback(() => { + const handleIframeLoad = useCallback(() => { clearLoadingTimeout(); dispatch({ type: "SET_LOAD_SUCCESS" }); - }, []); + }, [clearLoadingTimeout]); - const handleIframeError = useCallback(() => { + const handleIframeError = useCallback(() => { clearLoadingTimeout(); dispatch({ type: "SET_LOAD_ERROR", payload: "Failed to load website. This could be due to CORS restrictions or the site blocking iframes.", }); - }, []); + }, [clearLoadingTimeout]); useEffect(() => { if (state.isLoading && currentUrl) { clearLoadingTimeout(); loadingTimeoutRef.current = setTimeout(() => { dispatch({ type: "SET_LOAD_ERROR", payload: "Loading timeout - the website may be taking too long to respond", }); loadingTimeoutRef.current = null; }, LOADING_TIMEOUT_MS); return clearLoadingTimeout; } - }, [state.isLoading, currentUrl]); + }, [state.isLoading, currentUrl, clearLoadingTimeout]);public/live-preview.js (2)
157-198: Lock replies to the validated parent originWe’re still replying with
postMessage(..., "*"), so any ancestor can intercept. Capture the allowed origin once it passes the whitelist and reuse it for every response (including the ready ping).-// ----- MESSAGE SENDING ----- -function sendMessageToParent(message) { +let PARENT_ORIGIN = "*"; + +// ----- MESSAGE SENDING ----- +function sendMessageToParent(message) { if (window.parent && window.parent !== window) { try { - window.parent.postMessage(message, "*"); + window.parent.postMessage(message, PARENT_ORIGIN); @@ - if (!ALLOWED_ORIGINS.includes(event.origin)){ - sendMessageToParent({ type: TWEAKCN_MESSAGE.EMBED_ERROR, payload: { error: "Origin not allowed. Preview failed to establish the connection with tweakcn." } }); + if (!ALLOWED_ORIGINS.includes(event.origin)){ + sendMessageToParent({ + type: TWEAKCN_MESSAGE.EMBED_ERROR, + payload: { error: "Origin not allowed. Preview failed to establish the connection with tweakcn." } + }); return; - } ; + } ; + PARENT_ORIGIN = event.origin;
205-211: Scope the switch-case declaration
const supportInfois still declared directly under thecase, trippingnoSwitchDeclarations. Wrap the case body in braces to keep the binding local.- case TWEAKCN_MESSAGE.CHECK_SHADCN: - const supportInfo = checkShadcnSupport(); - sendMessageToParent({ - type: TWEAKCN_MESSAGE.SHADCN_STATUS, - payload: supportInfo, - }); - break; + case TWEAKCN_MESSAGE.CHECK_SHADCN: { + const supportInfo = checkShadcnSupport(); + sendMessageToParent({ + type: TWEAKCN_MESSAGE.SHADCN_STATUS, + payload: supportInfo, + }); + break; + }components/dynamic-website-preview.tsx (1)
51-53: Close the embed<script>tag properlyThe embed snippet still uses
<script .../>, which is invalid HTML when copied verbatim. Give consumers a proper closing tag so browsers load the script instead of treating the rest of the page as inline JS.-const TWEAKCN_EMBED_SCRIPT_TAG = `<script src="${SCRIPT_URL}"/>`; +const TWEAKCN_EMBED_SCRIPT_TAG = `<script src="${SCRIPT_URL}"></script>`;
🧹 Nitpick comments (1)
components/editor/theme-preview-panel.tsx (1)
55-57: Consider removing the handleTabChange wrapper.The
handleTabChangefunction is a simple pass-through tosetActiveTabwith no additional logic. SinceonValueChangeat line 69 already usessetActiveTabdirectly, the wrapper is redundant.Apply this diff to simplify the code:
- const handleTabChange = (value: string) => { - setActiveTab(value); - }; -And update line 93 to use
setActiveTabdirectly:- <DropdownMenuItem onClick={() => handleTabChange("typography")}> + <DropdownMenuItem onClick={() => setActiveTab("typography")}>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
components/ai-elements/code-block.tsx(2 hunks)components/dynamic-website-preview.tsx(1 hunks)components/editor/theme-preview-panel.tsx(5 hunks)hooks/use-website-preview.ts(1 hunks)public/live-preview.js(1 hunks)store/website-preview-store.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
public/live-preview.js (3)
utils/fonts/index.ts (1)
extractFontFamily(188-200)utils/fonts/google-fonts.ts (2)
buildFontCssUrl(34-38)loadGoogleFont(42-54)utils/theme-fonts.ts (1)
fonts(37-68)
hooks/use-website-preview.ts (1)
store/website-preview-store.ts (1)
useWebsitePreviewStore(12-25)
components/editor/theme-preview-panel.tsx (2)
components/examples/custom/index.tsx (1)
CustomDemo(3-5)types/theme.ts (1)
ThemeEditorPreviewProps(89-92)
components/dynamic-website-preview.tsx (7)
components/block-viewer.tsx (3)
BlockViewerProvider(28-43)BlockViewerToolbar(69-123)BlockViewerDisplay(125-176)lib/utils.ts (1)
cn(6-8)hooks/use-website-preview.ts (1)
useWebsitePreview(44-162)hooks/use-iframe-theme-injector.ts (1)
useIframeThemeInjector(18-175)components/ai-elements/code-block.tsx (2)
CodeBlock(27-76)CodeBlockCopyButton(84-124)components/editor/ai/loading-logo.tsx (1)
LoadingLogo(4-20)types/live-preview-embed.ts (1)
IframeStatus(35-42)
components/ai-elements/code-block.tsx (2)
components/theme-provider.tsx (1)
useTheme(84-92)lib/utils.ts (1)
cn(6-8)
🪛 Biome (2.1.2)
public/live-preview.js
[error] 206-206: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🔇 Additional comments (5)
components/editor/theme-preview-panel.tsx (5)
4-6: LGTM! Import path refactor improves consistency.The change from relative to absolute imports using the
@/alias enhances maintainability and consistency across the codebase.
34-34: LGTM! Lazy loading CustomDemo optimizes bundle size.The lazy import ensures the CustomDemo component is only loaded when needed, improving initial page load performance.
126-126: LGTM! asChild prop enables proper component composition.Adding the
asChildprop prevents DOM nesting issues and follows the same pattern used elsewhere in the component (lines 86, 109).
159-163: LGTM! Custom tab content structure aligns with existing patterns.The Custom tab implementation follows the same structure as other preview tabs, properly wrapping the CustomDemo in an ExamplesPreviewContainer. The
@containerclass enables responsive container queries.
27-27: NuqsAdapter wrapper is properly configured. Verified import and usage in app/layout.tsx; no further action needed.
| const { theme } = useTheme(); | ||
| return ( | ||
| <CodeBlockContext.Provider value={{ code }}> | ||
| <div | ||
| className={cn( | ||
| "bg-background text-foreground relative w-full overflow-hidden rounded-md border", | ||
| className | ||
| )} | ||
| {...props} | ||
| > | ||
| <div className="relative"> | ||
| <SyntaxHighlighter | ||
| className="overflow-hidden" | ||
| codeTagProps={{ | ||
| className: "font-mono text-sm", | ||
| }} | ||
| customStyle={{ | ||
| margin: 0, | ||
| padding: "1rem", | ||
| fontSize: "0.875rem", | ||
| background: "hsl(var(--background))", | ||
| color: "hsl(var(--foreground))", | ||
| }} | ||
| language={language} | ||
| lineNumberStyle={{ | ||
| color: "hsl(var(--muted-foreground))", | ||
| paddingRight: "1rem", | ||
| minWidth: "2.5rem", | ||
| }} | ||
| showLineNumbers={showLineNumbers} | ||
| style={theme === "light" ? oneLight : oneDark} | ||
| > |
There was a problem hiding this comment.
Use resolved theme when selecting syntax style
When the app is set to “system”, theme stays "system" so this branch always falls back to oneDark, showing a dark code theme even for system-light users. Switch to the resolved theme so the highlight style matches the actual UI mode.
- const { theme } = useTheme();
+ const { resolvedTheme } = useTheme();
…
- style={theme === "light" ? oneLight : oneDark}
+ style={resolvedTheme === "light" ? oneLight : oneDark}📝 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 { theme } = useTheme(); | |
| return ( | |
| <CodeBlockContext.Provider value={{ code }}> | |
| <div | |
| className={cn( | |
| "bg-background text-foreground relative w-full overflow-hidden rounded-md border", | |
| className | |
| )} | |
| {...props} | |
| > | |
| <div className="relative"> | |
| <SyntaxHighlighter | |
| className="overflow-hidden" | |
| codeTagProps={{ | |
| className: "font-mono text-sm", | |
| }} | |
| customStyle={{ | |
| margin: 0, | |
| padding: "1rem", | |
| fontSize: "0.875rem", | |
| background: "hsl(var(--background))", | |
| color: "hsl(var(--foreground))", | |
| }} | |
| language={language} | |
| lineNumberStyle={{ | |
| color: "hsl(var(--muted-foreground))", | |
| paddingRight: "1rem", | |
| minWidth: "2.5rem", | |
| }} | |
| showLineNumbers={showLineNumbers} | |
| style={theme === "light" ? oneLight : oneDark} | |
| > | |
| const { resolvedTheme } = useTheme(); | |
| return ( | |
| <CodeBlockContext.Provider value={{ code }}> | |
| <div | |
| className={cn( | |
| "bg-background text-foreground relative w-full overflow-hidden rounded-md border", | |
| className | |
| )} | |
| {...props} | |
| > | |
| <div className="relative"> | |
| <SyntaxHighlighter | |
| className="overflow-hidden" | |
| codeTagProps={{ | |
| className: "font-mono text-sm", | |
| }} | |
| customStyle={{ | |
| margin: 0, | |
| padding: "1rem", | |
| fontSize: "0.875rem", | |
| background: "hsl(var(--background))", | |
| color: "hsl(var(--foreground))", | |
| }} | |
| language={language} | |
| lineNumberStyle={{ | |
| color: "hsl(var(--muted-foreground))", | |
| paddingRight: "1rem", | |
| minWidth: "2.5rem", | |
| }} | |
| showLineNumbers={showLineNumbers} | |
| style={resolvedTheme === "light" ? oneLight : oneDark} | |
| > |
🤖 Prompt for AI Agents
In components/ai-elements/code-block.tsx around lines 35–66, the component
currently uses theme (which can be "system") to pick the SyntaxHighlighter style
causing a dark theme for system-light users; update the hook call to get the
resolved theme (e.g., const { theme, resolvedTheme } = useTheme()) and use
resolvedTheme when selecting the style (use oneLight when resolvedTheme ===
"light", otherwise oneDark), then remove reliance on raw "theme" for this
decision so the syntax highlighting matches the actual UI mode.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
public/live-preview.js (2)
237-246: Restrict postMessage targetOrigin to validated parent origin.Using "*" as targetOrigin is a security risk. The validated parent origin should be captured from incoming messages and used for all replies.
282-293: Wrap this switch case body in a block.Declaring
const supportInfodirectly undercase CHECK_SHADCNviolates thenoSwitchDeclarationsrule. The declaration must be wrapped in braces to restrict its scope to this case.
🧹 Nitpick comments (1)
public/live-preview.js (1)
266-277: Address the TODO and consider capturing validated origin.The origin validation logic is correct. However:
- The TODO comment indicates localhost should be removed before production deployment.
- After validating the origin, consider capturing it to use in
sendMessageToParentinstead of "*" (see related comment on lines 237-246).Do you want me to open a new issue to track removing localhost before production deployment?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
public/live-preview.js(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
public/live-preview.js (3)
utils/fonts/index.ts (1)
extractFontFamily(188-200)utils/fonts/google-fonts.ts (2)
buildFontCssUrl(34-38)loadGoogleFont(42-54)utils/theme-fonts.ts (1)
fonts(37-68)
🪛 Biome (2.1.2)
public/live-preview.js
[error] 287-287: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Vercel Agent Review
🔇 Additional comments (12)
public/live-preview.js (12)
1-30: LGTM!The SHADCN support detection correctly checks for all essential CSS variables on the document root and returns a status object.
32-64: LGTM!The font loading utilities are self-contained duplicates of the codebase's font utilities, which is appropriate for a standalone embed script that must run independently on external sites.
66-91: LGTM!The font class override function correctly creates/updates a dynamic style element to apply theme fonts with appropriate specificity.
93-171: LGTM!The shadow override implementation correctly generates multi-layer shadow CSS using color-mix() and handles all shadow size variations. The complexity is justified for the feature requirements.
173-193: LGTM!The theme font loading correctly extracts font families, loads them via Google Fonts, and applies class overrides with proper error handling.
195-235: LGTM!The theme application flow correctly applies light mode styles first, then dark mode overrides, with proper validation and error handling.
248-257: LGTM!The message type constants are well-defined with appropriate namespacing.
260-264: LGTM!The initialization guard correctly prevents multiple script executions.
294-304: LGTM!The THEME_UPDATE case properly validates the payload before applying the theme, and the default case appropriately handles unknown message types.
307-307: LGTM!The message event listener is correctly attached to the window.
309-316: LGTM!The public API correctly exposes initialization status, version information, and a cleanup method that properly removes the event listener.
318-320: LGTM!The EMBED_LOADED message correctly signals to the parent that the embed script is initialized and ready to receive messages.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
components/editor/theme-preview-panel.tsx (1)
55-57: Consider removing redundant wrapper function.The
handleTabChangefunction is a thin wrapper aroundsetActiveTaband is only used in one place (line 93). Consider callingsetActiveTabdirectly in the dropdown menu click handler for simplicity.Apply this diff to simplify:
- const handleTabChange = (value: string) => { - setActiveTab(value); - }; -And update line 93:
- <DropdownMenuItem onClick={() => handleTabChange("typography")}> + <DropdownMenuItem onClick={() => setActiveTab("typography")}>Alternatively, keep the function if you plan to add additional logic (e.g., analytics tracking) when tabs change.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/editor/theme-preview-panel.tsx(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
components/editor/theme-preview-panel.tsx (1)
components/examples/custom/index.tsx (1)
CustomDemo(3-5)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Vercel Agent Review
🔇 Additional comments (6)
components/editor/theme-preview-panel.tsx (6)
4-6: LGTM: Import consolidation improves maintainability.The switch from relative to absolute imports using the
@/alias is a good practice for consistency and makes refactoring easier.
22-22: LGTM: Appropriate use of lazy loading.Adding the
lazyimport for code-splitting theCustomDemocomponent is consistent with the existing pattern for other demo components.
34-34: LGTM: Lazy loading CustomDemo is appropriate.The lazy import is consistent with other demo components and helps with code splitting.
74-74: Verify CustomDemo handles its own scrolling correctly.The Custom tab (lines 159-163) differs from other tabs by not wrapping the content in a
ScrollArea. This is likely intentional sinceCustomDemorendersDynamicWebsitePreview(likely an iframe) that may manage its own scrolling.However, verify that:
- The iframe content scrolls properly without the
ScrollAreawrapper- The layout doesn't break when the iframe content exceeds the viewport
If scrolling issues arise, consider whether
CustomDemoshould handle scrolling internally or if it needs theScrollAreawrapper like other tabs.Note on tab ordering: Placing the Custom tab first (line 74) while keeping
defaultValue: "cards"(line 39) means users without a query parameter will still see the Cards tab by default. This maintains backward compatibility while allowing direct linking to the Custom tab via?p=custom.Also applies to: 159-163
126-126: LGTM: asChild enables proper composition.Adding
asChildto theTooltipWrapperprevents unnecessary DOM nesting and follows the shadcn/ui composition pattern.
27-27: NuqsAdapter setup verified; confirm URL-backed tab state is intentionalNuqsAdapter is imported and used in app/layout.tsx (lines 12, 82–95). Using useQueryState exposes the “p” param in the URL and ties tab selection to browser back/forward. If tabs should remain local-only, revert to useState.
jnsahaj
force-pushed
the
feature/preview-external
branch
from
fbfcc5e to
3553a2d
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/dynamic-website-preview.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
components/dynamic-website-preview.tsx (6)
components/block-viewer.tsx (3)
BlockViewerProvider(28-43)BlockViewerToolbar(69-123)BlockViewerDisplay(125-176)hooks/use-website-preview.ts (1)
useWebsitePreview(44-162)hooks/use-iframe-theme-injector.ts (1)
useIframeThemeInjector(18-175)types/live-preview-embed.ts (1)
IframeStatus(35-42)components/ai-elements/code-block.tsx (2)
CodeBlock(27-76)CodeBlockCopyButton(84-124)components/editor/ai/loading-logo.tsx (1)
LoadingLogo(4-20)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Vercel Agent Review
| const VITE_SNIPPET = `<!-- index.html -->\n<!doctype html> | ||
| <html lang="en"> | ||
| <head> | ||
| <script | ||
| crossOrigin="anonymous" | ||
| src="${SCRIPT_URL}" | ||
| /> | ||
| </head> | ||
| <body> | ||
| <!-- ... --> | ||
| </body> | ||
| </html>`; |
There was a problem hiding this comment.
Fix the Vite embed snippet’s <script> tag.
The Vite snippet still uses a self-closing <script />, which is invalid HTML. When someone pastes this into their index.html, the browser treats everything after it as script text and the embed never loads. Please swap it for a normal opening tag with a matching </script> closing tag.
- <script
- crossOrigin="anonymous"
- src="${SCRIPT_URL}"
- />
+ <script
+ crossOrigin="anonymous"
+ src="${SCRIPT_URL}"
+ ></script>📝 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 VITE_SNIPPET = `<!-- index.html -->\n<!doctype html> | |
| <html lang="en"> | |
| <head> | |
| <script | |
| crossOrigin="anonymous" | |
| src="${SCRIPT_URL}" | |
| /> | |
| </head> | |
| <body> | |
| <!-- ... --> | |
| </body> | |
| </html>`; | |
| const VITE_SNIPPET = `<!-- index.html -->\n<!doctype html> | |
| <html lang="en"> | |
| <head> | |
| <script | |
| crossOrigin="anonymous" | |
| src="${SCRIPT_URL}" | |
| ></script> | |
| </head> | |
| <body> | |
| <!-- ... --> | |
| </body> | |
| </html>`; |
🤖 Prompt for AI Agents
In components/dynamic-website-preview.tsx around lines 92 to 103, the Vite embed
snippet uses a self-closing <script /> tag which is invalid HTML; replace it
with a normal opening <script ...> tag and a matching </script> closing tag
(preserving the existing attributes like crossOrigin="anonymous" and
src="${SCRIPT_URL}") so that browsers correctly parse the page and the embed
script loads.
| useEffect(() => { | ||
| if (websitePreviewState.currentUrl) { | ||
| setTimeout(() => { | ||
| // capturing after 1s delay so status is finalized | ||
| posthog.capture("DYNAMIC_PREVIEW_LOADED", { | ||
| url: websitePreviewState.currentUrl, | ||
| status: statusRef.current, | ||
| }); | ||
| }, 1000); | ||
| } | ||
| }, [websitePreviewState.currentUrl, posthog]); |
There was a problem hiding this comment.
Guard PostHog before capturing analytics.
usePostHog() can return null/undefined until the client is ready (or when the provider isn’t mounted). Calling posthog.capture(...) unconditionally will throw in those cases. Please bail out when PostHog isn’t available.
- if (websitePreviewState.currentUrl) {
+ if (websitePreviewState.currentUrl && posthog) {
setTimeout(() => {
// capturing after 1s delay so status is finalized
posthog.capture("DYNAMIC_PREVIEW_LOADED", {
url: websitePreviewState.currentUrl,
status: statusRef.current,
});
}, 1000);
}📝 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.
| useEffect(() => { | |
| if (websitePreviewState.currentUrl) { | |
| setTimeout(() => { | |
| // capturing after 1s delay so status is finalized | |
| posthog.capture("DYNAMIC_PREVIEW_LOADED", { | |
| url: websitePreviewState.currentUrl, | |
| status: statusRef.current, | |
| }); | |
| }, 1000); | |
| } | |
| }, [websitePreviewState.currentUrl, posthog]); | |
| useEffect(() => { | |
| if (websitePreviewState.currentUrl && posthog) { | |
| setTimeout(() => { | |
| // capturing after 1s delay so status is finalized | |
| posthog.capture("DYNAMIC_PREVIEW_LOADED", { | |
| url: websitePreviewState.currentUrl, | |
| status: statusRef.current, | |
| }); | |
| }, 1000); | |
| } | |
| }, [websitePreviewState.currentUrl, posthog]); |
🤖 Prompt for AI Agents
In components/dynamic-website-preview.tsx around lines 195 to 205, the effect
unconditionally calls posthog.capture which can throw if usePostHog() returns
null/undefined; guard against a missing PostHog client by checking that posthog
is truthy before scheduling the capture (or return early from the effect), and
ensure the timeout is cleared on cleanup to avoid calling capture after unmount
if posthog becomes available later.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
components/dynamic-website-preview.tsx (2)
186-189: Avoid '*' targetOrigin for postMessage by deriving the iframe originFor cross-origin messaging, prefer a specific targetOrigin to tighten security. Consider extending useIframeThemeInjector to accept targetOrigin derived from currentUrl, and pass it here.
Example:
// Derive once in provider: const targetOrigin = websitePreviewState.currentUrl ? new URL(websitePreviewState.currentUrl).origin : "*"; // Then pass into the hook (after adding a new prop to its signature): useIframeThemeInjector({ allowCrossOrigin: allowCrossOrigin && !!websitePreviewState.currentUrl, iframeRef: websitePreviewState.iframeRef, targetOrigin, // new });In the hook, replace postMessage(msg, "*") with postMessage(msg, targetOrigin).
461-471: Remove redundant status truthiness checkstatus is always a non-empty string from the hook; !!status is redundant.
- {!previewIsLoading && !!status && allowCrossOrigin && ( + {!previewIsLoading && allowCrossOrigin && (
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/dynamic-website-preview.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
components/dynamic-website-preview.tsx (7)
components/block-viewer.tsx (3)
BlockViewerProvider(28-43)BlockViewerToolbar(69-123)BlockViewerDisplay(125-176)lib/utils.ts (1)
cn(6-8)hooks/use-website-preview.ts (1)
useWebsitePreview(44-162)hooks/use-iframe-theme-injector.ts (1)
useIframeThemeInjector(18-175)types/live-preview-embed.ts (1)
IframeStatus(35-42)components/ai-elements/code-block.tsx (2)
CodeBlock(27-76)CodeBlockCopyButton(84-124)components/editor/ai/loading-logo.tsx (1)
LoadingLogo(4-20)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Vercel Agent Review
| <script | ||
| crossOrigin="anonymous" | ||
| src="${SCRIPT_URL}" | ||
| /> | ||
| </head> |
There was a problem hiding this comment.
Fix Vite snippet: script is not self-closing in HTML; use closing tag and lowercase crossorigin
Self-closing <script /> breaks HTML parsing. Update to a proper closing tag and use crossorigin (lowercase) for raw HTML.
- <script
- crossOrigin="anonymous"
- src="${SCRIPT_URL}"
- />
+ <script
+ crossorigin="anonymous"
+ src="${SCRIPT_URL}"
+ ></script>📝 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.
| <script | |
| crossOrigin="anonymous" | |
| src="${SCRIPT_URL}" | |
| /> | |
| </head> | |
| <script | |
| crossorigin="anonymous" | |
| src="${SCRIPT_URL}" | |
| ></script> | |
| </head> |
🤖 Prompt for AI Agents
In components/dynamic-website-preview.tsx around lines 95 to 99, the script
element is written as a self-closing tag and uses CrossOrigin camelCase; change
it to a normal <script> element with an explicit closing tag and use the
lowercase crossorigin attribute (e.g., crossorigin="anonymous") so the injected
HTML is valid and parses correctly in browsers.
| useEffect(() => { | ||
| if (websitePreviewState.currentUrl) { | ||
| setTimeout(() => { | ||
| // capturing after 1s delay so status is finalized | ||
| posthog.capture("DYNAMIC_PREVIEW_LOADED", { | ||
| url: websitePreviewState.currentUrl, | ||
| status: statusRef.current, | ||
| }); | ||
| }, 1000); | ||
| } | ||
| }, [websitePreviewState.currentUrl, posthog]); |
There was a problem hiding this comment.
Guard PostHog usage and clear timeout to avoid crash/leak
usePostHog can be null; calling capture unguarded can throw. Also clear the timeout on unmount.
- useEffect(() => {
- if (websitePreviewState.currentUrl) {
- setTimeout(() => {
- // capturing after 1s delay so status is finalized
- posthog.capture("DYNAMIC_PREVIEW_LOADED", {
- url: websitePreviewState.currentUrl,
- status: statusRef.current,
- });
- }, 1000);
- }
- }, [websitePreviewState.currentUrl, posthog]);
+ useEffect(() => {
+ if (!websitePreviewState.currentUrl || !posthog) return;
+ const timeout = setTimeout(() => {
+ // capture after 1s so status is finalized
+ posthog.capture("DYNAMIC_PREVIEW_LOADED", {
+ url: websitePreviewState.currentUrl,
+ status: statusRef.current,
+ });
+ }, 1000);
+ return () => clearTimeout(timeout);
+ }, [websitePreviewState.currentUrl, posthog]);Based on learnings
📝 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.
| useEffect(() => { | |
| if (websitePreviewState.currentUrl) { | |
| setTimeout(() => { | |
| // capturing after 1s delay so status is finalized | |
| posthog.capture("DYNAMIC_PREVIEW_LOADED", { | |
| url: websitePreviewState.currentUrl, | |
| status: statusRef.current, | |
| }); | |
| }, 1000); | |
| } | |
| }, [websitePreviewState.currentUrl, posthog]); | |
| useEffect(() => { | |
| if (!websitePreviewState.currentUrl || !posthog) return; | |
| const timeout = setTimeout(() => { | |
| // capture after 1s so status is finalized | |
| posthog.capture("DYNAMIC_PREVIEW_LOADED", { | |
| url: websitePreviewState.currentUrl, | |
| status: statusRef.current, | |
| }); | |
| }, 1000); | |
| return () => clearTimeout(timeout); | |
| }, [websitePreviewState.currentUrl, posthog]); |
🤖 Prompt for AI Agents
In components/dynamic-website-preview.tsx around lines 195 to 205, the useEffect
schedules a timeout that calls posthog.capture without checking that posthog is
non-null and does not clear the timeout on unmount, which can throw or leak;
update the effect to (1) store the timeout id in a variable, (2) guard the
capture call with if (posthog) (or return early if posthog is falsy), and (3)
return a cleanup function that clears the timeout via clearTimeout(timeoutId) to
prevent leaks/crashes on unmount or when posthog is null.
| const showTimeoutRef = React.useRef<NodeJS.Timeout | null>(null); | ||
| const hideTimeoutRef = React.useRef<NodeJS.Timeout | null>(null); |
There was a problem hiding this comment.
Fix timeout ref types for browser builds
NodeJS.Timeout causes TS issues in DOM/browser projects. Use ReturnType.
- const showTimeoutRef = React.useRef<NodeJS.Timeout | null>(null);
- const hideTimeoutRef = React.useRef<NodeJS.Timeout | null>(null);
+ const showTimeoutRef = React.useRef<ReturnType<typeof setTimeout> | null>(null);
+ const hideTimeoutRef = React.useRef<ReturnType<typeof setTimeout> | null>(null);📝 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 showTimeoutRef = React.useRef<NodeJS.Timeout | null>(null); | |
| const hideTimeoutRef = React.useRef<NodeJS.Timeout | null>(null); | |
| const showTimeoutRef = React.useRef<ReturnType<typeof setTimeout> | null>(null); | |
| const hideTimeoutRef = React.useRef<ReturnType<typeof setTimeout> | null>(null); |
🤖 Prompt for AI Agents
In components/dynamic-website-preview.tsx around lines 489 to 490, the refs are
typed as NodeJS.Timeout which breaks TypeScript in browser/DOM projects; replace
those types with ReturnType<typeof setTimeout> (e.g.
React.useRef<ReturnType<typeof setTimeout> | null>(null)) for both
showTimeoutRef and hideTimeoutRef so the code uses the correct
browser-compatible timer type.
2 participants
Summary by CodeRabbit
New Features
Improvements
Reliability