-
Notifications
You must be signed in to change notification settings - Fork 5.5k
[FEATURE] Figma - Remove Async Options on Project prop #18862
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
base: master
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughUpdates Figma component to comply with OAuth scope restrictions by removing async option methods for fetching projects and files. Removes dependency mappings from property definitions and updates versions across multiple action and source files. Updates platform dependency to version 3.1.0. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
components/figma/actions/delete-comment/delete-comment.mjs (1)
30-38: MakecommentIdrequired and fix description for delete action.This inherits
optional: trueand a “reply” description from the shared prop, which doesn’t fit deletion and can cause invalid calls withundefined.- commentId: { - propDefinition: [ - figmaApp, - "commentId", - ({ fileId }) => ({ - fileId, - }), - ], - }, + commentId: { + propDefinition: [ + figmaApp, + "commentId", + ({ fileId }) => ({ fileId }), + ], + optional: false, + description: "The ID of the comment to delete.", + },components/figma/actions/list-comments/list-comments.mjs (1)
18-23: Remove the unusedprojectIdprop from this action.The description text is correctly defined in
figmaAppwith proper instructions for extracting the Project ID from the Figma URL. However, theprojectIdprop is declared in this action but never used—therunmethod only destructures and usesfileIdto calllistFileComments. This unused prop should be removed from lines 18-23.
🧹 Nitpick comments (5)
components/figma/sources/new-comment/new-comment.mjs (1)
37-42: Harden error handling for non-Axios errors.Accessing
error.response.statuscan throw ifresponseis undefined. Use optional chaining.- if (error.response.status === 400) { + if (error?.response?.status === 400) { throw new ConfigurationError("A Figma Organization Plan or higher is required to use webhooks. See Figma's [pricing page](https://www.figma.com/pricing) for more details."); }components/figma/actions/delete-comment/delete-comment.mjs (1)
17-23: Remove unusedprojectIdprop here.
projectIdisn’t referenced inrun()and no longer drivesfileIdoptions. Remove to streamline the UI.props: { ...common.props, - projectId: { - propDefinition: [ - figmaApp, - "projectId", - ], - }, fileId: {components/figma/figma.app.mjs (1)
27-39: Guard options whenfileIdis unset and prefersliceoversubstr.Prevents accidental
/commentscalls withundefinedand uses non-deprecated string API.- async options({ fileId }) { - const files = await this.listFileComments(fileId); - return files.map((item) => { - const message = item.message.length > 50 - ? `${item.message.substr(0, 50)}...` - : item.message; + async options({ fileId }) { + if (!fileId) return []; + const comments = await this.listFileComments(fileId); + return comments.map((item) => { + const message = item.message.length > 50 + ? `${item.message.slice(0, 50)}...` + : item.message;components/figma/actions/post-a-comment/post-a-comment.mjs (2)
18-23: Remove unusedprojectIdprop.Not referenced in
run(). Removing reduces user friction post-async options removal.props: { ...common.props, - projectId: { - propDefinition: [ - figmaApp, - "projectId", - ], - }, fileId: {
52-55: Build payload conditionally to avoid sendingcomment_id: undefined.Cleaner request body; avoids accidental API complaints.
- const data = { - message, - comment_id: commentId, - }; + const data = commentId + ? { message, comment_id: commentId } + : { message };
📜 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)
components/figma/actions/delete-comment/delete-comment.mjs(1 hunks)components/figma/actions/list-comments/list-comments.mjs(1 hunks)components/figma/actions/post-a-comment/post-a-comment.mjs(1 hunks)components/figma/figma.app.mjs(1 hunks)components/figma/package.json(2 hunks)components/figma/sources/new-comment/new-comment.mjs(1 hunks)
⏰ 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). (4)
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
🔇 Additional comments (7)
components/figma/sources/new-comment/new-comment.mjs (1)
10-10: Version bump LGTM. Aligns with the package update and no behavior change here.components/figma/figma.app.mjs (2)
15-16: Descriptions updated — meets the PR requirement.Clear URL-based instructions for Project ID and File ID. Nice.
Also applies to: 20-21
3-3: Verification confirms no remaining references to removed methods.The search found zero matches for
listTeamProjectsandlistProjectFilesacross the repository, confirming the removed async options are fully cleaned up with no dangling references.components/figma/package.json (1)
3-3: Confirm platform v3 compatibility across this package.Verified: Figma component correctly uses
axiosandConfigurationErrorfrom@pipedream/platformv3.1.0. All imports are standard v3 APIs with no deprecated v1.x dependencies detected. Version constraint^3.1.0is appropriate.components/figma/actions/list-comments/list-comments.mjs (3)
9-9: LGTM!Version bump is appropriate for the prop mapping changes.
31-42: LGTM!The
runmethod correctly fetches and returns file comments with appropriate summary exports.
24-29: Description text is present and appropriate in the fileId propDefinition.Verification confirms that the
fileIdpropDefinition incomponents/figma/figma.app.mjsincludes the required description text guiding users on how to obtain the File ID from Figma URLs. The implementation is consistent withprojectId: both provide clear navigation steps, specify where to find the ID in the URL, and include concrete examples.
WHY
Resolves #18841
Summary by CodeRabbit
New Features
Bug Fixes