Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletions .claude/skills/playwright-roll/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -174,9 +174,9 @@ Implement the change in `playwright/_impl/<module>.py`. Use the upstream JS impl
|---|---|
| `async foo(): Promise<X>` | `async def foo(self) -> X:` |
| `foo(): X` (sync getter, no args, no body) | `@property def foo(self) -> X:` (the generator makes argument-less sync members properties — see `isPythonProperty` in `scripts/codegen/generate_api.js` — and reads `self._impl_obj.foo` without calling it. mypy does not catch a plain method here; the test does.) |
| `await this._channel.foo({ a, b })` | `await self._channel.send("foo", None, locals_to_params(locals()))` |
| `(await this._channel.foo()).value` | `await self._channel.send("foo", None)` (Python's `send()` auto-unwraps single-key responses; only call `send_return_as_dict` when the protocol returns multiple keys.) |
| `(await this._channel.foo()).artifact` (multi-key, may be empty) | `result = await self._channel.send_return_as_dict("foo", None); (result or {}).get("artifact")` — `send_return_as_dict` returns **`None`** (not `{}`) when the protocol response carries no fields. |
| `await this._channel.foo({ a, b }, this._timeout(options))` | `await self._channel.foo(a=a, b=b, timeout=self._timeout(timeout))`. The generated channel method takes every protocol parameter, with no defaults; an absent option is `None`. The implementation method's own parameters use the Python (snake_case) names, with no defaults. |
| `(await this._channel.foo()).value` | `(await self._channel.foo())["value"]`. The result dict has every top-level field; a field that the driver left out is `None`. |
| `this._channel.on('foo', ({ a }) => ...)` | `self._channel.onFoo(lambda event: ...)`; `event["a"]` is `None` when the driver left the optional field out. |
| `try { ... } catch (e) { if (isTargetClosedError(e)) return; throw e; }` | `try: ...; except Exception as e: if is_target_closed_error(e): return; raise` (import from `playwright._impl._errors`) |
| Inline `[Object]` return like `{endpoint: string}` | A `TypedDict` in `playwright/_impl/_api_structures.py`, which the generator writes. It names the struct from the docs: `` "Name" <[Object]> ``, or an `* alias-python:` line under it. See `RemoteAddr`, `BrowserBindResult`, `DebuggerPausedDetails`. |
| `binary` event/return field | The Python channel layer hands you a base64 string. Decode with `base64.b64decode(value)` before exposing as `bytes`. See `Screencast._dispatch_frame`. |
Expand Down Expand Up @@ -216,7 +216,7 @@ The generator fails with "the entry … no longer matches anything; remove it" w
**Do not suppress** these — they're PORTs in disguise:

- "Internal tooling" classes/methods whose `langs` field is empty (`Screencast.*`, `Debugger.*`, `Page.pick_locator`, `BrowserContext.debugger`, `Browser.bind/unbind`, `Page.{clear,console}_*`). The 1.59 roll suppressed all of these initially, then had to walk every one back. Verify `langs` first.
- A renamed positional parameter (mypy: unexpected keyword argument `newName` for the impl method). Just rename in `_impl/` and update the channel-send dict key.
- A renamed positional parameter (mypy: unexpected keyword argument `newName` for the impl method). Just rename in `_impl/`; if the wire name changed too, mypy reports the generated channel call.
- A struct return the generator cannot name ("No Python name for the struct at …"). Name it, so that the generator writes a TypedDict — don't type it `Dict`.

#### N/A
Expand All @@ -241,7 +241,7 @@ The script, for `sync` and then `async`:
1. runs `node scripts/codegen/generate_api.js --flavor <flavor>` (reads the docs from `$PW_SRC_DIR`) straight into `playwright/<flavor>_api/_generated.py`,
2. invokes `pre-commit run --files` on the generated file.

It then runs `python scripts/codegen/generate_protocol.py`, which reads `packages/protocol/spec/*.yml` from `$PW_SRC_DIR` and writes the command parameter tables to `playwright/_impl/_protocol.py`.
It then runs `python scripts/codegen/generate_protocol.py`, which reads `packages/protocol/spec/*.yml` from `$PW_SRC_DIR` and writes the command parameter tables to `playwright/_impl/_protocol.py`, the message types to `playwright/_impl/_protocol_types.py` and the typed channels to `playwright/_impl/_protocol_channels.py`.

Then run `mypy playwright` and work through the errors in `_generated.py` — each one is a call into `_impl/` that the implementation does not support yet.

Expand All @@ -261,7 +261,8 @@ Failure modes and fixes:
| Symptom | Cause | Fix |
|---|---|---|
| mypy: `"XImpl" has no attribute "y"` in `_generated.py` | The docs have a new member `X.y`; `_impl/` does not. | PORT it. |
| mypy: `Unexpected keyword argument "z" for "y" of "XImpl"` | New parameter or option on an existing method. | Add the kwarg (docs camelCase name) in `_impl/`. If Python must not have it, fix the docs: `* langs:` on the option. |
| mypy: `Unexpected keyword argument "z" for "y" of "XImpl"` | New parameter or option on an existing method. | Add the parameter in `_impl/` under its Python (snake_case) name, as `Optional[X]` with no default, and pass it in the channel call. mypy then reports each caller that must pass it. If Python must not have it, fix the docs: `* langs:` on the option. |
| mypy: `Missing named argument "z" for "y" of "XChannel"` | The protocol added a parameter to a command. | Pass the value in the channel call. If the implementation method needs a new parameter for it, add it as above. |
| mypy: incompatible argument type in `_generated.py` | The docs type changed, or `_impl/` declares a narrower type. | Match the type in `_impl/`, or fix the docs with a Python-only declaration (`* langs: python`). |
| generator: `No Python name for the struct at X.y.z` | A new inline `[Object]` with no name in the docs. | Name it in the docs: `` "Name" <[Object]> ``. |
| generator: `The struct X has different fields at A and B` | Two places in the docs share one struct name, but their fields differ. | Give one place its own name in the docs. |
Expand Down Expand Up @@ -328,7 +329,7 @@ All keys use docs names: `Class.member` or `Class.member.arg`, with the upstream
- **A cluster of rules on the same class is a smell.** If you're about to add five entries for `Foo.*`, you're almost certainly looking at a class that needs to be implemented. Implement the whole thing once and the entries disappear.
- **Watch for revert pairs in the same range.** 1.59 added and reverted `Browser.isRemote` (#39613 / #39620) inside the same release. Walking chronologically lets you skip the add when you see the revert later.
- **Watch for rename-revert pairs.** 1.59 had `Locator.normalize` → `Locator.toCode` (#39648) → `Locator.normalize` (#39754). Final state wins; only port the last.
- **Doc renames almost always include a wire-protocol rename.** Whenever you see `### param: X.y.oldName` → `### param: X.y.newName` in a doc commit, also `git -C "$PW_SRC_DIR" show <sha> -- packages/protocol/spec/` and the corresponding `*Dispatcher.ts` file. If the wire field changed too, the channel-send dict key in `_impl/` must change. Suppressing the doc-side mismatch is hiding a real bug — the previous Python code is silently sending an unknown field that the new server ignores. The test suite reports such a field as a protocol violation, but only in tests that send it.
- **Doc renames almost always include a wire-protocol rename.** Whenever you see `### param: X.y.oldName` → `### param: X.y.newName` in a doc commit, also `git -C "$PW_SRC_DIR" show <sha> -- packages/protocol/spec/` and the corresponding `*Dispatcher.ts` file. If the wire field changed too, the regenerated `_protocol_channels.py` renames the argument of the channel method, and mypy reports each call in `_impl/` that passes the old name.
- **TypedDicts beat `Dict[str, X]` for any structured return.** When the docs describe a return as `[Object]` with named fields, name the struct for the generator, which writes its `TypedDict` into `_api_structures.py`, and re-export it from both public `__init__.py` files. Zero runtime cost (it's still a `dict`), and the docstrings expand its fields from the docs.
- **Positional renames are free.** A param with no default before any `*` separator is positional-or-keyword in Python, but realistic call sites pass it positionally. Renaming such a param doesn't break callers.
- **The "Backport changes" GitHub issue can be misleading.** In the 1.59 roll its checkboxes were all marked `[x]` with annotations like "✅ IMPLEMENTED", but several of those features had not actually been merged into the Python port. Trust the `docs/src/api/` walk over the issue.
Expand Down
8 changes: 6 additions & 2 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@ Python bindings for [Playwright](https://playwright.dev). The Python client talk
- `playwright/async_api/_generated.py`, `playwright/sync_api/_generated.py`, and the public TypedDicts in `playwright/_impl/_api_structures.py` — **auto-generated** from the upstream API docs. Never edit by hand; rerun `./scripts/update_api.sh` after a roll or after changing `scripts/codegen/`.
- `scripts/codegen/generate_api.js` — the generator. A Node.js script that loads the upstream doc parser (`utils/doclint/api_parser.js`) from `$PW_SRC_DIR`, filters the docs for Python, and writes the whole public API: signatures, types, docstrings, event overloads, the calls into `_impl/`, and the TypedDicts. It does not read `_impl/`.
- `scripts/codegen/python_rules.js` — the Python facts that the docs do not contain (positional options, struct `total=`, hidden members and options). Policy: the shipped Python API wins; each entry is a candidate for an upstream docs fix, and the generator fails on entries that no longer match anything. Every entry needs a one-line rationale comment above it.
- `playwright/_impl/_protocol.py` — **auto-generated** by `scripts/codegen/generate_protocol.py` (run by `./scripts/update_api.sh`) from the upstream protocol spec: the parameters of every command. With `PW_CHECK_PROTOCOL=1`, `playwright/_impl/_protocol_check.py` checks each message that the client sends against it. The test suite sets this, and fails a test that sends an unknown command, an unknown or missing parameter, or a value of the wrong type.
- `playwright/_impl/_protocol.py`, `_protocol_types.py` and `_protocol_channels.py` — **auto-generated** by `scripts/codegen/generate_protocol.py` (run by `./scripts/update_api.sh`) from the upstream protocol spec.
- `_protocol_channels.py` has a typed channel class for each interface (`PageChannel`, `FrameChannel`, …), like upstream's `channels.d.ts`. Each command is an async method with the protocol name and parameters (`await self._channel.goto(url=..., waitUntil=..., referer=..., timeout=...)`); each event has `on<Event>` / `off<Event>` methods. A result or event payload has every top-level field; a field that the driver left out is `None`.
- `_protocol_types.py` has the TypedDicts for initializers, results, events and nested types, named as upstream's `utils/generate_channels.js` names them (`PageInitializer`, `FrameGotoResult`, `PageRouteEvent`, …).
- `_protocol.py` has the parameters of every command. With `PW_CHECK_PROTOCOL=1`, `playwright/_impl/_protocol_check.py` checks each message that the client sends against it. The test suite sets this, and fails a test that sends an unknown command, an unknown or missing parameter, or a value of the wrong type.
- `scripts/codegen/compare_api.py` — compares two generated files (signatures, annotations, call bodies, docstrings) and lists every difference. Use it to check that a generator change does not change the API.
- `tests/async/`, `tests/sync/` — pytest suites, ported from the upstream tests. Most new tests are added to the async file with a sync mirror. See [Tests](#tests).
- `DRIVER_VERSION` — the single source of truth for which Playwright release the driver is assembled from (one line, the `playwright-core` npm version, e.g. `1.61.0`, no `v` prefix). Read by `setup.py`, `scripts/build_driver.py`, and CI. The wheel build downloads `playwright-core` at this version from npm plus the matching Node.js binary and assembles the per-platform bundles — no source build. The version is baked into the staged bundle filenames (`driver/playwright-<version>-<suffix>.zip`), so it doubles as the build cache key.
Expand Down Expand Up @@ -80,7 +83,8 @@ It documents the full process: the upstream commit-range diff over `docs/src/api
- Don't add `# type: ignore` or modify `_generated.py` to silence pyright; fix the source of the mismatch.
- New public methods on impl classes need a sync test mirror under `tests/sync/`.
- Keep `scripts/codegen/python_rules.js` minimal — every entry needs a one-line rationale comment above it. Prefer fixing the upstream docs.
- Prefer `locals_to_params(locals())` for forwarding optional kwargs to channel sends, matching the rest of the codebase.
- Implementation methods take the Python (snake_case) names that the generated API passes. Their options have no default: an option that can be absent is `Optional[X]` without `= None`. So mypy reports each caller that does not pass a new option.
- Send messages through the generated channel methods, never by name: `await self._channel.click(selector=selector, ..., clickCount=click_count, timeout=self._timeout(timeout))`. Pass every parameter, with the protocol (camelCase) name; an absent option is `None`, and the connection drops it. Declare the channel type on each ChannelOwner subclass (`_channel: "PageChannel"`). Listen to events with `self._channel.on<Event>(handler)`.

## Commit Convention

Expand Down
45 changes: 12 additions & 33 deletions playwright/_impl/_artifact.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,20 @@

import pathlib
from pathlib import Path
from typing import Dict, Optional, Union, cast
from typing import TYPE_CHECKING, Dict, Optional, Union, cast

from playwright._impl._connection import ChannelOwner, from_channel
from playwright._impl._errors import Error, patch_error_message
from playwright._impl._helper import make_dirs_for_file
from playwright._impl._stream import Stream

if TYPE_CHECKING: # pragma: no cover
from playwright._impl._protocol_channels import ArtifactChannel


class Artifact(ChannelOwner):
_channel: "ArtifactChannel"

def __init__(
self, parent: ChannelOwner, type: str, guid: str, initializer: Dict
) -> None:
Expand All @@ -34,55 +39,29 @@ async def path_after_finished(self) -> pathlib.Path:
raise Error(
"Path is not available when using browser_type.connect(). Use save_as() to save a local copy."
)
path = await self._channel.send(
"pathAfterFinished",
None,
)
path = (await self._channel.pathAfterFinished())["value"]
return pathlib.Path(path)

async def save_as(self, path: Union[str, Path]) -> None:
stream = cast(
Stream,
from_channel(
await self._channel.send(
"saveAsStream",
None,
)
),
Stream, from_channel((await self._channel.saveAsStream())["stream"])
)
make_dirs_for_file(path)
await stream.save_as(path)

async def failure(self) -> Optional[str]:
reason = await self._channel.send(
"failure",
None,
)
reason = (await self._channel.failure())["error"]
if reason is None:
return None
return patch_error_message(reason)

async def delete(self) -> None:
await self._channel.send(
"delete",
None,
)
await self._channel.delete()

async def read_info_buffer(self) -> bytes:
stream = cast(
Stream,
from_channel(
await self._channel.send(
"stream",
None,
)
),
)
stream = cast(Stream, from_channel((await self._channel.stream())["stream"]))
buffer = await stream.read_all()
return buffer

async def cancel(self) -> None: # pyright: ignore[reportIncompatibleMethodOverride]
await self._channel.send(
"cancel",
None,
)
await self._channel.cancel()
Loading
Loading