-
Notifications
You must be signed in to change notification settings - Fork 1
feature: added preview option for reviewing the component #68
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: main
Are you sure you want to change the base?
Conversation
WalkthroughIntroduces a preview system for development mode allowing mock visualization of tool outputs without executing runtime logic. Adds Preview type definitions, initialization function, mocked window.openai API, and state management, then integrates across all tool frameworks and updates examples with preview metadata. Changes
Sequence Diagram(s)sequenceDiagram
participant Tool as Tool Framework
participant Init as initializePreview
participant Merge as Config Merger
participant API as window.openai
participant State as setWidgetState
participant Event as CustomEvent
participant Sub as Subscribers
Tool->>Init: Call initializePreview(widget.preview, defaults?)
Note over Init: Dev mode check (import.meta.env.DEV)
alt In Development Mode
Init->>Merge: Merge preview config with defaults
Merge->>Init: Returns merged config
Init->>API: Assign mocked window.openai with:<br/>theme, locale, maxHeight, displayMode,<br/>toolInput, toolOutput, toolResponseMetadata
Note over API: Includes stubbed methods:<br/>callTool, sendFollowUpMessage,<br/>openExternal (throw on call)
API->>API: requestDisplayMode & setWidgetState<br/>are functional
par Optional: State Update
State->>Event: Dispatch CustomEvent<br/>(SET_GLOBALS_EVENT_TYPE)
Event->>Sub: Notify subscribers<br/>(e.g., useWidgetState hook)
end
else Not in Development Mode
Init-->>Tool: Return early (no initialization)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
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: 1
♻️ Duplicate comments (12)
packages/create-chapplin/templates/solid/src/tools/get.tsx (1)
53-55: LGTM! Consistent preview pattern.Matches the preview integration pattern used across all framework templates.
packages/create-chapplin/templates/hono/src/tools/get.tsx (1)
53-55: LGTM! Consistent preview pattern.Matches the preview integration pattern used across all framework templates.
packages/create-chapplin/templates/react/src/tools/get.tsx (1)
53-55: LGTM! Consistent preview pattern.Matches the preview integration pattern used across all framework templates.
packages/chapplin/src/tool-react.ts (3)
7-7: LGTM! Consistent preview integration.Matches the pattern used across all framework adapters.
9-12: LGTM! Type extension for preview support.Widget type correctly extended with optional preview metadata.
23-25: LGTM! Preview initialization matches adapter pattern.Initialization is properly placed and guarded.
packages/chapplin/src/tool-solid.ts (3)
5-5: LGTM! Consistent preview integration.Matches the pattern used across all framework adapters.
7-10: LGTM! Type extension for preview support.Widget type correctly extended with optional preview metadata.
19-21: LGTM! Preview initialization matches adapter pattern.Initialization is properly placed and guarded.
packages/chapplin/src/tool-preact.ts (3)
7-7: LGTM! Consistent preview integration.Matches the pattern used across all framework adapters.
9-12: LGTM! Type extension for preview support.Widget type correctly extended with optional preview metadata.
23-25: LGTM! Preview initialization matches adapter pattern.Initialization is properly placed and guarded. This completes the preview integration across all framework adapters (React, Preact, Solid, and Hono), providing a consistent developer experience.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
examples/express-react-todo/src/tools/get.tsx(1 hunks)examples/hono-todo/src/tools/get.tsx(1 hunks)packages/chapplin/src/preview.ts(1 hunks)packages/chapplin/src/tool-hono.ts(2 hunks)packages/chapplin/src/tool-preact.ts(2 hunks)packages/chapplin/src/tool-react.ts(2 hunks)packages/chapplin/src/tool-solid.ts(2 hunks)packages/chapplin/src/tool.ts(1 hunks)packages/create-chapplin/templates/hono/src/tools/get.tsx(1 hunks)packages/create-chapplin/templates/preact/src/tools/get.tsx(1 hunks)packages/create-chapplin/templates/react/src/tools/get.tsx(1 hunks)packages/create-chapplin/templates/solid/src/tools/get.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
packages/chapplin/src/tool-hono.ts (1)
packages/chapplin/src/preview.ts (2)
Preview(7-11)initializePreview(47-97)
packages/chapplin/src/tool-preact.ts (2)
packages/chapplin/src/openai.ts (1)
OpenAiGlobals(110-130)packages/chapplin/src/preview.ts (2)
Preview(7-11)initializePreview(47-97)
packages/chapplin/src/preview.ts (2)
packages/chapplin/src/openai.ts (3)
DisplayMode(140-140)OpenAiGlobals(110-130)SET_GLOBALS_EVENT_TYPE(108-108)packages/chapplin/src/tool-solid.ts (1)
widgetState(86-88)
packages/chapplin/src/tool-react.ts (2)
packages/chapplin/src/openai.ts (1)
OpenAiGlobals(110-130)packages/chapplin/src/preview.ts (2)
Preview(7-11)initializePreview(47-97)
🔇 Additional comments (13)
examples/express-react-todo/src/tools/get.tsx (1)
53-55: LGTM! Preview data correctly matches the app component's needs.The preview field properly exposes
toolOutput.todosusing the existing todos array, which aligns with what the app component renders (lines 43-50). Good reuse of the existing data constant.examples/hono-todo/src/tools/get.tsx (1)
57-60: LGTM! Preview data correctly matches the app component's requirements.The preview field properly exposes both
toolOutput.todosandtoolResponseMetadata.timestamp, aligning with what the app component renders (lines 46-53). The timestamp will be static during the dev session (evaluated once at module load), but this is acceptable for preview purposes since the runtime function (line 38) generates fresh timestamps in production.packages/create-chapplin/templates/preact/src/tools/get.tsx (1)
53-55: Clean preview integration for development.The preview configuration enables component visualization during development without executing runtime logic. While the preview data will be included in the production bundle (as noted in the PR description), this is acceptable for template/example code with minimal data. For production applications with larger preview datasets, consider build-time stripping or separate preview files.
packages/chapplin/src/tool-hono.ts (3)
5-5: LGTM! Preview system integration.Correctly imports the Preview type and initialization function from the new preview module.
7-10: LGTM! Type extension for preview support.The Widget type is correctly extended to include optional preview metadata while maintaining backward compatibility.
22-24: LGTM! Clean preview initialization.The initialization call is properly guarded by the widget existence check. Based on the preview module implementation,
initializePreviewwill only execute in development mode (isDevMode()check) and whenwindow.openaiis not already defined, ensuring zero runtime impact in production.packages/chapplin/src/preview.ts (7)
13-18: LGTM!The PreviewDefaults type appropriately defines optional configuration for the preview environment with sensible types.
20-25: LGTM!Sensible default values for development preview mode.
31-40: Safe dev-mode detection with appropriate fallback.The try-catch with a production fallback ensures that preview initialization fails safely in environments where
import.meta.envis unavailable. Theanycast is necessary for accessing Vite-specific properties and is appropriately documented.
47-52: LGTM!The guard conditions correctly prevent initialization when preview data is absent, window.openai is already set by the host, or not in development mode.
54-74: LGTM!The configuration merging and mock window.openai properties are well-structured. The widgetState getter correctly returns the mutable local variable, enabling state updates via setWidgetState.
76-84: LGTM!The stub methods correctly throw descriptive errors in preview mode, preventing accidental invocation of host-only APIs during development.
85-95: LGTM!The setWidgetState implementation correctly updates the local state and dispatches a CustomEvent with SET_GLOBALS_EVENT_TYPE, enabling reactivity for hooks like useWidgetState. The event detail structure matches the expected Partial type.
| /** | ||
| * Preview data for local development. | ||
| * Used to display mock data before MCP response is available. | ||
| */ | ||
| preview?: { | ||
| toolInput?: Shape<InputArgs>; | ||
| toolOutput?: Shape<OutputArgs>; | ||
| toolResponseMetadata?: OutputMeta extends undefined | ||
| ? UnknownObject | ||
| : OutputMeta; | ||
| }; |
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.
Preview data will be included in production bundles.
The preview field is a welcome addition for development, but since it's part of the widget configuration type without build-time stripping, all preview data (toolInput, toolOutput, toolResponseMetadata) will be bundled into production builds even though they're only used when import.meta.env.DEV is true.
Consider these approaches to prevent preview data from reaching production:
- Use a separate file suffix (e.g.,
*.preview.ts) that can be excluded by bundler configuration - Wrap preview data in
if (import.meta.env.DEV)blocks so dead-code elimination can remove it - Document that developers should keep preview data minimal
Additionally, the preview types use Shape<T> which is correct, but the actual Preview type in preview.ts uses Record<string, unknown>, losing the type safety provided by the schemas. Consider tightening the Preview type to preserve schema types if feasible.
Would you like me to generate examples showing how to structure preview data for better tree-shaking, or help document best practices for keeping preview data minimal?
|
@yugo-ibuki Thank you for the PR! And sorry for the late review. If we include preview information in the tool definition, it will also be included in the final bundle, so we want to move the data to a separate file. I'm imagining something like |
|
@ssssota
They seem good. It might be a bit more workload, but the code could be cleaner. |
|
I plan to offer a development mode similar to the actual Apps in ChatGPT, accessible via iframe. The idea is to enable passing preview data to the iframe from there. |
Hi, I thought it would be nice to preview components during development with stub data.
I added a
previewoption todefineToolto check how the component looks in the browser.Here's a comparison with the Hono example:
What do you think?
Note: The preview is only initialized when
import.meta.env.DEVis true, so it won't affect production at runtime. However, the preview data is still included in the production bundle. Separating the preview system (e.g., via a separate file or build-time stripping) would be a better approach, but I couldn't figure out how to implement it cleanly.Anyway, thank you for creating this great framework!
It really helps me work with the complex, newly-released SDK :)
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.