Skip to content

Fix Vite config-loader warnings and async_hooks browser stub in the CLI build - #4774

Merged
wojtekn merged 1 commit into
trunkfrom
fix-vite-config-loader-warnings
Sep 4, 2026
Merged

Fix Vite config-loader warnings and async_hooks browser stub in the CLI build#4774
wojtekn merged 1 commit into
trunkfrom
fix-vite-config-loader-warnings

Conversation

@wojtekn

@wojtekn wojtekn commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Related issues

  • N/A

How AI was used in this PR

Claude Code investigated the warnings, produced the changes, and verified them. All build output and the running server were inspected directly rather than trusting the absence of warnings.

Proposed Changes

Two problems surfaced by the same build output on npm start.

Vite 8 config-loader warnings. Every build and test run printed warnings that our configs use features unsupported by configLoader: 'native', which becomes the default in a future Vite major: __dirname, extensionless relative imports, and JSON imports without attributes. Fixing them now keeps the noise out of everyone's terminal and means the eventual loader switch is a non-event rather than a broken build.

Node >= 22 is already required, so import.meta.dirname is available. JSON imports use createRequire (the pattern apps/ui already used) rather than the with { type: 'json' } attribute the warning suggests — the pinned wp-prettier@3.0.3 cannot parse import attributes, and tsconfig targets es2022, which doesn't permit them. Moving to the modern syntax is a separate change gated on those two upgrades.

A real bug hiding behind one of those warnings. The async_hooks externalization message was not cosmetic. The CLI is a Node program, but Vite builds it in the default client environment, so bundled Express dependencies were resolved with browser conditions. require('async_hooks') was rewritten to an empty-object stub:

function tryRequireAsyncHooks() {
  try { return require___vite_browser_external(); }  // module.exports = {}
  catch (e) { return {}; }
}

That left asyncHooks.AsyncResource permanently undefined, so on-finished and raw-body silently fell back to unwrapped callbacks — losing async-context propagation in the studio ui server. It fails quietly, which is why it only ever showed up as a build warning. After the fix the same code emits __require("async_hooks"), resolving to the real module via the createRequire shim the config already installs per chunk.

No user-visible behaviour change is expected; the studio ui server regains correct async-context propagation.

Testing Instructions

  1. npm start — the config-loader and async_hooks externalization warnings should be gone.
  2. npm run cli:build && npm run cli:build:ui — both clean.
  3. Confirm the fix in the output: grep -r "vite_browser_external" apps/cli/dist/cli/*.mjs returns nothing, and grep -r "async_hooks" apps/cli/dist/cli/*.mjs shows __require("async_hooks").
  4. Confirm the CLI is still self-contained: grep -rn 'from "express"' apps/cli/dist/cli/*.mjs returns nothing (express stays bundled).
  5. node apps/cli/dist/cli/main.mjs ui --no-open --port 8099, then check /index.local.html → 200, /api/sites → 200, and a POST with a JSON body → 400 (exercises the raw-body path).
  6. npm test -- --tagsFilter='!e2e' and npm run typecheck.

Pre-merge Checklist

  • Have you checked for TypeScript, React or other console errors?

🤖 Generated with Claude Code

Vite 8 warns that the build configs use features unsupported by the
upcoming `configLoader: 'native'` default: `__dirname`, extensionless
relative imports, and JSON imports without attributes. Node >=22 is
already required, so `import.meta.dirname` is available; JSON imports use
createRequire (as apps/ui already did) because the pinned wp-prettier
cannot parse import attributes and tsconfig targets es2022.

The same build also silently mis-resolved async_hooks. The CLI builds in
Vite's default `client` environment, so bundled Express dependencies
(on-finished, raw-body) had their `require('async_hooks')` rewritten to an
empty-object browser stub, leaving `AsyncResource` undefined and dropping
async context propagation in the `studio ui` server. Adding async_hooks to
the existing Node built-in externals resolves it through the per-chunk
createRequire shim while keeping express bundled, so the shipped CLI stays
self-contained.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@wpmobilebot

Copy link
Copy Markdown
Collaborator

📊 Performance Test Results

Comparing 4a6696f vs trunk

app-size

Metric trunk 4a6696f Diff Change
App Size (Mac) 1435.56 MB 1435.56 MB 0.00 MB ⚪ 0.0%

site-editor

Metric trunk 4a6696f Diff Change
load 1169 ms 1184 ms +15 ms ⚪ 0.0%

site-startup

Metric trunk 4a6696f Diff Change
siteCreation 7514 ms 7519 ms +5 ms ⚪ 0.0%
siteStartup 3396 ms 3387 ms 9 ms ⚪ 0.0%

Results are median values from multiple test runs.

Legend: 🟢 Improvement (faster) | 🔴 Regression (slower) | ⚪ No change (<50ms diff)

@wojtekn
wojtekn merged commit 7cc04cc into trunk Sep 4, 2026
15 checks passed
@wojtekn
wojtekn deleted the fix-vite-config-loader-warnings branch September 4, 2026 09:21
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