Skip to content

fix(types): document inline options#41860

Closed
pavelfeldman wants to merge 1 commit into
microsoft:mainfrom
pavelfeldman:fix-inline-options-docs
Closed

fix(types): document inline options#41860
pavelfeldman wants to merge 1 commit into
microsoft:mainfrom
pavelfeldman:fix-inline-options-docs

Conversation

@pavelfeldman

Copy link
Copy Markdown
Member

Summary

  • emit dotted @param options.<property> documentation from API docs
  • include nested option property descriptions in generated declarations

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 35f115a2-a27a-4abf-9568-d6fbd5fd46e7
@github-actions

Copy link
Copy Markdown
Contributor

Test results for "MCP"

1 failed
❌ [webkit] › mcp/annotate.spec.ts:230 › should capture annotations via show --annotate @mcp-macos-latest-webkit

7759 passed, 1249 skipped


Merge workflow run.

@github-actions

Copy link
Copy Markdown
Contributor

Test results for "tests 1"

7 flaky ⚠️ [chromium-library] › library/video.spec.ts:294 › screencast › should capture navigation `@frozen-time-library-chromium-linux`
⚠️ [chromium-library] › library/video.spec.ts:736 › screencast › should work with video+trace `@frozen-time-library-chromium-linux`
⚠️ [chromium-library] › library/video.spec.ts:294 › screencast › should capture navigation `@chromium-ubuntu-22.04-arm-node20`
⚠️ [chromium-library] › library/video.spec.ts:664 › screencast › should capture full viewport `@realtime-time-library-chromium-linux`
⚠️ [chromium-library] › library/video.spec.ts:495 › screencast › should capture static page in persistent context @smoke `@chromium-ubuntu-22.04-node20`
⚠️ [chromium-library] › library/video.spec.ts:699 › screencast › should capture full viewport on hidpi `@chromium-ubuntu-22.04-node22`
⚠️ [playwright-test] › ui-mode-trace.spec.ts:778 › should partition action tree state by test `@windows-latest-node22`

49956 passed, 1186 skipped


Merge workflow run.

@github-actions

Copy link
Copy Markdown
Contributor

🟢 CI is clear — the one failure is a pre-existing flake

Hi, I'm the Playwright bot and I took a look at the failing CI.

The only failure is [webkit] › mcp/annotate.spec.ts:230 › should capture annotations via show --annotate. This PR only regenerates type declarations (utils/generate_types/index.js and the generated types.d.ts / test.d.ts files) — it doesn't touch the MCP annotate code path — and the test is a long-standing cross-browser flake, so this run is not caused by the PR.

Details

Pre-existing flake / infra

  • [webkit] › mcp/annotate.spec.ts:230 › should capture annotations via show --annotate — pre-existing flake, not caused by this PR. In the aggregated CI results DB this test flips verdict across runs on SHAs and PRs unrelated to this one: it has failed as a final verdict on firefox (8), webkit (7), chrome (5), chromium (5), and been rescued by a retry dozens more times (firefox 21, chromium 13, webkit 12, chrome 11). It fails on every engine, on pushes to main and on unrelated PRs alike. This PR is a types/docs-only change (four files: the type generator plus the generated .d.ts outputs), so it can't reach the MCP show --annotate flow.

Triaged by the Playwright bot - agent run

@@ -133,6 +133,11 @@ export interface Page {
* @param arg Optional argument to pass to
* [`pageFunction`](https://playwright.dev/docs/api/class-page#page-evaluate-option-expression).
* @param options
* @param options.exposeFunctions When set to `true`, functions passed inside

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we instead document individual fields inline (same as e.g. goto does) ?

* without any content or with `display:none` has an empty bounding box and is not considered visible.
* - `'hidden'` - wait for element to be either detached from DOM, or have an empty bounding box or
* `visibility:hidden`. This is opposite to the `'visible'` option.
* @param options.strict When true, the call requires selector to resolve to a single element. If given selector resolves to more than one

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these options are already documented. Here is my VSCode knowing about them:

Image

@@ -969,6 +1119,12 @@ export interface Page {
*
* @param type
* @param options
* @param options.behavior Specifies whether to wait for already running listeners and what to do if they throw errors:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here - options.behavior is actually documented 10 lines below with the same comment.

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.

3 participants