-
Notifications
You must be signed in to change notification settings - Fork 5.4k
[BUG] Fix packages with Import failure #17881
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
|
WalkthroughThis update addresses import and packaging issues for several components by updating package metadata (such as adding or correcting the Changes
Sequence Diagram(s)Example: Pdfless Generate PDF (Refactored Flow)sequenceDiagram
participant User
participant PdflessAction
participant PdflessApp
participant PdflessAPI
User->>PdflessAction: Trigger generate-pdf
PdflessAction->>PdflessApp: Call generate({templateId, payload})
PdflessApp->>PdflessAPI: POST /pdfs with template_id and payload
PdflessAPI-->>PdflessApp: PDF generation response
PdflessApp-->>PdflessAction: Return response
PdflessAction-->>User: Success/Failure summary
Example: Shortcut Create Story (New Flow)sequenceDiagram
participant User
participant CreateStoryAction
participant ShortcutApp
participant ShortcutAPI
User->>CreateStoryAction: Provide story details
CreateStoryAction->>ShortcutApp: callWithRetry(createStory)
ShortcutApp->>ShortcutAPI: POST /stories with payload
ShortcutAPI-->>ShortcutApp: Story created response
ShortcutApp-->>CreateStoryAction: Return story data
CreateStoryAction-->>User: Story info
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Suggested labels
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 ignored due to path filters (1)
📒 Files selected for processing (31)
✅ Files skipped from review due to trivial changes (5)
🚧 Files skipped from review as they are similar to previous changes (24)
🧰 Additional context used🧠 Learnings (1)📚 Learning: in the salesloft api integration (components/salesloft/salesloft.app.mjs), the _makerequest method r...
Applied to files:
⏰ 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)
🔇 Additional comments (4)
✨ 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. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
c9006e8
to
36880b0
Compare
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: 1
🔭 Outside diff range comments (9)
components/puppeteer/puppeteer.app.mjs (2)
59-61
: Await thelaunch()
promise before using the browser instance
this.launch()
returns a Promise; callingbrowser.newPage()
without awaiting will throwTypeError: browser.newPage is not a function
.-const browser = this.launch(); -const page = await browser.newPage(); +const browser = await this.launch(); +const page = await browser.newPage();
75-80
: Same await problem propagates togoto()
helper
this.newPage()
isasync
but you are not awaiting it.-const { page, browser } = this.newPage(); +const { page, browser } = await this.newPage();components/netlify/netlify.app.mjs (1)
139-140
: Potential crash whenLink
header absent
parseLinkHeader(headers.link)
returnsnull
when the server omits theLink
header. Destructuringconst { next } = null
throws. Guarding avoids runtime failure:-const { next } = parseLinkHeader(headers.link); +const link = parseLinkHeader(headers.link); +const next = link?.next;components/netlify/actions/rollback-deploy/rollback-deploy.mjs (1)
27-30
: Await the rollback call to avoid returning a pending Promise
this.netlify.rollbackDeploy()
is an async network call. Returning the unresolved promise breaks$summary
timing and prevents proper error propagation.- const response = this.netlify.rollbackDeploy(this.siteId, this.deployId); + const response = await this.netlify.rollbackDeploy( + this.siteId, + this.deployId, + );components/playwright/playwright.app.mjs (2)
57-60
:await
the browser before calling.newPage()
this.launch()
returns a Promise. Calling.newPage()
on the promise throws at runtime.- const browser = this.launch(); + const browser = await this.launch();
73-77
:goto()
helper suffers from the same missingawait
- const { - page, browser, - } = this.newPage(); + const { + page, browser, + } = await this.newPage();components/shortcut/actions/create-story/create-story.mjs (1)
300-306
: Typo breaks validation oflinkedFileIds
inkedFileIds
is a misspelling, solinkedFileIds
never gets validated by validate.js.- inkedFileIds: utils.validateArrayWhenPresent, + linkedFileIds: utils.validateArrayWhenPresent,components/shortcut/shortcut.app.mjs (2)
24-41
:bail
expects anError
instance – passing a string drops stack traces
This hampers debugging and violatesasync-retry
’s contract.-if (!this._isRetriableStatusCode(statusCode)) { - bail(` - Unexpected error (status code: ${statusCode}): - ${JSON.stringify(err.message)} - `); +if (!this._isRetriableStatusCode(statusCode)) { + bail( + new Error( + `Unexpected error (status code: ${statusCode}): ${err.message}`, + ), + ); }
101-106
:decodeURIComponent
is called even whenresults.data.next
is null → TypeError
When the final page is reached,results.data.next
is usuallynull
; decoding it will crash the loop and mask a successful fetch.-const decodedNext = decodeURIComponent(results.data.next); -const idxQuestionMark = decodedNext.indexOf("?"); -const nextQueryString = decodedNext.substring(idxQuestionMark + 1); -let searchParams = new URLSearchParams(nextQueryString); -next = searchParams.get("next"); +if (results.data.next) { + const decodedNext = decodeURIComponent(results.data.next); + const idxQuestionMark = decodedNext.indexOf("?"); + const nextQueryString = decodedNext.substring(idxQuestionMark + 1); + const searchParams = new URLSearchParams(nextQueryString); + next = searchParams.get("next"); +} else { + next = undefined; +}
🧹 Nitpick comments (9)
components/netlify/actions/list-files/list-files.mjs (1)
7-7
: Version bump acknowledged—sync ancillary assets
Theversion
property has been bumped to0.1.1
, matching the coordinated Netlify release. Confirm that:
components/netlify/package.json
reflects the same bump (already appears to be0.4.2
);- Any changelog or release-notes generator picks this up.
No code-level concerns.
components/playwright/actions/get-page-title/get-page-title.mjs (1)
7-7
: Version updated, logic unchanged
Increment to0.0.3
is fine. Remember to regenerate any cached component metadata used in the catalog so consumers see the new version promptly.components/playwright/playwright.app.mjs (1)
4-8
: Comment now contradicts the code – update the docstringLines 4-5 still claim Playwright is “locked to an old version”, but the import was changed to the unpinned
playwright-core
. Update the explanatory comment so future maintainers aren’t misled.components/shortcut/package.json (1)
1-21
: Declare the package as ESM via the"type": "module"
fieldAll source files in this package use the
.mjs
extension and nativeimport
/export
syntax. Declaring the module type explicitly avoids surprises for downstream tooling ( e.g. Jest, ts-node, build pipelines ) that default to CommonJS when the field is missing."main": "shortcut.app.mjs", + "type": "module",
components/shortcut/common/utils.mjs (1)
25-27
:convertEmptyStringToUndefined
treats falsy numbers as empty
value || undefined
converts0
toundefined
, which may be unintended if numeric zero is ever a valid value. A safer guard checks specifically for empty strings/nullish values:- return value || undefined; + return value === "" || value == null ? undefined : value;components/shortcut/actions/create-story/create-story.mjs (2)
61-64
: Repeated option-loading pattern – extract to helper to reduce duplicationSix nearly identical blocks build select-list options from paginated API calls. Factor this into a small helper (e.g.
utils.listToOptions(apiMethod, labelKey, valueKey)
) to keep the action readable and lower maintenance cost.Also applies to: 103-106, 135-138, 165-168, 201-204, 265-275
1-3
: Importing full lodash inflates bundle sizeOnly
get
is used. Importing the scoped path keeps the component smaller:-import lodash from "lodash"; +import get from "lodash/get.js"; ... -const isEpicDataAvailable = lodash.get(epics, ["data", "length"]); +const isEpicDataAvailable = get(epics, ["data", "length"]);Repeat for other occurrences.
components/shortcut/shortcut.app.mjs (2)
2-2
: Importing the wholelodash
bundle inflates bundle size unnecessarily
Onlyget
is used – pulling the entire library increases cold-start time for users’ workflows.-import lodash from "lodash"; +import get from "lodash/get";Update the three
lodash.get
calls accordingly:-const statusCode = lodash.get(err, [ +const statusCode = get(err, [ … -const isMembersDataAvailable = lodash.get(members, [ +const isMembersDataAvailable = get(members, [ … -const isStoryDataAvailable = lodash.get(results, [ +const isStoryDataAvailable = get(results, [
12-18
: Include standard 5xx codes in the retry whitelist
Transient upstream errors like 502, 503, 504 are common; excluding them prevents automatic recovery.- 408, - 429, - 500, + 408, // Request Timeout + 429, // Too Many Requests + 500, // Internal Server Error + 502, // Bad Gateway + 503, // Service Unavailable + 504, // Gateway Timeout
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (33)
components/google_tag_manager/package.json
(2 hunks)components/netlify/actions/get-site/get-site.mjs
(1 hunks)components/netlify/actions/list-files/list-files.mjs
(1 hunks)components/netlify/actions/list-site-deploys/list-site-deploys.mjs
(1 hunks)components/netlify/actions/rollback-deploy/rollback-deploy.mjs
(1 hunks)components/netlify/netlify.app.mjs
(1 hunks)components/netlify/package.json
(2 hunks)components/netlify/sources/new-deploy-failure/new-deploy-failure.mjs
(1 hunks)components/netlify/sources/new-deploy-start/new-deploy-start.mjs
(1 hunks)components/netlify/sources/new-deploy-success/new-deploy-success.mjs
(1 hunks)components/netlify/sources/new-form-submission/new-form-submission.mjs
(1 hunks)components/pdfless/actions/generate-pdf/generate-pdf.mjs
(1 hunks)components/pdfless/package.json
(1 hunks)components/pdfless/pdfless.app.mjs
(2 hunks)components/playwright/actions/get-page-html/get-page-html.mjs
(1 hunks)components/playwright/actions/get-page-title/get-page-title.mjs
(1 hunks)components/playwright/actions/page-pdf/page-pdf.mjs
(1 hunks)components/playwright/actions/take-screenshot/take-screenshot.mjs
(1 hunks)components/playwright/package.json
(2 hunks)components/playwright/playwright.app.mjs
(1 hunks)components/puppeteer/actions/get-html/get-html.mjs
(1 hunks)components/puppeteer/actions/get-page-title/get-page-title.mjs
(1 hunks)components/puppeteer/actions/get-pdf/get-pdf.mjs
(1 hunks)components/puppeteer/actions/screenshot-page/screenshot-page.mjs
(1 hunks)components/puppeteer/package.json
(2 hunks)components/puppeteer/puppeteer.app.mjs
(1 hunks)components/shortcut/actions/create-story/create-story.mjs
(7 hunks)components/shortcut/actions/search-stories/search-stories.mjs
(1 hunks)components/shortcut/common/constants.mjs
(1 hunks)components/shortcut/common/utils.mjs
(1 hunks)components/shortcut/package.json
(2 hunks)components/shortcut/shortcut.app.mjs
(4 hunks)components/zoho_people/package.json
(2 hunks)
🧰 Additional context used
🧠 Learnings (15)
components/playwright/package.json (1)
Learnt from: jcortes
PR: #14935
File: components/sailpoint/package.json:15-18
Timestamp: 2024-12-12T19:23:09.039Z
Learning: When developing Pipedream components, do not add built-in Node.js modules like fs
to package.json
dependencies, as they are native modules provided by the Node.js runtime.
components/netlify/sources/new-deploy-failure/new-deploy-failure.mjs (2)
Learnt from: GTFalcao
PR: #12697
File: components/salesforce_rest_api/sources/common-webhook-methods.mjs:1-71
Timestamp: 2024-07-24T02:06:47.016Z
Learning: The common-webhook-methods.mjs
object is designed to be extended, similar to an abstract class, and intentionally does not implement certain methods like generateWebhookMeta
and getEventType
to enforce implementation in subclasses.
Learnt from: GTFalcao
PR: #12697
File: components/salesforce_rest_api/sources/common-webhook-methods.mjs:1-71
Timestamp: 2024-10-08T15:33:38.240Z
Learning: The common-webhook-methods.mjs
object is designed to be extended, similar to an abstract class, and intentionally does not implement certain methods like generateWebhookMeta
and getEventType
to enforce implementation in subclasses.
components/netlify/sources/new-deploy-success/new-deploy-success.mjs (2)
Learnt from: GTFalcao
PR: #12697
File: components/salesforce_rest_api/sources/common-webhook-methods.mjs:1-71
Timestamp: 2024-07-24T02:06:47.016Z
Learning: The common-webhook-methods.mjs
object is designed to be extended, similar to an abstract class, and intentionally does not implement certain methods like generateWebhookMeta
and getEventType
to enforce implementation in subclasses.
Learnt from: GTFalcao
PR: #12697
File: components/salesforce_rest_api/sources/common-webhook-methods.mjs:1-71
Timestamp: 2024-10-08T15:33:38.240Z
Learning: The common-webhook-methods.mjs
object is designed to be extended, similar to an abstract class, and intentionally does not implement certain methods like generateWebhookMeta
and getEventType
to enforce implementation in subclasses.
components/pdfless/actions/generate-pdf/generate-pdf.mjs (1)
Learnt from: jcortes
PR: #14467
File: components/gainsight_px/actions/create-account/create-account.mjs:4-6
Timestamp: 2024-10-30T15:24:39.294Z
Learning: In components/gainsight_px/actions/create-account/create-account.mjs
, the action name should be "Create Account" instead of "Create Memory".
components/netlify/package.json (1)
Learnt from: jcortes
PR: #14935
File: components/sailpoint/package.json:15-18
Timestamp: 2024-12-12T19:23:09.039Z
Learning: When developing Pipedream components, do not add built-in Node.js modules like fs
to package.json
dependencies, as they are native modules provided by the Node.js runtime.
components/shortcut/common/utils.mjs (2)
Learnt from: GTFalcao
PR: #12731
File: components/hackerone/actions/get-members/get-members.mjs:3-28
Timestamp: 2024-10-08T15:33:38.240Z
Learning: When exporting a summary message in the run
method of an action, ensure the message is correctly formatted. For example, in the hackerone-get-members
action, the correct format is Successfully retrieved ${response.data.length} members
.
Learnt from: GTFalcao
PR: #12731
File: components/hackerone/actions/get-members/get-members.mjs:3-28
Timestamp: 2024-07-04T18:11:59.822Z
Learning: When exporting a summary message in the run
method of an action, ensure the message is correctly formatted. For example, in the hackerone-get-members
action, the correct format is Successfully retrieved ${response.data.length} members
.
components/puppeteer/package.json (1)
Learnt from: jcortes
PR: #14935
File: components/sailpoint/package.json:15-18
Timestamp: 2024-12-12T19:23:09.039Z
Learning: When developing Pipedream components, do not add built-in Node.js modules like fs
to package.json
dependencies, as they are native modules provided by the Node.js runtime.
components/pdfless/package.json (1)
Learnt from: jcortes
PR: #14935
File: components/sailpoint/package.json:15-18
Timestamp: 2024-12-12T19:23:09.039Z
Learning: When developing Pipedream components, do not add built-in Node.js modules like fs
to package.json
dependencies, as they are native modules provided by the Node.js runtime.
components/shortcut/package.json (2)
Learnt from: js07
PR: #17375
File: components/zerobounce/actions/get-validation-results-file/get-validation-results-file.mjs:23-27
Timestamp: 2025-07-01T17:07:48.193Z
Learning: "dir" props in Pipedream components are hidden in the component form and not user-facing, so they don't require labels or descriptions for user clarity.
Learnt from: jcortes
PR: #14935
File: components/sailpoint/package.json:15-18
Timestamp: 2024-12-12T19:23:09.039Z
Learning: When developing Pipedream components, do not add built-in Node.js modules like fs
to package.json
dependencies, as they are native modules provided by the Node.js runtime.
components/netlify/sources/new-form-submission/new-form-submission.mjs (2)
Learnt from: GTFalcao
PR: #12697
File: components/salesforce_rest_api/sources/common-webhook-methods.mjs:1-71
Timestamp: 2024-07-24T02:06:47.016Z
Learning: The common-webhook-methods.mjs
object is designed to be extended, similar to an abstract class, and intentionally does not implement certain methods like generateWebhookMeta
and getEventType
to enforce implementation in subclasses.
Learnt from: GTFalcao
PR: #12697
File: components/salesforce_rest_api/sources/common-webhook-methods.mjs:1-71
Timestamp: 2024-10-08T15:33:38.240Z
Learning: The common-webhook-methods.mjs
object is designed to be extended, similar to an abstract class, and intentionally does not implement certain methods like generateWebhookMeta
and getEventType
to enforce implementation in subclasses.
components/netlify/sources/new-deploy-start/new-deploy-start.mjs (3)
Learnt from: GTFalcao
PR: #15376
File: components/monday/sources/name-updated/name-updated.mjs:6-6
Timestamp: 2025-01-23T03:55:15.166Z
Learning: Source names in Monday.com components don't need to start with "New" if they emit events for updated items (e.g., "Name Updated", "Column Value Updated") rather than new items. This follows the component guidelines exception where the "New" prefix is only required when emits are limited to new items.
Learnt from: GTFalcao
PR: #12697
File: components/salesforce_rest_api/sources/common-webhook-methods.mjs:1-71
Timestamp: 2024-07-24T02:06:47.016Z
Learning: The common-webhook-methods.mjs
object is designed to be extended, similar to an abstract class, and intentionally does not implement certain methods like generateWebhookMeta
and getEventType
to enforce implementation in subclasses.
Learnt from: GTFalcao
PR: #12697
File: components/salesforce_rest_api/sources/common-webhook-methods.mjs:1-71
Timestamp: 2024-10-08T15:33:38.240Z
Learning: The common-webhook-methods.mjs
object is designed to be extended, similar to an abstract class, and intentionally does not implement certain methods like generateWebhookMeta
and getEventType
to enforce implementation in subclasses.
components/zoho_people/package.json (1)
Learnt from: jcortes
PR: #14935
File: components/sailpoint/package.json:15-18
Timestamp: 2024-12-12T19:23:09.039Z
Learning: When developing Pipedream components, do not add built-in Node.js modules like fs
to package.json
dependencies, as they are native modules provided by the Node.js runtime.
components/google_tag_manager/package.json (1)
Learnt from: jcortes
PR: #14935
File: components/sailpoint/package.json:15-18
Timestamp: 2024-12-12T19:23:09.039Z
Learning: When developing Pipedream components, do not add built-in Node.js modules like fs
to package.json
dependencies, as they are native modules provided by the Node.js runtime.
components/shortcut/shortcut.app.mjs (1)
Learnt from: GTFalcao
PR: #16954
File: components/salesloft/salesloft.app.mjs:14-23
Timestamp: 2025-06-04T17:52:05.780Z
Learning: In the Salesloft API integration (components/salesloft/salesloft.app.mjs), the _makeRequest method returns response.data which directly contains arrays for list endpoints like listPeople, listCadences, listUsers, and listAccounts. The propDefinitions correctly call .map() directly on these responses without needing to destructure a nested data property.
components/shortcut/actions/create-story/create-story.mjs (3)
Learnt from: jcortes
PR: #14467
File: components/gainsight_px/actions/create-account/create-account.mjs:4-6
Timestamp: 2024-10-30T15:24:39.294Z
Learning: In components/gainsight_px/actions/create-account/create-account.mjs
, the action name should be "Create Account" instead of "Create Memory".
Learnt from: js07
PR: #17375
File: components/tinypng/actions/convert-image/convert-image.mjs:33-37
Timestamp: 2025-07-01T16:56:20.177Z
Learning: 'dir' props with sync functionality do not expose a path to the sync directory directly. For files to be synced, they must be either: (1) written to process.env.STASH_DIR
(/tmp/__stash
), or (2) written to /tmp
and have their file path returned from the run
method as a string, [filepath]
, [_, filepath]
, { filePath }
, { filepath }
, or { path }
.
Learnt from: GTFalcao
PR: #16954
File: components/salesloft/salesloft.app.mjs:14-23
Timestamp: 2025-06-04T17:52:05.780Z
Learning: In the Salesloft API integration (components/salesloft/salesloft.app.mjs), the _makeRequest method returns response.data which directly contains arrays for list endpoints like listPeople, listCadences, listUsers, and listAccounts. The propDefinitions correctly call .map() directly on these responses without needing to destructure a nested data property.
⏰ 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). (3)
- GitHub Check: Publish TypeScript components
- GitHub Check: Lint Code Base
- GitHub Check: Verify TypeScript components
🔇 Additional comments (29)
components/pdfless/package.json (1)
3-3
: LGTM! Version bump aligns with import fix.The version increment from 0.1.0 to 0.1.1 is appropriate for the bug fix addressing ES module import failures. The package correctly has the "main" field pointing to "pdfless.app.mjs", which helps resolve the import issues mentioned in the linked issue.
components/pdfless/pdfless.app.mjs (3)
1-10
: Excellent solution for ES module compatibility issues!The dynamic import implementation properly addresses the ES module compatibility problems mentioned in the PR objectives. The lazy loading pattern with conditional initialization is well-implemented and will resolve the "requiring ES modules from CommonJS modules" issue.
32-40
: Async method update is correct.The
listTemplatesOpts
method is properly updated to be async and awaits service initialization before usingTemplateService
. This ensures the dynamic import completes before service instantiation.
41-52
: Generate method properly handles async initialization.The
generate
method correctly awaits service initialization before usingPdfService
. The async/await pattern is consistently applied and maintains the same functionality while resolving the import compatibility issues.components/pdfless/actions/generate-pdf/generate-pdf.mjs (1)
5-5
: Version bump aligns with coordinated import fix.The version increment from "0.0.1" to "0.0.2" is appropriate and aligns with the dynamic import changes in the parent
pdfless.app.mjs
module. This coordinated versioning properly reflects the bug fix for ES module compatibility issues.components/playwright/package.json (1)
3-3
: Dependency bump looks good – confirm no additional peer/engine constraints are required
@pipedream/types
is a pure-type helper and should not affect runtime size, but you may want to double-check that all consumers compile against the new minor (0.3.x
) range and that the component’s"engines"
field (if any) still reflects the minimum Node version required by the latest@pipedream/types
.Would you run CI locally or the automated canary build to ensure the new dependency does not introduce type-checking regressions?
Also applies to: 15-18
components/zoho_people/package.json (1)
5-6
: Main entry-point file confirmed present
The filecomponents/zoho_people/zoho_people.app.mjs
exists and is correctly named; no further action needed.components/google_tag_manager/package.json (1)
5-6
: Main file verified – import will succeed
I confirmed thatgoogle_tag_manager.app.mjs
exists atcomponents/google_tag_manager/google_tag_manager.app.mjs
, matching the"main"
entry in package.json. No further action needed.components/netlify/actions/get-site/get-site.mjs (1)
7-7
: Version bump acknowledged
No functional changes – version increment to0.1.1
is fine.components/puppeteer/puppeteer.app.mjs (1)
4-5
: Switching to bare imports may re-introduce the “unfulfilled promise” bug
The comment directly above still references the need to pin to an old Chromium build. If that bug has not been fixed upstream, dropping the sub-path version lock could break production. Please confirm with a quick smoke test.components/playwright/actions/get-page-html/get-page-html.mjs (1)
7-7
: Version bump only – looks goodThe increment to
0.0.3
keeps semantic versioning in sync with the rest of the Playwright package. No functional code touched.components/puppeteer/actions/get-page-title/get-page-title.mjs (1)
10-10
: Version bump only – looks good
1.0.3
aligns this action with the coordinated package update. No other changes.components/puppeteer/actions/get-html/get-html.mjs (1)
10-10
: Version bump only – looks goodSyncs the action to
1.0.3
; no behavioral impact.components/netlify/actions/list-site-deploys/list-site-deploys.mjs (1)
7-7
: Version bump only – looks goodUpdate to
0.1.1
matches the wider Netlify package release. Implementation unchanged.components/playwright/actions/take-screenshot/take-screenshot.mjs (1)
7-7
: Version bump only – looks goodMoving to
0.0.3
keeps the Playwright actions aligned; no logic modified.components/netlify/sources/new-deploy-failure/new-deploy-failure.mjs (1)
10-10
: Consistent version increment—double-check dedupe impact
The source moves to0.1.1
. Looks good. Just verify thededupe: "unique"
behaviour hasn’t changed across versions, otherwise no action required.components/playwright/actions/page-pdf/page-pdf.mjs (1)
7-7
: Playwright action version bump OK
0.0.3
aligns with the package-wide bump. Ensure dependency updates (@pipedream/types
, unpinnedplaywright-core
) are covered by integration tests, but no code changes here.components/netlify/sources/new-deploy-success/new-deploy-success.mjs (1)
10-10
: Version bump confirmed—no functional changes
0.1.1
matches the rest of the Netlify source bumps. No issues detected.components/puppeteer/actions/get-pdf/get-pdf.mjs (1)
12-12
: Version bump acknowledged – no issues spotted
The isolated change to theversion
field looks consistent with the coordinated package-wide bump.components/netlify/package.json (1)
3-3
: Major platform-SDK jump – double-check for breaking changes
@pipedream/platform
is upgraded from a 1.x range to^3.1.0
. That’s a two-major leap; silent breaking changes are likely. Please confirm that all Netlify components still build & execute under v3 (e.g. prop-type changes, renamed helpers, auth handling).Also applies to: 13-13
components/netlify/sources/new-deploy-start/new-deploy-start.mjs (1)
10-10
: Version bump only – looks good
Nothing else changed in the source definition.components/netlify/netlify.app.mjs (1)
7-8
: Import style switch correctly resolves CJS ↔ ESM mismatch
Replacing the named import with default-import-then-destructure is the right fix fornetlify
’s CommonJS export pattern. No functional concerns.components/netlify/sources/new-form-submission/new-form-submission.mjs (1)
10-10
: Version bump only – looks good
Change is limited to metadata.components/netlify/actions/rollback-deploy/rollback-deploy.mjs (1)
7-7
: Version bump looks goodThe patch-level bump is consistent with the rest of the Netlify component set.
components/puppeteer/package.json (2)
3-3
: 👍 Package version bump is fineNo concerns with
1.0.4
.
16-16
: Verify platform-SDK major upgradeJumping from
@pipedream/platform@^1.x
to^3.1.0
is a breaking-change range. Make sure every action & app in this package has been smoke-tested against v3 APIs (e.g.$request
,$actions
,$bucket
).components/puppeteer/actions/screenshot-page/screenshot-page.mjs (1)
13-13
: Patch version bump acknowledgedNo functional changes in this file.
components/shortcut/common/constants.mjs (1)
1-7
: Looks good – constant list correctly migrated to ESM
No functional issues identified.components/shortcut/actions/search-stories/search-stories.mjs (1)
1-27
: Action migrated cleanly to ESM
Import path, props, andrun
implementation look correct.
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.
LGTM! Ready for QA!
18155d5
to
4170ea9
Compare
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: 1
🔭 Outside diff range comments (1)
components/shortcut/actions/create-story/create-story.mjs (1)
289-289
: Fix typo in constraint property name.There's a typo:
inkedFileIds
should belinkedFileIds
.Apply this diff to fix the typo:
- inkedFileIds: utils.validateArrayWhenPresent, + linkedFileIds: utils.validateArrayWhenPresent,
🧹 Nitpick comments (1)
components/pdfless/pdfless.app.mjs (1)
11-21
: Consider adding error handling for API requests.The pagination implementation looks correct, but the method lacks error handling. If
listTemplates
fails, users will see an unhelpful error in the UI.Consider adding a try-catch block:
async options({ page }) { + try { const templates = await this.listTemplates({ params: { page: page + 1, }, }); return templates.map((template) => ({ label: template.name, value: template.id, })); + } catch (error) { + console.error("Failed to fetch templates:", error); + return []; + } },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (32)
components/google_tag_manager/package.json
(2 hunks)components/netlify/actions/get-site/get-site.mjs
(2 hunks)components/netlify/actions/list-files/list-files.mjs
(2 hunks)components/netlify/actions/list-site-deploys/list-site-deploys.mjs
(1 hunks)components/netlify/actions/rollback-deploy/rollback-deploy.mjs
(2 hunks)components/netlify/package.json
(2 hunks)components/netlify/sources/new-deploy-failure/new-deploy-failure.mjs
(1 hunks)components/netlify/sources/new-deploy-start/new-deploy-start.mjs
(1 hunks)components/netlify/sources/new-deploy-success/new-deploy-success.mjs
(1 hunks)components/netlify/sources/new-form-submission/new-form-submission.mjs
(1 hunks)components/pdfless/actions/generate-pdf/generate-pdf.mjs
(2 hunks)components/pdfless/package.json
(2 hunks)components/pdfless/pdfless.app.mjs
(2 hunks)components/playwright/actions/get-page-html/get-page-html.mjs
(1 hunks)components/playwright/actions/get-page-title/get-page-title.mjs
(1 hunks)components/playwright/actions/page-pdf/page-pdf.mjs
(1 hunks)components/playwright/actions/take-screenshot/take-screenshot.mjs
(1 hunks)components/playwright/package.json
(2 hunks)components/playwright/playwright.app.mjs
(1 hunks)components/puppeteer/actions/get-html/get-html.mjs
(1 hunks)components/puppeteer/actions/get-page-title/get-page-title.mjs
(1 hunks)components/puppeteer/actions/get-pdf/get-pdf.mjs
(3 hunks)components/puppeteer/actions/screenshot-page/screenshot-page.mjs
(4 hunks)components/puppeteer/package.json
(2 hunks)components/puppeteer/puppeteer.app.mjs
(1 hunks)components/shortcut/actions/create-story/create-story.mjs
(9 hunks)components/shortcut/actions/search-stories/search-stories.mjs
(1 hunks)components/shortcut/common/constants.mjs
(1 hunks)components/shortcut/common/utils.mjs
(2 hunks)components/shortcut/package.json
(2 hunks)components/shortcut/shortcut.app.mjs
(5 hunks)components/zoho_people/package.json
(2 hunks)
✅ Files skipped from review due to trivial changes (10)
- components/playwright/actions/get-page-html/get-page-html.mjs
- components/puppeteer/puppeteer.app.mjs
- components/playwright/actions/get-page-title/get-page-title.mjs
- components/netlify/package.json
- components/puppeteer/package.json
- components/netlify/actions/list-site-deploys/list-site-deploys.mjs
- components/playwright/actions/take-screenshot/take-screenshot.mjs
- components/shortcut/actions/search-stories/search-stories.mjs
- components/google_tag_manager/package.json
- components/playwright/playwright.app.mjs
🚧 Files skipped from review as they are similar to previous changes (19)
- components/pdfless/package.json
- components/netlify/sources/new-form-submission/new-form-submission.mjs
- components/playwright/package.json
- components/netlify/sources/new-deploy-success/new-deploy-success.mjs
- components/puppeteer/actions/get-html/get-html.mjs
- components/playwright/actions/page-pdf/page-pdf.mjs
- components/netlify/sources/new-deploy-failure/new-deploy-failure.mjs
- components/puppeteer/actions/get-page-title/get-page-title.mjs
- components/zoho_people/package.json
- components/shortcut/common/constants.mjs
- components/netlify/actions/rollback-deploy/rollback-deploy.mjs
- components/netlify/actions/get-site/get-site.mjs
- components/netlify/sources/new-deploy-start/new-deploy-start.mjs
- components/puppeteer/actions/get-pdf/get-pdf.mjs
- components/shortcut/package.json
- components/netlify/actions/list-files/list-files.mjs
- components/puppeteer/actions/screenshot-page/screenshot-page.mjs
- components/pdfless/actions/generate-pdf/generate-pdf.mjs
- components/shortcut/shortcut.app.mjs
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: in `components/gainsight_px/actions/create-account/create-account.mjs`, the action name should be "c...
Learnt from: jcortes
PR: PipedreamHQ/pipedream#14467
File: components/gainsight_px/actions/create-account/create-account.mjs:4-6
Timestamp: 2024-10-30T15:24:39.294Z
Learning: In `components/gainsight_px/actions/create-account/create-account.mjs`, the action name should be "Create Account" instead of "Create Memory".
Applied to files:
components/shortcut/actions/create-story/create-story.mjs
📚 Learning: for "dir" props in pipedream components, whether to mark them as optional depends on the action's fi...
Learnt from: js07
PR: PipedreamHQ/pipedream#17375
File: components/zerobounce/actions/get-validation-results-file/get-validation-results-file.mjs:23-27
Timestamp: 2025-07-01T17:07:48.193Z
Learning: For "dir" props in Pipedream components, whether to mark them as optional depends on the action's file I/O behavior - if an action always writes files as output, the "dir" prop should not be marked as optional.
Applied to files:
components/shortcut/actions/create-story/create-story.mjs
📚 Learning: 'dir' props with sync functionality do not expose a path to the sync directory directly. for files t...
Learnt from: js07
PR: PipedreamHQ/pipedream#17375
File: components/tinypng/actions/convert-image/convert-image.mjs:33-37
Timestamp: 2025-07-01T16:56:20.177Z
Learning: 'dir' props with sync functionality do not expose a path to the sync directory directly. For files to be synced, they must be either: (1) written to `process.env.STASH_DIR` (`/tmp/__stash`), or (2) written to `/tmp` and have their file path returned from the `run` method as a string, `[filepath]`, `[_, filepath]`, `{ filePath }`, `{ filepath }`, or `{ path }`.
Applied to files:
components/shortcut/actions/create-story/create-story.mjs
📚 Learning: in the salesloft api integration (components/salesloft/salesloft.app.mjs), the _makerequest method r...
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#16954
File: components/salesloft/salesloft.app.mjs:14-23
Timestamp: 2025-06-04T17:52:05.780Z
Learning: In the Salesloft API integration (components/salesloft/salesloft.app.mjs), the _makeRequest method returns response.data which directly contains arrays for list endpoints like listPeople, listCadences, listUsers, and listAccounts. The propDefinitions correctly call .map() directly on these responses without needing to destructure a nested data property.
Applied to files:
components/shortcut/actions/create-story/create-story.mjs
⏰ 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: pnpm publish
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (12)
components/pdfless/pdfless.app.mjs (4)
1-1
: LGTM! Import change resolves ES module compatibility issue.The switch from
@pdfless/pdfless-js
SDK to@pipedream/platform
's axios addresses the ES module import failure mentioned in issue #17876.
25-33
: LGTM! Clean helper methods for API communication.The URL construction and header setup methods are well-implemented with proper authentication and content type handling.
34-48
: LGTM! Well-structured HTTP request wrapper methods.The
makeRequest
andpost
methods provide a clean abstraction over axios with proper context passing and argument forwarding.
49-60
: Confirm API endpoint versioningPlease ensure these resource paths are correctly prefixed with the API version (for example,
/v1
) by your HTTP client configuration—otherwise the requests won’t match the Pdfless API docs:• listTemplates (lines 49–53): GET
/v1/document-templates
• generate (lines 54–60): POST/v1/pdfs
No code changes required here if your client automatically adds
/v1
to thepath
values; otherwise, please update thepath
strings accordingly.components/shortcut/common/utils.mjs (1)
42-85
: LGTM! ES module conversion is correct.The conversion from CommonJS to ES modules is properly implemented with
export default
and theparseJson
function is correctly added to the exported utilities object.components/shortcut/actions/create-story/create-story.mjs (7)
1-6
: LGTM! ES module conversion is correct.The import statements are properly converted to ES module syntax with correct
.mjs
extensions and the lodash import change fromlodash/get
to fulllodash
is appropriate.
11-11
: Version bump to 1.0.0 is appropriate.The major version bump reflects the significant migration from CommonJS to ES modules, which is a breaking change.
15-20
: Good refactoring to use propDefinition.Using
propDefinition
forprojectId
improves consistency and maintainability by centralizing the prop definition in the app module.
21-25
: LGTM! Name prop definition is clear.The explicit
name
prop definition improves code clarity and follows good practices.
72-75
: LGTM! lodash.get usage is correct.The updates to use
lodash.get
instead of the destructuredget
function are consistent with the import change and maintain the same functionality.Also applies to: 114-117, 145-148, 175-178, 249-252, 257-260
266-266
: LGTM! Direct number value is more appropriate.Using
state.id
directly instead of string interpolation${state.id}
is more appropriate for integer values and avoids unnecessary type conversion.
327-358
: parseJson usage in create-story conditional spreads is safeThe
parseJson
incomponents/shortcut/common/utils.mjs
correctly handles both object and JSON-string inputs by:
- Returning a deep-walked object when given an object.
- Parsing valid JSON strings into objects (and falling back to the original value on parse errors).
This matches its use throughout the codebase (e.g., metadata, filters, tasks in other actions). No changes are needed here.
4170ea9
to
79d27c1
Compare
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: 1
🔭 Outside diff range comments (1)
components/shortcut/actions/create-story/create-story.mjs (1)
282-282
: Fix typo in validation constraint.Line 282 has "inkedFileIds" which should be "linkedFileIds" to match the property being validated.
- inkedFileIds: utils.validateArrayWhenPresent, + linkedFileIds: utils.validateArrayWhenPresent,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (32)
components/google_tag_manager/package.json
(2 hunks)components/netlify/actions/get-site/get-site.mjs
(2 hunks)components/netlify/actions/list-files/list-files.mjs
(2 hunks)components/netlify/actions/list-site-deploys/list-site-deploys.mjs
(1 hunks)components/netlify/actions/rollback-deploy/rollback-deploy.mjs
(2 hunks)components/netlify/package.json
(2 hunks)components/netlify/sources/new-deploy-failure/new-deploy-failure.mjs
(1 hunks)components/netlify/sources/new-deploy-start/new-deploy-start.mjs
(1 hunks)components/netlify/sources/new-deploy-success/new-deploy-success.mjs
(1 hunks)components/netlify/sources/new-form-submission/new-form-submission.mjs
(1 hunks)components/pdfless/actions/generate-pdf/generate-pdf.mjs
(2 hunks)components/pdfless/package.json
(2 hunks)components/pdfless/pdfless.app.mjs
(2 hunks)components/playwright/actions/get-page-html/get-page-html.mjs
(1 hunks)components/playwright/actions/get-page-title/get-page-title.mjs
(1 hunks)components/playwright/actions/page-pdf/page-pdf.mjs
(1 hunks)components/playwright/actions/take-screenshot/take-screenshot.mjs
(1 hunks)components/playwright/package.json
(2 hunks)components/playwright/playwright.app.mjs
(1 hunks)components/puppeteer/actions/get-html/get-html.mjs
(1 hunks)components/puppeteer/actions/get-page-title/get-page-title.mjs
(1 hunks)components/puppeteer/actions/get-pdf/get-pdf.mjs
(3 hunks)components/puppeteer/actions/screenshot-page/screenshot-page.mjs
(4 hunks)components/puppeteer/package.json
(2 hunks)components/puppeteer/puppeteer.app.mjs
(1 hunks)components/shortcut/actions/create-story/create-story.mjs
(6 hunks)components/shortcut/actions/search-stories/search-stories.mjs
(1 hunks)components/shortcut/common/constants.mjs
(1 hunks)components/shortcut/common/utils.mjs
(2 hunks)components/shortcut/package.json
(2 hunks)components/shortcut/shortcut.app.mjs
(5 hunks)components/zoho_people/package.json
(2 hunks)
✅ Files skipped from review due to trivial changes (11)
- components/playwright/actions/take-screenshot/take-screenshot.mjs
- components/puppeteer/puppeteer.app.mjs
- components/playwright/actions/get-page-html/get-page-html.mjs
- components/puppeteer/package.json
- components/shortcut/actions/search-stories/search-stories.mjs
- components/netlify/actions/list-site-deploys/list-site-deploys.mjs
- components/playwright/actions/get-page-title/get-page-title.mjs
- components/google_tag_manager/package.json
- components/netlify/actions/list-files/list-files.mjs
- components/netlify/package.json
- components/playwright/playwright.app.mjs
🚧 Files skipped from review as they are similar to previous changes (19)
- components/playwright/package.json
- components/puppeteer/actions/get-html/get-html.mjs
- components/netlify/actions/get-site/get-site.mjs
- components/shortcut/common/constants.mjs
- components/pdfless/package.json
- components/netlify/sources/new-deploy-success/new-deploy-success.mjs
- components/netlify/actions/rollback-deploy/rollback-deploy.mjs
- components/shortcut/common/utils.mjs
- components/pdfless/actions/generate-pdf/generate-pdf.mjs
- components/shortcut/shortcut.app.mjs
- components/netlify/sources/new-form-submission/new-form-submission.mjs
- components/puppeteer/actions/get-page-title/get-page-title.mjs
- components/playwright/actions/page-pdf/page-pdf.mjs
- components/shortcut/package.json
- components/netlify/sources/new-deploy-start/new-deploy-start.mjs
- components/puppeteer/actions/get-pdf/get-pdf.mjs
- components/netlify/sources/new-deploy-failure/new-deploy-failure.mjs
- components/puppeteer/actions/screenshot-page/screenshot-page.mjs
- components/zoho_people/package.json
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: in `components/gainsight_px/actions/create-account/create-account.mjs`, the action name should be "c...
Learnt from: jcortes
PR: PipedreamHQ/pipedream#14467
File: components/gainsight_px/actions/create-account/create-account.mjs:4-6
Timestamp: 2024-10-30T15:24:39.294Z
Learning: In `components/gainsight_px/actions/create-account/create-account.mjs`, the action name should be "Create Account" instead of "Create Memory".
Applied to files:
components/shortcut/actions/create-story/create-story.mjs
📚 Learning: for "dir" props in pipedream components, whether to mark them as optional depends on the action's fi...
Learnt from: js07
PR: PipedreamHQ/pipedream#17375
File: components/zerobounce/actions/get-validation-results-file/get-validation-results-file.mjs:23-27
Timestamp: 2025-07-01T17:07:48.193Z
Learning: For "dir" props in Pipedream components, whether to mark them as optional depends on the action's file I/O behavior - if an action always writes files as output, the "dir" prop should not be marked as optional.
Applied to files:
components/shortcut/actions/create-story/create-story.mjs
📚 Learning: 'dir' props with sync functionality do not expose a path to the sync directory directly. for files t...
Learnt from: js07
PR: PipedreamHQ/pipedream#17375
File: components/tinypng/actions/convert-image/convert-image.mjs:33-37
Timestamp: 2025-07-01T16:56:20.177Z
Learning: 'dir' props with sync functionality do not expose a path to the sync directory directly. For files to be synced, they must be either: (1) written to `process.env.STASH_DIR` (`/tmp/__stash`), or (2) written to `/tmp` and have their file path returned from the `run` method as a string, `[filepath]`, `[_, filepath]`, `{ filePath }`, `{ filepath }`, or `{ path }`.
Applied to files:
components/shortcut/actions/create-story/create-story.mjs
📚 Learning: in the salesloft api integration (components/salesloft/salesloft.app.mjs), the _makerequest method r...
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#16954
File: components/salesloft/salesloft.app.mjs:14-23
Timestamp: 2025-06-04T17:52:05.780Z
Learning: In the Salesloft API integration (components/salesloft/salesloft.app.mjs), the _makeRequest method returns response.data which directly contains arrays for list endpoints like listPeople, listCadences, listUsers, and listAccounts. The propDefinitions correctly call .map() directly on these responses without needing to destructure a nested data property.
Applied to files:
components/shortcut/actions/create-story/create-story.mjs
⏰ 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: Lint Code Base
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
- GitHub Check: pnpm publish
🔇 Additional comments (10)
components/pdfless/pdfless.app.mjs (3)
1-1
: LGTM! Import change addresses the core issue.Replacing the problematic
@pdfless/pdfless-js
SDK with@pipedream/platform
's axios client directly addresses the ES module import failures mentioned in issue #17876.
25-48
: LGTM! Clean HTTP request abstraction.The helper methods implement a well-structured pattern for API communication:
- Clear separation of concerns
- Proper authentication handling
- Flexible parameter passing with spread operators
- Good abstraction for common HTTP operations
49-60
: LGTM! Clean API method implementations.Both methods follow the established helper pattern consistently and provide clear abstractions for the core PDF operations. The endpoint paths and HTTP methods are appropriate for their respective functionalities.
components/shortcut/actions/create-story/create-story.mjs (7)
1-5
: LGTM! ES module imports are correctly structured.The migration from CommonJS to ES modules is properly implemented with correct .mjs extensions and import syntax.
7-11
: LGTM! Proper ES module export and appropriate version bump.The major version bump to 1.0.0 is appropriate given the CommonJS to ES module migration.
15-44
: LGTM! WorkflowStateId prop improvements are well-implemented.The changes properly convert to required prop, use lodash.get consistently, and maintain correct type handling with numeric IDs.
45-49
: LGTM! Name prop addition is properly structured.The required name prop follows Pipedream conventions with explicit type, label, and description.
50-55
: LGTM! Archived prop change improves flexibility.Changing from default value to optional is better practice, allowing the API to handle the default behavior.
96-99
: LGTM! Consistent lodash.get usage across async options methods.All async options methods properly use lodash.get with correct array syntax for nested property access, maintaining existing functionality.
Also applies to: 138-141, 169-172, 199-202
319-350
: LGTM! Improved payload construction with modern JavaScript patterns.The conditional spread syntax with utils.parseJson is cleaner and more maintainable than the previous approach.
79d27c1
to
844561e
Compare
844561e
to
8c297e7
Compare
/approve |
WHY
Resolves #17876
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores