Add --headful flag to wrangler dev for browser rendering#13011
Add --headful flag to wrangler dev for browser rendering#13011ruifigueira wants to merge 1 commit intomainfrom
Conversation
🦋 Changeset detectedLatest commit: 82b08a0 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
✅ All changesets look good |
There was a problem hiding this comment.
I'm Bonk, and I've done a quick review of your PR.
This PR adds a --headful flag to wrangler dev that propagates through the config pipeline to make Miniflare launch Chrome in visible (non-headless) mode for browser rendering.
Issues (ranked by severity):
-
Missing
--headful --remotevalidation — The--headfulflag only affects local browser launching. If a user passeswrangler dev --headful --remote, the flag is silently ignored. The existing pattern invalidateArgs(line 274) validates this for--live-reloadand throws a helpfulUserError. The same guard should be added for--headful. See suggestion ondev.tsbelow. -
No tests added — The PR template checks "Tests included/updated" but no test files were modified. At minimum, the
headfulschema acceptance should be tested (e.g. verifyingBrowserRenderingSchemaparses{ headful: true }correctly), and the conditional Chrome args inlaunchBrowsershould be covered.
Suggestion for issue 1 — In packages/wrangler/src/dev.ts, add the following validation in validateArgs after the liveReload check (after line 278):
if (args.headful && args.remote) {
throw new UserError(
"--headful is only supported in local mode. Please just use one of either --remote or --headful."
);
}| describe: | ||
| "Open the browser in headful mode when using the Browser Rendering API in local mode", | ||
| type: "boolean", | ||
| }, |
There was a problem hiding this comment.
The --headful flag only takes effect in local mode. Consider also adding a validation guard in validateArgs (after the liveReload && remote check at line 274) to throw a UserError when --headful is combined with --remote, matching the pattern used for --live-reload:
if (args.headful && args.remote) {
throw new UserError(
"--headful is only supported in local mode. Please just use one of either --remote or --headful."
);
}|
The review has been posted successfully on PR #13011. Here's a summary of what I found: Two actionable issues identified:
The rest of the implementation (data flow through 9 files, changeset format, types, schema) looks correct and well-structured. |
7ade82b to
2d6da62
Compare
2d6da62 to
82b08a0
Compare
Fixes BRAPI-1010
Add
--headfulflag towrangler devto launch Chrome in headful (visible) mode when using the Browser Rendering API locally. This is useful for debugging browser automation scripts during development.A picture of a cute animal (not mandatory, but encouraged)