-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Fix issues with authentication for apps with no auth type #17631
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
Fix issues with authentication for apps with no auth type #17631
Conversation
* Ignore the prop configuration and validation for apps with no authentication type (i.e. they do not require authentication, nor have a way to do so). * Replace usage of deprecated `components` method from the SDK with `getComponents` * Fix linter errors in `CHANGELOG.md`
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
|
@jverce is attempting to deploy a commit to the Pipedreamers Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe changes update the Changes
Sequence Diagram(s)sequenceDiagram
participant Component
participant useComponents Hook
participant SDK Client
Component->>useComponents Hook: Call useComponents(input)
useComponents Hook->>SDK Client: getComponents(input)
SDK Client-->>useComponents Hook: Return components list
useComponents Hook-->>Component: Return components data
sequenceDiagram
participant App
participant appPropErrors
App->>appPropErrors: Validate props
alt auth_type is falsy
appPropErrors-->>App: Return []
else auth_type present
appPropErrors->>appPropErrors: Check for errors
appPropErrors-->>App: Return errors[]
end
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
packages/connect-react/src/hooks/use-components.tsxOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs packages/connect-react/src/utils/component.tsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs ✨ 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.
Pull Request Overview
This PR addresses authentication issues for apps that don't require authentication by skipping prop validation and updating deprecated SDK methods. The changes ensure components can be deployed and executed without forcing users to authenticate to apps that have no authentication mechanism.
- Skip prop configuration and validation for apps with no authentication type
- Replace deprecated
componentsmethod withgetComponentsfrom the SDK - Fix markdown formatting issues in the changelog
Reviewed Changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/connect-react/src/utils/component.ts | Added early return for apps without auth_type and simplified auth validation logic |
| packages/connect-react/src/hooks/use-components.tsx | Updated import and method call from deprecated components to getComponents |
| packages/connect-react/package.json | Version bump from 1.3.1 to 1.3.2 |
| packages/connect-react/CHANGELOG.md | Fixed markdown formatting and added changelog entry for version 1.3.2 |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
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 (1)
packages/connect-react/CHANGELOG.md (1)
12-12: Fix informal language usage.Replace "anyways" with "anyway" for more formal documentation.
- authenticate anyways). + authenticate anyway).
📜 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 (4)
packages/connect-react/CHANGELOG.md(2 hunks)packages/connect-react/package.json(1 hunks)packages/connect-react/src/hooks/use-components.tsx(1 hunks)packages/connect-react/src/utils/component.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: jverce
PR: PipedreamHQ/pipedream#15479
File: packages/connect-react/src/components/ComponentForm.tsx:23-24
Timestamp: 2025-02-05T21:58:03.118Z
Learning: In the connect-react package, the `enableDebugging` property should be of type `boolean` as it's used for toggling debugging features and conditional rendering.
packages/connect-react/CHANGELOG.md (1)
Learnt from: jverce
PR: PipedreamHQ/pipedream#15479
File: packages/connect-react/src/components/ComponentForm.tsx:23-24
Timestamp: 2025-02-05T21:58:03.118Z
Learning: In the connect-react package, the `enableDebugging` property should be of type `boolean` as it's used for toggling debugging features and conditional rendering.
🧬 Code Graph Analysis (1)
packages/connect-react/src/hooks/use-components.tsx (1)
packages/sdk/src/shared/index.ts (1)
GetComponentsOpts(454-469)
🪛 LanguageTool
packages/connect-react/CHANGELOG.md
[style] ~12-~12: The word ‘anyways’ is informal American English. Did you mean “anyway”?
Context: ...pp doesn't have a way to authenticate anyways). - Use the getComponents method from...
(ANYWAYS)
🔇 Additional comments (6)
packages/connect-react/src/hooks/use-components.tsx (1)
2-2: LGTM! SDK method migration correctly implemented.The changes properly migrate from the deprecated
componentsmethod togetComponents, with consistent type updates and method calls.Also applies to: 8-8, 15-15
packages/connect-react/src/utils/component.ts (2)
155-159: LGTM! Correctly implements authentication bypass for apps without auth type.The early return properly skips validation for apps that don't require authentication, aligning with the PR objective.
172-186: Error collection remains correctly gated
The existing checks (app.auth_type === "oauth"andapp.auth_type === "oauth" || app.auth_type === "keys") already prevent any error pushes for apps without authentication. Removing the outerif (app.auth_type)guard does not introduce unintended error collection paths.Likely an incorrect or invalid review comment.
packages/connect-react/package.json (1)
3-3: LGTM! Version bump is appropriate.The patch version increment correctly reflects the bug fixes and deprecated method replacement documented in this release.
packages/connect-react/CHANGELOG.md (2)
5-15: LGTM! Changelog accurately documents the release changes.The new version entry properly documents both the authentication handling improvement and the SDK method migration.
16-183: LGTM! Consistent heading formatting applied.The standardization of heading levels improves the changelog's structure and readability.
Changelog
componentsmethod from the SDK withgetComponentsCHANGELOG.mdWHY
This will allow components to be deployed/executed without users having to
authenticate to the app (which would fail given that the app doesn't have a way
to authenticate anyways).
Summary by CodeRabbit
Bug Fixes
Documentation