Type improvements for configurable props #17056
Conversation
* Add types for the missing configurable props * Fix the `Defaultable` type to correctly handle arrays * Fixed the `ConfigurablePropTimer` type to define cron expressions and time intervals * Mark the `auth` field in the SQL prop type as optional
jverce
added
bug
|
@jverce is attempting to deploy a commit to the Pipedreamers Team on Vercel. A member of the Team first needs to authorize it. |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
|
""" WalkthroughThis update introduces SDK version 1.6.9, enhancing type definitions for configurable properties. The Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SDK
participant Component
User->>SDK: Define configurable property (with new/updated types)
SDK->>Component: Pass property with enhanced typing (e.g., labeled options, timer, etc.)
Component-->>SDK: Use property with improved type safety and optional fields
SDK-->>User: Enforce and expose accurate types in component interface
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/sdk/src/shared/index.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.
Pull Request Overview
This PR improves type definitions for configurable props within the SDK by adding missing types and refining type behaviors.
- Added new types for configurable props including Airtable, Discord, and Timer.
- Fixed the Defaultable type to correctly handle arrays and updated relevant prop types.
- Marked the auth field in the SQL prop type as optional and updated the package version and changelog.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/sdk/src/shared/component.ts | Updated various configurable prop types and refined the Defaultable type |
| packages/sdk/package.json | Bumped version from 1.6.8 to 1.6.9 |
| packages/sdk/CHANGELOG.md | Updated changelog to reflect the new version and changes |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (2)
packages/sdk/src/shared/component.ts (2)
1-1:⚠️ Potential issueIncorrect ESLint disable syntax
// eslint-disable @typescript-eslint/no-explicit-anydoes not disable the rule – ESLint expects either
/* eslint-disable @typescript-eslint/no-explicit-any */(file-wide) or// eslint-disable-next-line …(one line).
Currently the directive is ignored, so theanyusages below will still be lint-checked.-// eslint-disable @typescript-eslint/no-explicit-any +/* eslint-disable @typescript-eslint/no-explicit-any */
181-223:⚠️ Potential issue
PropValueis out of sync with the new prop typesAll newly-added
typestrings resolve tonever, which breaksConfiguredProps<T>and removes
type-safety for consumers.Add cases for each new literal, e.g.:
: T extends "string[]" ? string[] + : T extends "$.interface.timer" + ? TimerInterval | TimerCron + : T extends "$.interface.apphook" + ? unknown // TODO: refine + : T extends "$.interface.http" + ? unknown // TODO: refine + : T extends "$.service.db" + ? unknown // TODO: refine + : T extends "$.discord.channel" + ? string + : T extends "$.discord.channel[]" + ? string[] + : T extends "$.airtable.baseId" + ? string + : T extends "$.airtable.tableId" + ? string + : T extends "$.airtable.viewId" + ? string + : T extends "$.airtable.fieldId" + ? string : T extends "sql" ? { app: string; query: string; params: unknown[] } : never;Failing to update this mapping will surface as
nevertypes or unexpectedanyleakage in downstream code.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/sdk/CHANGELOG.md(1 hunks)packages/sdk/package.json(1 hunks)packages/sdk/src/shared/component.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: test
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
🔇 Additional comments (2)
packages/sdk/package.json (1)
4-4: Version bump acknowledgedPackage version correctly updated to 1.6.9 to reflect the new typings. No issues here.
packages/sdk/CHANGELOG.md (1)
5-17: Changelog entry looks goodThe 1.6.9 notes capture all noteworthy type changes and breaking-risk areas.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/sdk/CHANGELOG.md(1 hunks)packages/sdk/src/shared/index.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/sdk/CHANGELOG.md
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Lint Code Base
- GitHub Check: test
🔇 Additional comments (1)
packages/sdk/src/shared/index.ts (1)
517-519: Pagination intersection looks good, but double-check runtime callersChanging
GetAppsResponsetoPaginationResponse & { data: App[] }is type-safe and aligns with other paginated responses in this file. 👍
Please verify all callers (client.getApps, etc.) that previously assumed only{ data: App[] }now correctly handlepage_info. At runtime nothing breaks, but TypeScript consumers will start seeing property-existence errors if they destructure responses carelessly.
2 participants
Changelog
Defaultabletype to correctly handle arraysConfigurablePropTimertype to define cron expressions and time intervalsauthfield in the SQL prop type as optionalSummary by CodeRabbit