-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Adding support for custom OAuth in connect-react #17902
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
|
|
Caution Review failedThe pull request is closed. WalkthroughThis update introduces support for custom OAuth client configuration in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ComponentFormContainer
participant FormContextProvider
participant ControlApp
participant useAccounts
participant connectAccount
User->>ComponentFormContainer: Passes oauthAppConfig prop
ComponentFormContainer->>FormContextProvider: Receives oauthAppConfig in formProps
FormContextProvider->>ControlApp: Provides oauthAppConfig via context
ControlApp->>useAccounts: Calls with oauthAppId from oauthAppConfig[app.name_slug]
ControlApp->>connectAccount: Calls with oauthAppId from oauthAppConfig[app.name_slug]
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
✨ Finishing Touches
🧪 Generate unit tests
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: 0
🧹 Nitpick comments (2)
packages/connect-react/README.md (2)
156-160: Clarify where the “slug” value comes fromThe term “app name slug” may not be obvious to all users. Consider adding a short parenthetical note or link indicating where to find the slug for a Pipedream app (e.g. “the
slugreturned by the/appsAPI or shown in the app URL”).
162-183: Minor DX improvements for the example snippet
- The comment above declares
oauthAppConfigas aRecord<string, string>, which is fine, but readers may wonder whether these IDs are safe to embed client-side. A brief sentence such as “OAuth app IDs are public identifiers (they are not secrets)” would pre-empt that question.- Mixing
github-create-issue(componentKey) withslack-send-messagein the earlier example may confuse newcomers. Using the same component across examples or adding a note that the change is intentional would avoid second-guessing.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
packages/connect-react/CHANGELOG.md(1 hunks)packages/connect-react/README.md(1 hunks)packages/connect-react/package.json(1 hunks)packages/connect-react/src/components/ComponentForm.tsx(1 hunks)packages/connect-react/src/components/ControlApp.tsx(2 hunks)packages/connect-react/src/hooks/form-context.tsx(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/connect-react/src/components/ControlApp.tsx (1)
packages/connect-react/src/hooks/form-context.tsx (1)
useFormContext(60-69)
⏰ 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). (2)
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
🔇 Additional comments (10)
packages/connect-react/CHANGELOG.md (1)
5-10: LGTM! Well-documented changelog entry.The changelog entry properly documents the new custom OAuth clients feature with appropriate semantic versioning (minor bump) and clear description.
packages/connect-react/package.json (1)
3-3: LGTM! Appropriate version bump.The minor version increment correctly reflects the addition of the new custom OAuth clients feature without breaking changes.
packages/connect-react/src/components/ComponentForm.tsx (1)
32-37: LGTM! Well-designed type definition.The optional
oauthAppConfigproperty is properly typed and includes excellent documentation with a practical example. TheRecord<string, string>type clearly represents the app slug to OAuth app ID mapping.packages/connect-react/src/hooks/form-context.tsx (3)
47-47: LGTM! Proper type integration.The optional
oauthAppConfigproperty is correctly added to theFormContexttype definition.
85-85: LGTM! Consistent prop extraction.The
oauthAppConfigis properly extracted fromformPropsfollowing the established pattern for other properties.
598-598: LGTM! Complete context integration.The
oauthAppConfigis properly included in the context value, completing the integration from props to context consumers.packages/connect-react/src/components/ControlApp.tsx (4)
32-32: LGTM! Proper context consumption.The
oauthAppConfigis correctly extracted from the form context alongsideexternalUserId.
70-70: LGTM! Dynamic OAuth app ID derivation.The
oauthAppIdis properly derived from the configuration using the app's slug (app.name_slug), replacing the previous hardcoded undefined value. This enables custom OAuth app selection per integration.
80-80: LGTM! Correct hook integration.The
oauthAppIdis properly passed to theuseAccountshook, enabling custom OAuth app support for account fetching.
93-93: LGTM! Complete OAuth integration.The
oauthAppIdis correctly passed to theconnectAccountmethod, completing the custom OAuth app integration for the connection flow.
WHY
Summary by CodeRabbit
New Features
oauthAppConfigproperty to customize OAuth app IDs for specific apps.Documentation
Chores