Skip to content

Conversation

@js07
Copy link
Collaborator

@js07 js07 commented Oct 10, 2025

Adds dir props to Playwright actions:

  • Playwright Page PDF
  • Playwright Take Screenshot

WHY

The dir prop indicates that an action works with files, typically read from or written to the ephemeral /tmp directory. When executing actions using the Connect service, you can use this indicator to determine when to pass the stashId parameter. When stashId is passed, files in /tmp are synced with File Stash at the start or end of execution so they are accessible in other action executions or outside of Pipedream's execution environment entirely.

Summary by CodeRabbit

  • New Features

    • Added a configurable sync directory option to the PDF export and Screenshot actions, enabling write access and automatic synchronization of generated files to a chosen folder.
  • Chores

    • Bumped versions of the PDF export and Screenshot actions to 0.0.5.

- Playwright Page PDF
- Playwright Take Screenshot
@vercel
Copy link

vercel bot commented Oct 10, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
pipedream-docs Ignored Ignored Oct 10, 2025 3:12am
pipedream-docs-redirect-do-not-edit Ignored Ignored Oct 10, 2025 3:12am

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 10, 2025

Walkthrough

Added a new public prop syncDir (type: dir, write, sync: true) to two Playwright action modules and bumped their versions from 0.0.4 to 0.0.5. No runtime logic changes noted beyond exposing the new prop in the public declarations.

Changes

Cohort / File(s) Summary
Playwright actions: add syncDir and bump version
components/playwright/actions/page-pdf/page-pdf.mjs, components/playwright/actions/take-screenshot/take-screenshot.mjs
Version 0.0.4 ➜ 0.0.5; added public prop syncDir with { type: "dir", accessMode: "write", sync: true } to props.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

A hop and a bump to zero-dot-five,
New syncDir burrows where outputs thrive.
Screens flicker, pages freeze—click!
Folders sync in a tidy tick.
Ears up, ship fast—thump-thump, we thrive! 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title clearly and concisely summarizes the main change by indicating that file synchronization is being configured for Playwright components, which accurately reflects the addition of the new dir props for syncing files.
Description Check ✅ Passed The description follows the repository template by including a filled “## WHY” section and provides a clear summary of the added dir props and their purpose, demonstrating why the change is needed for synchronizing files in Playwright actions.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch playwright-file-sync

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 13c7230 and 02d5688.

📒 Files selected for processing (2)
  • components/playwright/actions/page-pdf/page-pdf.mjs (2 hunks)
  • components/playwright/actions/take-screenshot/take-screenshot.mjs (2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
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.
🔇 Additional comments (4)
components/playwright/actions/page-pdf/page-pdf.mjs (2)

7-7: LGTM! Version bump is appropriate.

The version bump from 0.0.4 to 0.0.5 correctly reflects the addition of the new syncDir prop.


45-49: No label/description needed for syncDir
syncDir in page-pdf follows the established pattern—other actions omit labels/descriptions on this platform-level prop. Approve as-is.

components/playwright/actions/take-screenshot/take-screenshot.mjs (2)

7-7: LGTM! Version bump is appropriate.

The version bump from 0.0.4 to 0.0.5 correctly reflects the addition of the new syncDir prop, consistent with the parallel change in page-pdf.mjs.


56-60: Verify if user-facing label and description are needed.

The syncDir prop is correctly configured:

  • Not marked as optional (appropriate since this action always writes files)
  • accessMode: "write" matches the file I/O behavior
  • sync: true aligns with the PR objective to synchronize files with File Stash

However, unlike other props in this file (lines 16-55), syncDir lacks a label and description. This could be intentional if dir type props are platform-level metadata not exposed to users in the UI. The implementation is consistent with the parallel change in page-pdf.mjs.

The verification script from the page-pdf.mjs review will check patterns across both files.

Based on learnings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@js07 js07 marked this pull request as ready for review October 10, 2025 03:23
@js07 js07 merged commit f373886 into master Oct 10, 2025
9 of 10 checks passed
@js07 js07 deleted the playwright-file-sync branch October 10, 2025 03:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant