Adding support for oauthAppId in connect-react #17116
Conversation
And patching Discord sentViaPipedream prop label and handling
|
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 8 minutes and 59 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 selected for processing (16)
WalkthroughThis update introduces conditional link generation in Discord action messages based on the presence of a workflow ID, updates property labels and descriptions for clarity, and increments version numbers across Discord components and sources. In the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ComponentForm
participant OAuthAppProvider
participant ControlApp
participant useOAuthAppContext
User->>ComponentForm: Render with props (including oauthAppId)
ComponentForm->>OAuthAppProvider: Wrap children, provide oauthAppId
OAuthAppProvider->>ControlApp: Render children
ControlApp->>useOAuthAppContext: Retrieve oauthAppId from context
useOAuthAppContext-->>ControlApp: Return oauthAppId
ControlApp->>ControlApp: Use oauthAppId in account connection logic
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: 1
🔭 Outside diff range comments (1)
components/discord/actions/common/common.mjs (1)
6-10: 🛠️ Refactor suggestionMissing
label/descriptiononchannelprop – breaks Pipedream lint checksThe CI warning you’re seeing comes from these two required fields:
channel: { type: "$.discord.channel", appProp: "discord", + label: "Channel", + description: "Discord channel where the message will be posted", },🧰 Tools
🪛 GitHub Check: Lint Code Base
[warning] 7-7:
Component prop channel must have a description. See https://pipedream.com/docs/components/guidelines/#props
[warning] 7-7:
Component prop channel must have a label. See https://pipedream.com/docs/components/guidelines/#props🪛 GitHub Actions: Pull Request Checks
[warning] 7-7: Component prop channel must have a label. See https://pipedream.com/docs/components/guidelines/#props (pipedream/props-label)
[warning] 7-7: Component prop channel must have a description. See https://pipedream.com/docs/components/guidelines/#props (pipedream/props-description)
🧹 Nitpick comments (5)
components/discord/actions/common/common.mjs (1)
60-66: Potential 2 000-char overflow when appending attributionDiscord hard-limits message content to 2 000 characters.
appendPipedreamTexttacks on a newline + attribution without guarding against overflow, so longer user messages will error.Option: truncate or throw when
content.length > 1990before appending.components/discord/package.json (1)
3-3: Remember to update the CHANGELOG when publishing 1.2.5
Pure version bump acknowledged.packages/connect-react/package.json (1)
3-3: Consider bumping minor version for new featureAdding the public
oauthAppIdprop constitutes new functionality rather than a pure fix, so semver would normally call for1.4.0instead of1.3.1. This signals to consumers that new capability is available.packages/connect-react/src/components/InternalField.tsx (1)
44-47: Avoid emitting undefined keys inextra
extra: { app, oauthAppId }sendsoauthAppId: undefinedwhen the prop isn’t set.
Not harmful, but for tidiness you could omit the key when undefined:- oauthAppId, + ...(oauthAppId ? { oauthAppId } : {}),packages/connect-react/src/components/ControlApp.tsx (1)
26-32: Fix eslintobject-curly-newlineviolationCurrent formatting violates the repo lint rule, causing the pipeline failure. Apply:
-export function ControlApp({ app, oauthAppId }: ControlAppProps) { +export function ControlApp({ + app, + oauthAppId, +}: ControlAppProps) {🧰 Tools
🪛 GitHub Check: Lint Code Base
[failure] 31-31:
Expected a line break before this closing brace
[failure] 31-31:
Expected a line break after this opening brace🪛 GitHub Actions: Pull Request Checks
[error] 31-31: Expected a line break after this opening brace (object-curly-newline)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
components/discord/actions/common/common.mjs(1 hunks)components/discord/actions/send-message-advanced/send-message-advanced.mjs(1 hunks)components/discord/actions/send-message-with-file/send-message-with-file.mjs(1 hunks)components/discord/actions/send-message/send-message.mjs(1 hunks)components/discord/discord.app.mjs(1 hunks)components/discord/package.json(1 hunks)packages/connect-react/CHANGELOG.md(1 hunks)packages/connect-react/package.json(1 hunks)packages/connect-react/src/components/ComponentForm.tsx(1 hunks)packages/connect-react/src/components/Control.tsx(1 hunks)packages/connect-react/src/components/ControlApp.tsx(2 hunks)packages/connect-react/src/components/InternalField.tsx(2 hunks)packages/connect-react/src/hooks/form-context.tsx(3 hunks)packages/connect-react/src/hooks/form-field-context.tsx(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Pull Request Checks
components/discord/actions/common/common.mjs
[warning] 7-7: Component prop channel must have a label. See https://pipedream.com/docs/components/guidelines/#props (pipedream/props-label)
[warning] 7-7: Component prop channel must have a description. See https://pipedream.com/docs/components/guidelines/#props (pipedream/props-description)
packages/connect-react/src/components/ControlApp.tsx
[error] 31-31: Expected a line break after this opening brace (object-curly-newline)
packages/connect-react/CHANGELOG.md
[warning] 1-1: File ignored because of a matching ignore pattern. Use "--no-ignore" to disable file ignore settings or use "--no-warn-ignored" to suppress this warning
🪛 GitHub Check: Lint Code Base
packages/connect-react/src/components/ControlApp.tsx
[failure] 31-31:
Expected a line break before this closing brace
[failure] 31-31:
Expected a line break after this opening brace
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
🔇 Additional comments (11)
components/discord/discord.app.mjs (1)
14-16: Docstring still accurate after logic change – LGTM
The new label / description now mirror the conditional link generation. No further action needed.components/discord/actions/send-message/send-message.mjs (1)
24-27:messageis required – safe call, LGTM
messageis mandatory in this action, so callingappendPipedreamText(message)can’t raise onundefined. Good.components/discord/actions/send-message-with-file/send-message-with-file.mjs (1)
65-67: Nil-coalescing guard is correct – LGTM
Usingmessage ?? ""avoids passingundefinedtoappendPipedreamText. Looks good.components/discord/actions/send-message-advanced/send-message-advanced.mjs (1)
9-9: Version Bump: Update action version to 1.0.3Aligns with coordinated version updates across Discord actions. No functional changes introduced.
packages/connect-react/src/components/ComponentForm.tsx (1)
32-35: Prop addition is cleanly integratedJSDoc and typing look correct; the field is optional so no breaking change is introduced.
packages/connect-react/src/components/InternalField.tsx (1)
18-20: Destructuring preserves backwards compatibilityForwarding
oauthAppIdthrough context without modifying existing signatures keeps the change safe and non-breaking.packages/connect-react/src/components/Control.tsx (1)
77-77: EnsureControlAppprop typings includeoauthAppId?The value is wired through correctly, but please verify
ControlApp’s prop interface marksoauthAppIdas optional to avoid TypeScript errors when it’s absent.packages/connect-react/src/hooks/form-field-context.tsx (1)
10-11: Context extension looks accurateLimiting
oauthAppIdtoConfigurablePropAppkeeps the extra data scoped to app fields—nice touch.packages/connect-react/CHANGELOG.md (1)
5-23: Changelog entry looks goodVersion bump, description, and usage example are all clear and consistent with the code changes. 👍
packages/connect-react/src/hooks/form-context.tsx (1)
43-48:oauthAppIdcorrectly threaded through contextThe new
oauthAppIdfield is (1) added to theFormContexttype, (2) pulled fromformProps, and (3) injected into the provider value. Nothing else in this file depends on it, so no additional dependency-array updates are needed.
Looks correct. ⚙️Also applies to: 84-86, 597-598
packages/connect-react/src/components/ControlApp.tsx (1)
78-82:✅ Verification successful
Verify param rename from
externalUserId→external_user_id
useAccountsnow expectsexternal_user_id. Ensure every other call site was updated, otherwise queries may silently fail.
🏁 Script executed:
#!/bin/bash # Search remaining call-sites for the old camelCase key. rg -n $'externalUserId:' --type ts,tsxLength of output: 68
🏁 Script executed:
#!/bin/bash # Search for remaining call-sites for the old camelCase key in TS/TSX files rg -n 'externalUserId' --glob '*.ts' --glob '*.tsx'Length of output: 7905
#!/bin/bash # Locate all useAccounts hook invocations within connect-react rg -n "useAccounts" --glob "packages/connect-react/src/**/*.tsx" # Check for external_user_id usage across connect-react components rg -n "external_user_id" --glob "packages/connect-react/src/**/*.{ts,tsx}"
🏁 Script executed:
#!/bin/bash # Search for useAccounts invocations in connect-react rg -n "useAccounts" --glob 'packages/connect-react/src/**/*.{ts,tsx}' # Search for external_user_id usage in connect-react components rg -n "external_user_id" --glob 'packages/connect-react/src/**/*.{ts,tsx}'Length of output: 505
External_user_id rename in useAccounts verified
Searched for
useAccountsinvocations in packages/connect-react and confirmed the only call site in ControlApp.tsx was updated to useexternal_user_id. No remainingexternalUserIdcall sites exist in this package.• Updated location: packages/connect-react/src/components/ControlApp.tsx:78
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
packages/connect-react/src/components/ComponentFormContainer.tsx (1)
14-26: Do thecomponentKeypresence check before callinguseComponent
useComponentis executed even whencomponentKeyis empty / undefined, which may trigger an unnecessary network request (and possibly internal errors) before we throw.
Moving the guard above the hook keeps the call order deterministic while preventing useless work:export function ComponentFormContainer<T extends ConfigurableProps>(props: ComponentFormContainerProps<T>) { - const { + if (!props.componentKey) { + throw new Error("componentKey required"); + } + + const { isLoading, error, component, } = useComponent({ key: props.componentKey, }); - if (!props.componentKey) { - throw new Error("componentKey required"); - }packages/connect-react/README.md (3)
96-105: Snippet mixesuserIdvariable name with newexternalUserIdpropThe example keeps the local variable named
userIdbut passes it toexternalUserId.
Renaming the variable avoids cognitive friction:- const userId = "my-authed-user-id"; + const externalUserId = "my-authed-user-id"; ... - <ComponentFormContainer - externalUserId={userId} + <ComponentFormContainer + externalUserId={externalUserId}
137-160: Prop table: clarify new “optional” semantics
onSubmit,onUpdateConfiguredProps,hideOptionalProps, andsdkResponseare now marked optional, but the docs don’t explain the behavioural change (e.g., is the submit button suppressed whenonSubmitis omitted?).
A one-liner per prop describing what happens when it’s undefined would eliminate guesswork.
191-197: Consistency nit – same variable name issue in customization snippetAs above, the snippet still uses
userIdwhile the prop isexternalUserId. Aligning names keeps the examples consistent.- externalUserId={userId} + externalUserId={externalUserId}
📜 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 (11)
components/discord/sources/message-deleted/message-deleted.mjs(1 hunks)components/discord/sources/new-command-received/new-command-received.mjs(1 hunks)components/discord/sources/new-guild-member/new-guild-member.mjs(1 hunks)components/discord/sources/new-message/new-message.mjs(1 hunks)components/discord/sources/reaction-added/reaction-added.mjs(1 hunks)packages/connect-react/README.md(3 hunks)packages/connect-react/src/components/ComponentForm.tsx(2 hunks)packages/connect-react/src/components/ComponentFormContainer.tsx(1 hunks)packages/connect-react/src/components/ControlApp.tsx(3 hunks)packages/connect-react/src/hooks/oauth-app-context.tsx(1 hunks)packages/connect-react/src/index.ts(1 hunks)
✅ Files skipped from review due to trivial changes (7)
- components/discord/sources/message-deleted/message-deleted.mjs
- components/discord/sources/new-message/new-message.mjs
- packages/connect-react/src/index.ts
- components/discord/sources/reaction-added/reaction-added.mjs
- components/discord/sources/new-guild-member/new-guild-member.mjs
- components/discord/sources/new-command-received/new-command-received.mjs
- packages/connect-react/src/hooks/oauth-app-context.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/connect-react/src/components/ControlApp.tsx
- packages/connect-react/src/components/ComponentForm.tsx
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: pnpm publish
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (1)
packages/connect-react/src/components/ComponentFormContainer.tsx (1)
40-41:componentKeyis now correctly omittedNice tidy-up – removing
componentKeyfrom the spread prevents React from complaining about an unknown prop onComponentForm.
And patching Discord sentViaPipedream prop label and handling
WHY
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores