Treat empty string prop value as a valid input #15325
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
WalkthroughThis pull request involves updates to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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: 0
🧹 Nitpick comments (1)
packages/connect-react/src/utils/component.ts (1)
100-104: Consider adding test coverage for empty string scenarios.Since this change represents a behavioral modification in string prop validation, it would be beneficial to add test cases that verify:
- Empty strings are now accepted as valid
- Undefined/null values are still properly validated when no default is provided
- Props with default values handle all cases correctly
Would you like me to help generate the test cases?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/connect-react/examples/nextjs/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
packages/connect-react/package.json(1 hunks)packages/connect-react/src/utils/component.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/connect-react/package.json
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
🔇 Additional comments (1)
packages/connect-react/src/utils/component.ts (1)
100-104: LGTM! The changes align with the PR objective.The modification correctly implements the goal of treating empty strings as valid inputs while maintaining validation for undefined/null values when no default is provided. This change makes string validation more consistent with how other prop types are handled in the codebase.
However, let's verify that this change doesn't break any existing components that might rely on empty string validation.
✅ Verification successful
Empty string validation change is safe to proceed
The codebase analysis shows no components relying on empty strings being invalid, and the change aligns with existing validation patterns across the codebase. The modification is well-contained within the connect-react package's prop validation logic.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for components that might be affected by this change # Look for string props with potential empty string handling # Search for string prop definitions that might expect non-empty validation rg -A 5 "type: 'string'" # Search for potential validation checks against empty strings rg -A 5 '(value\s*===?\s*""|\s*==\s*""|\s*\.length\s*===?\s*0)'Length of output: 66008
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/connect-react/CHANGELOG.md (1)
6-6: Consider expanding the changelog entry to clarify the impact.While the change description accurately reflects the implementation change, it would be helpful to:
- Mention that this enables support for empty string values as valid inputs
- Add an example use case (e.g., Dropbox's "Root Folder" option)
- Note any potential breaking changes if components relied on the previous behavior
This additional context would help users understand the significance and impact of this change.
Here's a suggested expansion:
- Check if a string prop is set instead of inspecting the contents of the string + Check if a string prop is set instead of inspecting the contents of the string + - Now treats empty strings as valid input values (e.g., Dropbox's "Root Folder" option) + - Note: Components that relied on string content validation may need updates
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/connect-react/CHANGELOG.md(1 hunks)packages/connect-react/src/utils/component.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/connect-react/src/utils/component.ts
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
🔇 Additional comments (1)
packages/connect-react/CHANGELOG.md (1)
4-4: LGTM! Version entry follows the established format.The version number and date format are consistent with previous entries.
bzwrk
deleted the
biz/dj-2809-submit-button-on-dropbox-new-file-trigger-doesnt-work
branch
An empty prop value is a valid configuration. In the following example, configuring the Dropbox new file trigger returns Root Folder async option with an empty value. There might be other cases where something like this is also true but I only tested dropbox.
Request
https://api.pipedream.com/v1/connect/components/configure{ "external_user_id": "demo-98522368-0f2e-4aeb-a237-902d170fa9c7", "id": "dropbox-new-file", "prop_name": "path", "configured_props": { "dropbox": { "authProvisionId": "apn_mnhGBaE" } }, "page": 0 }Response
{ "observations": [], "context": { "cursor": null }, "options": [ { "label": "Root Folder", "value": "" } ], "errors": [], "timings": { "api_to_sidekiq": 1737064862392.3438, "sidekiq_received": 1737064862426.3557, "sidekiq_to_lambda": 1737064862432.2803, "sidekiq_done": 1737064862848.1416, "lambda_configure_prop_called": 1737064862540, "lambda_done": 1737064862841 }, "stringOptions": null }Summary by CodeRabbit
Version Update
1.0.0-preview.19to1.0.0-preview.20Bug Fixes
Documentation