Skip to content

fix(hmr): allow watch-only mode without loader internals - #85

Open
XuJianrui wants to merge 2 commits into
cordiverse:mainfrom
XuJianrui:fix/hmr-internal-contract
Open

fix(hmr): allow watch-only mode without loader internals#85
XuJianrui wants to merge 2 commits into
cordiverse:mainfrom
XuJianrui:fix/hmr-internal-contract

Conversation

@XuJianrui

@XuJianrui XuJianrui commented Aug 19, 2026

Copy link
Copy Markdown

Problem

HMR crashes on mount in watch-only mode (root: []) when the loader does not expose its internal module loader, and the error message misleadingly points at --expose-internals — which that mode does not need.

Root cause

loader.internal is resolved by fromInternal() based on whether Node was launched with --expose-internals, and may be undefined. The Hmr constructor previously required it unconditionally, violating the ModuleLoader | undefined contract the loader declares.

Changes

  • The constructor only asserts loader.internal when root.length > 0 (module-reload mode); watch-only mode creates no watcher and tracks no externals.
  • The internal field is now typed ModuleLoader | undefined. Module-reload paths use a requiredInternal accessor (fails fast with an actionable error if somehow missing); getLinked degrades via ?. since it is reachable from external callers; the change handler also uses ?. as a second layer of defense (the watcher itself is gated by root.length); partialReload guards with a warning instead of crashing.
  • _resolve now throws on an unrecognized loader internal version instead of silently returning undefined; the LOADER_INTERNALS_ERROR message is unified and tells the user both ways to resolve it (--expose-internals, or watch-only root: []).
  • Test directory resolution switches to import.meta.dirname, fixing the double-drive bug of .pathname on Windows; isBuiltinOrExternal is extracted to remove a duplicated predicate.

Tests

3 new regression cases run on every CI (the repo's vitest.config.ts sets --expose-internals); the t.skip() in the watch-only × internal-present case is a defensive fallback for local runs without the flag. Module-reload × present is already exercised by the existing reload tests. Package tests pass 29/29.

Why

  • loader is left untouched: it already guards internal correctly (e.g. config/tree.ts checks if (this.ctx.loader.internal)), so editing it would only broaden the fix's scope.
  • The three internal access styles are intentional: requiredInternal fails fast on the module-reload path; ?. degrades silently for the public getLinked and acts as a second layer of defense in the change handler (the watcher itself is gated by root.length); partialReload warns instead of crashing.
  • The import.meta.dirname switch and the isBuiltinOrExternal / LOADER_INTERNALS_ERROR extractions are behavior-preserving cleanups kept with the fix rather than split out, since they are low-risk and already covered by the new tests.

@XuJianrui
XuJianrui force-pushed the fix/hmr-internal-contract branch 7 times, most recently from c0fafa7 to ba56385 Compare August 20, 2026 02:57
@XuJianrui
XuJianrui force-pushed the fix/hmr-internal-contract branch 14 times, most recently from 4295820 to d0ec823 Compare August 21, 2026 07:31
@XuJianrui
XuJianrui force-pushed the fix/hmr-internal-contract branch from d0ec823 to 4a32bb4 Compare August 21, 2026 10:27
The previous assertion reached into the private `watcher` field via
`as unknown as { watcher?: unknown }` to verify watch-only mode
created no FSWatcher. Replace it with an observable check: register
an `hmr/change` listener, write a probe file, and assert no event
fires. This pins the runtime contract (no watcher reacts to file
changes) without coupling the test to private implementation.

The unlink is best-effort to remain robust under the sandbox
safe-delete shim that aborts trash operations on Windows.
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.

1 participant