-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Adding support for the sql prop type in connect-react #17011
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
WalkthroughThe changes introduce a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Control
participant ControlSql
participant FormContext
User->>Control: Render with prop.type = "sql"
Control->>ControlSql: Render ControlSql
ControlSql->>FormContext: Access form field and context
User->>ControlSql: Enter SQL query in textarea
ControlSql->>FormContext: onChange({ app, query, params })
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
✨ 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 (
|
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: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/connect-react/src/components/Control.tsx(2 hunks)packages/connect-react/src/components/ControlSql.tsx(1 hunks)packages/connect-react/src/hooks/customization-context.tsx(2 hunks)packages/connect-react/src/index.ts(1 hunks)packages/sdk/src/shared/component.ts(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/connect-react/src/components/Control.tsx (2)
packages/connect-react/src/components/ControlSql.tsx (1)
ControlSql(6-64)packages/connect-react/src/index.ts (1)
ControlSql(11-11)
packages/connect-react/src/components/ControlSql.tsx (3)
packages/connect-react/src/index.ts (1)
ControlSql(11-11)packages/connect-react/src/hooks/form-context.tsx (1)
useFormContext(55-64)packages/connect-react/src/hooks/customization-context.tsx (1)
useCustomize(207-278)
🔇 Additional comments (12)
packages/sdk/src/shared/component.ts (3)
89-91: LGTM! SQL property type follows established patterns.The
ConfigurablePropSqltype definition is consistent with other configurable property types, properly extendingBaseConfigurablePropand usingDefaultable<string>for default value support.
106-106: LGTM! Properly integrated into the union type.The addition of
ConfigurablePropSqlto theConfigurablePropunion type maintains consistency with the existing type system.
127-128: LGTM! Structured value mapping is appropriate for SQL queries.The PropValue mapping for "sql" type to
{ app: string; query: string; params: any[]; }provides a structured approach that can include metadata about the target database app and parameterized queries, which is well-suited for SQL query management.packages/connect-react/src/index.ts (1)
11-11: LGTM! Export follows established patterns.The
ControlSqlexport is properly placed alphabetically among other control exports and follows the same pattern as existing components.packages/connect-react/src/components/Control.tsx (2)
14-14: LGTM! Import follows established patterns.The import of
ControlSqlis consistent with other control component imports and maintains alphabetical ordering.
86-87: LGTM! Switch case integration is consistent.The addition of the "sql" case to the switch statement follows the same pattern as other control types, simply returning the appropriate component instance.
packages/connect-react/src/hooks/customization-context.tsx (2)
32-32: LGTM! Import maintains alphabetical ordering.The import of
ControlSqlis properly placed in alphabetical order among other control component imports.
77-77: LGTM! Customization type integration is consistent.The addition of
controlSqltoCustomizablePropsfollows the exact same pattern as other controls, properly combiningComponentProps<typeof ControlSql>withFormFieldContext<ConfigurableProp>to enable full customization support.packages/connect-react/src/components/ControlSql.tsx (4)
1-6: LGTM! Clean component structure.The imports and component definition follow React best practices. The component appropriately uses context hooks rather than props for configuration.
27-35: LGTM! Well-structured change handler.The change handler correctly transforms the query string into the expected structured SQL object format. The comment about empty params array is helpful for future maintenance.
37-51: Excellent SQL-specific styling choices.The base styles are well-thought-out for SQL input:
- Monospace font for better code readability
- Appropriate minimum height and vertical resize capability
- Good use of theme values for consistency
53-63: LGTM! Proper textarea implementation with accessibility.The textarea element includes all necessary attributes for accessibility and form integration:
idandnamefor proper form associationrequiredbased on prop optionality- Appropriate placeholder text
- Proper integration with customization system
Remove SQL prop type definitions from SDK to separate into standalone PR, keeping only connect-react component changes. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
WHY
Summary by CodeRabbit