Adding support for object prop types and improving string inputs that have no options #16974
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
|
Warning Rate limit exceeded@dannyroosevelt has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 13 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
WalkthroughThis update introduces support for "object" prop types and array inputs in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Control
participant ControlArray
participant ControlObject
participant Form
User->>Control: Interacts with control (array/object/string)
alt Array type without options
Control->>ControlArray: Render array input controls
ControlArray->>Form: onChange with updated array
else Object type
Control->>ControlObject: Render object key-value controls
ControlObject->>Form: onChange with updated object
else Other types
Control->>Form: Render existing controls (select, input, etc.)
end
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
packages/connect-react/src/components/ControlArray.tsx (1)
84-105: Duplicate styling with ControlObjectThese styles are nearly identical to those in ControlObject.tsx, indicating an opportunity for code reuse.
This is the same issue identified in ControlObject.tsx - consider extracting shared styling utilities to reduce duplication.
🧹 Nitpick comments (7)
packages/connect-react/src/components/Description.tsx (1)
46-81: Optimize component performance by memoizing styles and moving component outside render.The current implementation creates a new component function on every render, which can impact performance. Consider extracting the link component and memoizing the styles.
+import { useMemo } from "react"; +const ExternalLink = ({ ...linkProps }) => { + const [isHovered, setIsHovered] = useState(false); + + const linkStyles: CSSProperties = useMemo(() => ({ + textDecoration: "underline", + textUnderlineOffset: "3px", + color: "inherit", + transition: "opacity 0.2s ease", + opacity: isHovered ? 0.7 : 1, + }), [isHovered]); + + const spanStyles: CSSProperties = useMemo(() => ({ + display: "inline-flex", + alignItems: "center", + gap: "2px", + }), []); + + const iconStyles: CSSProperties = useMemo(() => ({ + fontSize: "0.7em", + opacity: 0.7, + }), []); + + return ( + <span style={spanStyles}> + <a + {...linkProps} + target="_blank" + rel="noopener noreferrer" + style={linkStyles} + onMouseEnter={() => setIsHovered(true)} + onMouseLeave={() => setIsHovered(false)} + /> + <span style={iconStyles}>↗</span> + </span> + ); +}; return <div className={getClassNames("description", props)} style={getStyles("description", baseStyles, props)}> <Markdown components={{ - a: ({ ...linkProps }) => { - const [ - isHovered, - setIsHovered, - ] = useState(false); - - const linkStyles: CSSProperties = { - textDecoration: "underline", - textUnderlineOffset: "3px", - color: "inherit", - transition: "opacity 0.2s ease", - opacity: isHovered - ? 0.7 - : 1, - }; - - return ( - <span style={{ - display: "inline-flex", - alignItems: "center", - gap: "2px", - }}> - <a - {...linkProps} - target="_blank" - rel="noopener noreferrer" - style={linkStyles} - onMouseEnter={() => setIsHovered(true)} - onMouseLeave={() => setIsHovered(false)} - /> - <span style={{ - fontSize: "0.7em", - opacity: 0.7, - }}>↗</span> - </span> - ); - }, + a: ExternalLink,packages/connect-react/src/components/ControlObject.tsx (4)
78-82: Add validation for JSON parsing securityThe JSON.parse operation could be a security risk with untrusted input. Consider validating or sanitizing the parsed values.
try { - obj[pair.key] = JSON.parse(pair.value); + const parsed = JSON.parse(pair.value); + // Validate that parsed value is a safe type + if (typeof parsed === 'object' && parsed !== null && parsed.constructor !== Object && !Array.isArray(parsed)) { + obj[pair.key] = pair.value; // Keep as string for complex objects + } else { + obj[pair.key] = parsed; + } } catch { obj[pair.key] = pair.value; }
89-98: Optimize performance by debouncing onChange callsCurrently
updateObjectis called on every keystroke, which could cause performance issues with large objects or slow onChange handlers.Consider debouncing the
updateObjectcalls or only calling it onBlur for better performance:const handlePairChange = (index: number, field: "key" | "value", newValue: string) => { const newPairs = [...pairs]; newPairs[index] = { ...newPairs[index], [field]: newValue }; setPairs(newPairs); // Only update object on blur or after a delay }; const handleBlur = () => { updateObject(pairs); };
125-169: Extract shared styling to reduce code duplicationThe styling objects are very similar to those in ControlArray.tsx. Consider extracting shared styles to a common utility or theme.
Create a shared styling utility:
// In a shared utils file export const createControlStyles = (theme: Theme) => ({ container: { gridArea: "control", display: "flex", flexDirection: "column", gap: "0.5rem", } as CSSProperties, input: { color: theme.colors.neutral60, border: "1px solid", borderColor: theme.colors.neutral20, padding: 6, borderRadius: theme.borderRadius, boxShadow: theme.boxShadow.input, flex: 1, } as CSSProperties, // ... other shared styles });
190-199: Improve accessibility of remove buttonsThe remove buttons have basic aria-label but could benefit from better keyboard navigation and focus management.
<button type="button" onClick={() => removePair(index)} style={removeButtonStyles} - aria-label="Remove pair" + aria-label={`Remove key-value pair ${index + 1}`} + tabIndex={0} + onKeyDown={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + removePair(index); + } + }} > × </button>packages/connect-react/src/components/ControlArray.tsx (2)
43-47: Optimize performance with debounced updatesLike ControlObject, this calls
updateArrayon every keystroke which could impact performance.Consider the same debouncing approach as suggested for ControlObject, or only update on blur events for better performance.
115-115: Add meaningful placeholder textThe empty placeholder provides no guidance to users about what to enter.
-placeholder="" +placeholder="Enter value"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
packages/connect-react/CHANGELOG.md(1 hunks)packages/connect-react/package.json(1 hunks)packages/connect-react/src/components/Control.tsx(3 hunks)packages/connect-react/src/components/ControlArray.tsx(1 hunks)packages/connect-react/src/components/ControlObject.tsx(1 hunks)packages/connect-react/src/components/Description.tsx(2 hunks)packages/connect-react/src/hooks/customization-context.tsx(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/connect-react/src/components/Control.tsx (2)
packages/connect-react/src/components/ControlArray.tsx (1)
ControlArray(5-147)packages/connect-react/src/components/ControlObject.tsx (1)
ControlObject(12-216)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Lint Code Base
🔇 Additional comments (9)
packages/connect-react/src/components/Description.tsx (1)
68-75: Excellent UX improvement for external links.The addition of
target="_blank"andrel="noopener noreferrer"attributes along with the visual hover effects creates a clear user experience for external links. The security attributes prevent potential security vulnerabilities.packages/connect-react/package.json (1)
3-3: Version bump follows semantic versioning correctly.The minor version increment from 1.0.2 to 1.1.0 is appropriate for the addition of new features like object prop type support.
packages/connect-react/CHANGELOG.md (1)
5-10: Changelog properly documents the new features.The changelog entry clearly describes the three main improvements. However, verify that the date "2025-06-04" is correct, as this appears to be a future date which might be inconsistent with the current development timeline.
packages/connect-react/src/hooks/customization-context.tsx (2)
72-72: CustomizableProps type extensions are well-structured.The addition of
controlArrayandcontrolObjectto theCustomizablePropstype follows the established pattern and properly combines component props withFormFieldContext<ConfigurableProp>.Also applies to: 75-75
27-27:✅ Verification successful
New control imports follow established patterns.
The imports for
ControlArrayandControlObjectare properly added and will enable customization of these new components.Verify that the imported components exist:
Also applies to: 30-30
🏁 Script executed:
#!/bin/bash # Description: Verify that the new control components exist in the codebase # Expected: Both ControlArray.tsx and ControlObject.tsx files should be found echo "Searching for ControlArray component..." fd "ControlArray.tsx" packages/connect-react/src/components/ echo "Searching for ControlObject component..." fd "ControlObject.tsx" packages/connect-react/src/components/ echo "Verifying exports from these files..." if [ -f "packages/connect-react/src/components/ControlArray.tsx" ]; then echo "ControlArray.tsx exports:" rg "export.*ControlArray" packages/connect-react/src/components/ControlArray.tsx fi if [ -f "packages/connect-react/src/components/ControlObject.tsx" ]; then echo "ControlObject.tsx exports:" rg "export.*ControlObject" packages/connect-react/src/components/ControlObject.tsx fiLength of output: 994
Approve ControlArray and ControlObject imports
Verified that
packages/connect-react/src/components/ControlArray.tsxexportsControlArraypackages/connect-react/src/components/ControlObject.tsxexportsControlObjectThese follow the established pattern and integrate correctly into the customization context.
packages/connect-react/src/components/Control.tsx (3)
9-9: LGTM: Clean import additionsThe new component imports are properly structured and follow the existing import pattern.
Also applies to: 12-12
83-84: LGTM: Object type support added correctlyThe object case follows the established pattern and integrates cleanly with the existing switch statement.
59-62:❓ Verification inconclusive
Verify the array type handling logic
The logic correctly prioritizes predefined options over the new ControlArray component. However, ensure that this doesn't conflict with the existing options handling above (lines 39-50).
🏁 Script executed:
#!/bin/bash # Description: Verify that array types with options are handled consistently # Search for other array type handling patterns in the codebase rg -A 3 -B 3 "endsWith.*\[\]" --type tsx --type ts # Check for any existing array option handling rg -A 5 -B 5 "options.*\[\]|array.*options" --type tsx --type tsLength of output: 175
Verify array type handling in Control component
Ensure the fallback to
<ControlArray />(lines 59–62) aligns with—and doesn’t override—the existing options-driven rendering block (lines 39–50).• packages/connect-react/src/components/Control.tsx
– Lines 39–50: current handling forprop.options
– Lines 59–62: fallback to<ControlArray />packages/connect-react/src/components/ControlArray.tsx (1)
15-22: LGTM: Robust array initializationThe initialization properly handles non-array values and converts non-string elements to JSON strings, with a sensible fallback to a single empty string.
…edreamHQ/pipedream into danny/connect-react/object-prop-type
Summary by CodeRabbit