Skip to content

Conversation

@Skn0tt
Copy link
Member

@Skn0tt Skn0tt commented Jan 16, 2026

  • updates internal zod usage to zod v4, so we can leverage the built-in json schema converter and the prettifyErrors function
  • ensures external support for both zod 3 and zod 4
  • removes zod from utilsBundle so we don't ship it double

@Skn0tt Skn0tt requested a review from dgozman January 16, 2026 11:29
@Skn0tt Skn0tt self-assigned this Jan 16, 2026
@dgozman dgozman requested a review from yury-s January 16, 2026 11:39
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

"target": "ESNext",
"noEmit": true,
"moduleResolution": "node",
"allowSyntheticDefaultImports": true
Copy link
Member Author

Choose a reason for hiding this comment

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

zod v4 needs this set because its declaration files use syntax that depends on it. without it, there's a type checking error. Users that have zod v4 installed will already have this, and users without zod v4 will not need it, so this change is fine.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

"@lowire/loop": "^0.0.23",
"@modelcontextprotocol/sdk": "^1.25.2",
"zod": "^3.25.76",
"zod": "^4.3.5",
Copy link
Member

Choose a reason for hiding this comment

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

Please measure the bundle compilation time and the output size (e.g. ls -lh packages/playwright-core/lib/mcpBundleImpl/index.js) with old/new version. Rolling sdk and zod without any hack for sdk would leave in a bad state (see modelcontextprotocol/typescript-sdk#1313), let's make sure this doesn't regress because of the zod.

Copy link
Member Author

@Skn0tt Skn0tt Jan 16, 2026

Choose a reason for hiding this comment

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

old

886k

==== Running esbuild: packages/playwright-core/bundles/mcp/src/mcpBundleImpl.ts

Package contribution to bundle:
zod                            119.9 KB  29.35%
ajv                            102.0 KB  24.97%
@modelcontextprotocol/sdk      97.1 KB  23.77%
@lowire/loop                   23.5 KB  5.75%
zod-to-json-schema             20.3 KB  4.98%
ajv-formats                    9.9 KB  2.42%
fast-uri                       9.3 KB  2.27%
@hono/node-server              7.7 KB  1.88%
eventsource                    5.2 KB  1.27%
cross-spawn                    2.9 KB  0.70%
content-type                   1.9 KB  0.46%
eventsource-parser             1.8 KB  0.44%
isexe                          1.5 KB  0.37%
which                          1.4 KB  0.35%
json-schema-traverse           1.2 KB  0.30%
bytes                          1.0 KB  0.26%
pkce-challenge                 0.8 KB  0.20%
fast-deep-equal                0.7 KB  0.18%
path-key                       0.2 KB  0.06%
shebang-command                0.2 KB  0.05%
shebang-regex                  0.1 KB  0.01%
==== Done in 114 ms

new

666K

==== Running esbuild: packages/playwright-core/bundles/mcp/src/mcpBundleImpl.ts

Package contribution to bundle:
zod                            374.6 KB  56.46%
ajv                            102.2 KB  15.40%
@modelcontextprotocol/sdk      97.1 KB  14.64%
@lowire/loop                   23.5 KB  3.54%
zod-to-json-schema             20.4 KB  3.07%
ajv-formats                    9.9 KB  1.49%
fast-uri                       9.3 KB  1.40%
@hono/node-server              7.7 KB  1.16%
eventsource                    5.2 KB  0.79%
cross-spawn                    2.9 KB  0.43%
content-type                   1.9 KB  0.28%
eventsource-parser             1.8 KB  0.27%
isexe                          1.5 KB  0.23%
which                          1.4 KB  0.22%
json-schema-traverse           1.2 KB  0.18%
bytes                          1.0 KB  0.16%
pkce-challenge                 0.8 KB  0.12%
fast-deep-equal                0.7 KB  0.11%
path-key                       0.2 KB  0.03%
shebang-command                0.2 KB  0.03%
shebang-regex                  0.1 KB  0.01%
==== Done in 132 ms

Copy link
Member

Choose a reason for hiding this comment

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

The size is shrinking, yay!

Copy link
Member Author

Choose a reason for hiding this comment

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

i did something wrong when measuring. it now shows 411K on main, so the size actually grew

if (!tool)
throw new Error(`Tool "${name}" not found`);
const parsedArguments = tool.schema.inputSchema.parse(rawArguments || {});
const parsedArguments = tool.schema.inputSchema.parse(rawArguments || {}) as any;
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need this?

Copy link
Member Author

Choose a reason for hiding this comment

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

this used to be any, now it's unknown. we're casting it back so we can use it

});

test('retrieve a secret', async ({ context }) => {
// cannot get it to work reliably in CI
Copy link
Member

Choose a reason for hiding this comment

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

Should it be skip then or is it failing locally too?

But I'd rather we made it work.

Copy link
Member Author

@Skn0tt Skn0tt Jan 16, 2026

Choose a reason for hiding this comment

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

it's failing locally too, wrong comment. i'll make it work in a followup

import { ZodTypeAny, z } from 'zod';
// @ts-ignore this will be any if zod is not installed
type ZodInfer<T extends ZodTypeAny> = import('zod').infer<T>;
import * as z3 from 'zod/v3';
Copy link
Member

Choose a reason for hiding this comment

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

Is there zod/v3 in v3 zod?

Copy link
Member Author

@Skn0tt Skn0tt Jan 16, 2026

Choose a reason for hiding this comment

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

no, it'll be any. but the other one exists, so everything works :D

@github-actions

This comment has been minimized.

@github-actions
Copy link
Contributor

Test results for "MCP"

4 failed
❌ [chromium] › mcp/launch.spec.ts:21 › test reopen browser @mcp-macos-15
❌ [firefox] › mcp/test-debug.spec.ts:48 › test_debug (pause/resume) @mcp-macos-15
❌ [firefox] › mcp/test-debug.spec.ts:167 › test_debug (pause/snapshot/resume) @mcp-macos-15
❌ [firefox] › mcp/test-debug.spec.ts:234 › test_debug / evaluate x 2 @mcp-macos-15

2826 passed, 121 skipped


Merge workflow run.

@github-actions
Copy link
Contributor

Test results for "tests 1"

26 failed
❌ [chromium-library] › library/agent-expect.spec.ts:223 › expectURL success @ubuntu-22.04-chromium-tip-of-tree
❌ [chromium-library] › library/agent-expect.spec.ts:241 › expectURL wrong URL error @ubuntu-22.04-chromium-tip-of-tree
❌ [chromium-library] › library/agent-expect.spec.ts:261 › expectURL with regex @ubuntu-22.04-chromium-tip-of-tree
❌ [chromium-library] › library/agent-expect.spec.ts:279 › expectURL with regex error @ubuntu-22.04-chromium-tip-of-tree
❌ [chromium-library] › library/agent-expect.spec.ts:223 › expectURL success @chromium-ubuntu-22.04-node24
❌ [chromium-library] › library/agent-expect.spec.ts:241 › expectURL wrong URL error @chromium-ubuntu-22.04-node24
❌ [chromium-library] › library/agent-expect.spec.ts:261 › expectURL with regex @chromium-ubuntu-22.04-node24
❌ [chromium-library] › library/agent-expect.spec.ts:279 › expectURL with regex error @chromium-ubuntu-22.04-node24
❌ [chromium-library] › library/agent-expect.spec.ts:223 › expectURL success @chromium-ubuntu-22.04-node20
❌ [chromium-library] › library/agent-expect.spec.ts:241 › expectURL wrong URL error @chromium-ubuntu-22.04-node20
❌ [chromium-library] › library/agent-expect.spec.ts:261 › expectURL with regex @chromium-ubuntu-22.04-node20
❌ [chromium-library] › library/agent-expect.spec.ts:279 › expectURL with regex error @chromium-ubuntu-22.04-node20
❌ [chromium-library] › library/agent-expect.spec.ts:223 › expectURL success @chromium-ubuntu-22.04-node22
❌ [chromium-library] › library/agent-expect.spec.ts:241 › expectURL wrong URL error @chromium-ubuntu-22.04-node22
❌ [chromium-library] › library/agent-expect.spec.ts:261 › expectURL with regex @chromium-ubuntu-22.04-node22
❌ [chromium-library] › library/agent-expect.spec.ts:279 › expectURL with regex error @chromium-ubuntu-22.04-node22
❌ [firefox-library] › library/agent-expect.spec.ts:223 › expectURL success @firefox-ubuntu-22.04-node20
❌ [firefox-library] › library/agent-expect.spec.ts:241 › expectURL wrong URL error @firefox-ubuntu-22.04-node20
❌ [firefox-library] › library/agent-expect.spec.ts:261 › expectURL with regex @firefox-ubuntu-22.04-node20
❌ [firefox-library] › library/agent-expect.spec.ts:279 › expectURL with regex error @firefox-ubuntu-22.04-node20
❌ [webkit-library] › library/agent-expect.spec.ts:223 › expectURL success @webkit-ubuntu-22.04-node20
❌ [webkit-library] › library/agent-expect.spec.ts:241 › expectURL wrong URL error @webkit-ubuntu-22.04-node20
❌ [webkit-library] › library/agent-expect.spec.ts:261 › expectURL with regex @webkit-ubuntu-22.04-node20
❌ [webkit-library] › library/agent-expect.spec.ts:279 › expectURL with regex error @webkit-ubuntu-22.04-node20
❌ [playwright-test] › playwright.spec.ts:877 › page.pause() should disable test timeout @macos-latest-node20
❌ [playwright-test] › runner.spec.ts:124 › should ignore subprocess creation error because of SIGINT @macos-latest-node20

4 flaky ⚠️ [firefox-library] › library/inspector/cli-codegen-1.spec.ts:1082 › cli codegen › should not throw csp directive violation errors `@firefox-ubuntu-22.04-node20`
⚠️ [firefox-page] › page/page-wait-for-function.spec.ts:104 › should work with strict CSP policy `@firefox-ubuntu-22.04-node20`
⚠️ [playwright-test] › ui-mode-trace.spec.ts:688 › should indicate current test status `@macos-latest-node20`
⚠️ [playwright-test] › ui-mode-test-tree.spec.ts:407 › should collapse all `@windows-latest-node20`

34567 passed, 691 skipped


Merge workflow run.

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.

2 participants