Skip to content

chore(build): fix esbuild bundle warnings and externals#40155

Merged
pavelfeldman merged 3 commits intomicrosoft:mainfrom
pavelfeldman:fix-bundle-warnings
Apr 10, 2026
Merged

chore(build): fix esbuild bundle warnings and externals#40155
pavelfeldman merged 3 commits intomicrosoft:mainfrom
pavelfeldman:fix-bundle-warnings

Conversation

@pavelfeldman
Copy link
Copy Markdown
Member

Summary

  • Suppress direct-eval warnings for all bundled esbuild steps (Playwright uses eval in evaluate() callbacks stringified for the browser)
  • Add missing externals to bundle steps: 'playwright' for babelBundle, '../transform/esmLoader.js' for common/runner/worker bundles
  • Fix babelBundle.ts JSX importSource to use bare 'playwright' specifier instead of path.dirname(require.resolve('playwright'))
  • Remove unused WebSocketEventEmitter type alias in browserServerImpl.ts

- Suppress direct-eval warnings for all bundled steps (Playwright
  intentionally uses eval in evaluate() callbacks sent to browser)
- Add 'playwright' to babelBundle externals (require.resolve at runtime)
- Add '../transform/esmLoader.js' to common/runner/worker bundle
  externals (require.resolve for ESM loader registration)
- Fix babelBundle.ts importSource to use bare 'playwright' specifier
- Remove unused WebSocketEventEmitter type alias in browserServerImpl
@github-actions

This comment has been minimized.

The bare 'playwright' specifier breaks under pnpm's strict
node_modules — transitive deps aren't hoisted, so user test
files can't resolve 'playwright/jsx-runtime'. Revert to
path.dirname(require.resolve('playwright')) which gives an
absolute path that always resolves.

Suppress the resulting require-resolve-not-external warning
in the babelBundle step since 'playwright' is already external.
@github-actions

This comment has been minimized.

Instead of babelBundle.ts doing require.resolve('playwright')
(which creates a bundling dependency on the 'playwright' package),
pass the resolved path through TransformConfig.jsxImportSource.
configLoader.ts resolves it once at config load time and passes
it through the existing transform config pipeline.
@github-actions
Copy link
Copy Markdown
Contributor

Test results for "MCP"

6481 passed, 383 skipped


Merge workflow run.

@pavelfeldman pavelfeldman merged commit cc09e9e into microsoft:main Apr 10, 2026
35 of 37 checks passed
@github-actions
Copy link
Copy Markdown
Contributor

Test results for "tests 1"

1 failed
❌ [playwright-test] › reporter-html.spec.ts:271 › merged › should include image diffs for same expectation @macos-latest-node20

4 flaky ⚠️ [chromium-library] › library/video.spec.ts:687 › screencast › should capture full viewport on hidpi `@ubuntu-22.04-chromium-tip-of-tree`
⚠️ [chromium-page] › page/page-request-continue.spec.ts:754 › propagate headers cross origin redirect after interception `@chromium-ubuntu-22.04-node22`
⚠️ [firefox-library] › library/inspector/cli-codegen-1.spec.ts:1080 › 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`

39096 passed, 846 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