CustomJS: Add Run Puppoteer and HTML to PNG actions #14117
Conversation
|
@hamadawan 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
|
WalkthroughThe pull request introduces new functionalities to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
|
Thank you so much for submitting this! We've added it to our backlog to review, and our team has been notified. |
|
Thanks for submitting this PR! When we review PRs, we follow the Pipedream component guidelines. If you're not familiar, here's a quick checklist:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Outside diff range and nitpick comments (2)
components/customjs/actions/convert-html-to-png/convert-html-to-png.mjs (1)
5-24: LGTM: Action definition is well-structured. Consider making the filename prop optional.The action definition follows the guidelines mentioned in the PR objectives:
- The key format is correct:
customjs-convert-html-to-png.- The description includes a link to the documentation.
- The version is set appropriately for a new component.
The props are well-defined, but consider the following suggestion:
Make the
filenameprop optional with a default value. This allows users to run the action without specifying a filename, which could default to a timestamp-based name. Here's a suggested modification:filename: { propDefinition: [ customjs, "filename", ], optional: true, default: () => `html_to_png_${Date.now()}.png`, },This change aligns with the PR objective of using optional props with default values where applicable.
components/customjs/customjs.app.mjs (1)
Line range hint
1-73: Suggestions for code organization and documentationWhile the individual changes look good and align with the PR objectives, consider the following suggestions to improve code organization and documentation:
Add JSDoc comments for each method to describe their functionality, parameters, and return values. This aligns with the PR objectives mentioned in the comments summary.
Consider extracting the
customjs-originheader values into constants at the top of the file for easier maintenance and consistency.If these methods are intended to be used externally, consider adding them to the
propDefinitionsobject with appropriate descriptions and labels.Here's an example of how you could implement these suggestions:
import { axios } from "@pipedream/platform"; const ORIGINS = { PDF: "pipedream/generatePDF", PNG: "pipedream/generatePNG", SCREENSHOT: "pipedream/screenshot", MERGE_PDFS: "pipedream/mergePDFs", PUPPETEER: "pipedream/puppeteer", }; export default { type: "app", app: "customjs", propDefinitions: { filename: { type: "string", label: "Filename", description: "Download the file to the `/tmp` directory with the specified filename.", }, // Add other prop definitions here }, methods: { // ... existing _makeRequest method ... /** * Converts HTML to PDF * @param {Object} opts - Options for the conversion * @returns {Promise<ArrayBuffer>} The PDF file as an ArrayBuffer */ convertHtmlToPdf(opts = {}) { return this._makeRequest({ headers: { "customjs-origin": ORIGINS.PDF, }, ...opts, }); }, // Add similar JSDoc comments and use ORIGINS constant for other methods }, };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- .wordlist.txt (2 hunks)
- components/customjs/actions/convert-html-to-png/convert-html-to-png.mjs (1 hunks)
- components/customjs/actions/run-puppoteer/run-puppoteer.mjs (1 hunks)
- components/customjs/customjs.app.mjs (1 hunks)
🔇 Additional comments (6)
components/customjs/actions/run-puppoteer/run-puppoteer.mjs (1)
1-3: LGTM: Imports are appropriate and consistent.The imports from
customjs.app.mjs,fs, andutils.mjsare relevant to the action's functionality. The use of ES modules (.mjsextension) is consistent with modern JavaScript practices.components/customjs/actions/convert-html-to-png/convert-html-to-png.mjs (1)
1-3: LGTM: Imports are appropriate and use modern syntax.The imports are well-chosen for the action's requirements:
customjsfor the main functionalityfsfor file system operationsnormalizeFilepathutility for file path handlingThe use of ES6 import syntax is consistent with modern JavaScript practices.
components/customjs/customjs.app.mjs (4)
34-37: LGTM: Header update for convertHtmlToPdfThe
customjs-originheader has been correctly updated to "pipedream/generatePDF", which accurately reflects the Pipedream origin and the PDF generation functionality. This change aligns with the PR objectives of updating the CustomJS framework.
39-44: LGTM: New method convertHtmlToPng addedThe new
convertHtmlToPngmethod has been successfully added, following the same structure as other methods in the file. Thecustomjs-originheader is correctly set to "pipedream/generatePNG", which accurately reflects the Pipedream origin and the PNG generation functionality. This addition aligns with the PR objectives of introducing new actions to the CustomJS framework.
50-50: LGTM: Header update for createScreenshotThe
customjs-originheader has been correctly updated to "pipedream/screenshot", which accurately reflects the Pipedream origin and the screenshot functionality. This change aligns with the PR objectives of updating the CustomJS framework.
58-61: LGTM: Header update for mergePdfsThe
customjs-originheader has been correctly updated to "pipedream/mergePDFs", which accurately reflects the Pipedream origin and the PDF merging functionality. This change aligns with the PR objectives of updating the CustomJS framework.
components/customjs/actions/convert-html-to-png/convert-html-to-png.mjs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Thank you so much for your contribution! Looks good. You'll just need to update the versions of convert-html-to-pdf.mjs, create-screenshot.mjs, merge-pdfs.mjs, and package.json, remove a couple unnecessary lines (commented below), and then it should be ready to test.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- components/customjs/actions/run-puppeteer/run-puppeteer.mjs (1 hunks)
- components/customjs/customjs.app.mjs (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- components/customjs/customjs.app.mjs
🔇 Additional comments (2)
components/customjs/actions/run-puppeteer/run-puppeteer.mjs (2)
1-3: LGTM: Imports are appropriate and use modern syntax.The imports are well-structured and include necessary modules for the action's functionality.
1-44: Overall assessment: Good implementation with room for improvement.The "Run Puppeteer" action is well-structured and implements the required functionality. However, there are a few areas for improvement:
- Update the component key format to align with guidelines.
- Consider making the 'code' prop optional with a default value.
- Implement error handling in the run method.
- Use asynchronous file writing to avoid potential event loop blocking.
Addressing these points will enhance the robustness and efficiency of the action while aligning it more closely with Pipedream's component guidelines.
|
@michelle0927 I have resolved the comments, Kindly review and merge. Thank you |
|
Hello everyone, I have tested this PR and there're some test cases failed or needed improvement. Please check the test report below for more information |
|
@hamadawan I'm not able to get the "Run Puppeteer" action to run successfully. Can you provide an example of what should be entered in the "Code" prop? |
|
Hi, guys sorry for the inconvenience, Here is the CustomJS doc.
https://www.customjs.space/api/docs#_5-run-puppoteer
According to docs. we should pass this input:
{ "input": "const page = await browser.newPage(); await
page.goto(\"https://example.com\"); return await page.screenshot({
encoding: \"binary\" });", "code": "const { PUPPETEER } =
require(\"./utils\"); return PUPPETEER(input);", "returnBinary":
"true"}
…On Mon, Sep 30, 2024 at 8:49 PM michelle0927 ***@***.***> wrote:
@hamadawan <https://github.com/hamadawan> I'm not able to get the "Run
Puppeteer" action to run successfully. Can you provide an example of what
should be entered in the "Code" prop?
—
Reply to this email directly, view it on GitHub
<#14117 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALK6JK6RZKJQEUJICK3DDV3ZZFXHRAVCNFSM6AAAAABO5ZGAVKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGOBTGU3TEMBVGE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@hamadawan And that works for you when testing the action? For me, it was creating a blank .png file. Would you mind sharing screenshot(s) of the component working successfully for you and the resulting .png file? |
|
@michelle0927 I just rechecked; it is not working for me as well. But I found that we need to pass this in as input.
This will work. I'll update the CustomJS docs as well. |
|
@michelle0927 I just rechecked; it is not working for me as well. But I found that we need to pass this in as input.
This will work. |
|
Hi everyone, all test cases are passed! Ready for release! |
Add the following actions in CustomJS.
1: Run Puppeteer.
2: HTML to PNG.
Summary by CodeRabbit
Summary by CodeRabbit
New Features
CustomJSandPuppeteer.customjsmodule.Bug Fixes
Version Updates
@pipedream/customjsfrom0.1.0to0.2.0.