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

Author: XuJianrui

packages/hmr/src/index.ts

@@ -32,7 +32,7 @@ async function loadDependencies(job: ModuleJob, ignored = new Set<string>()) {
   const dependencies = new Set<string>()
   async function traverse(job: ModuleJob) {
     if (ignored.has(job.url) || dependencies.has(job.url)) return
-    if (job.url.startsWith('node:') || job.url.includes('/node_modules/')) return
+    if (isBuiltinOrExternal(job.url)) return
     dependencies.add(job.url)
     const children = await job.linked
     await Promise.all(Array.prototype.map.call(children, traverse))
@@ -41,6 +41,18 @@ async function loadDependencies(job: ModuleJob, ignored = new Set<string>()) {
   return dependencies
 }
 
+const LOADER_INTERNALS_ERROR =
+  'HMR module reload requires loader internals: run with --expose-internals (Node process flag) or use watch-only mode (root: [])'
+
+/**
+ * True for Node builtins (`node:` scheme) and anything inside node_modules —
+ * i.e. framework / dependency code that should never be treated as user code
+ * for HMR purposes. Shared by `loadDependencies` and `analyzeChanges`.
+ */
+function isBuiltinOrExternal(url: string): boolean {
+  return url.startsWith('node:') || url.includes('/node_modules/')
+}
+
 interface Reload {
   filename: string
   runtime?: Plugin.Runtime
@@ -51,8 +63,24 @@ interface Reload {
 class Hmr extends Service {
   public baseDir: string
 
-  private internal: ModuleLoader
-  private watcher!: FSWatcher
+  private internal: ModuleLoader | undefined
+  private watcher: FSWatcher | undefined
+
+  /**
+   * Non-null accessor for module-reload paths. The constructor guarantees
+   * internals exist whenever `root.length > 0`, so callers in that mode can
+   * rely on this being defined; this getter turns that runtime guarantee into
+   * a compile-time fact, eliminating scattered non-null assertions.
+   * In watch-only mode (`root: []`) callers must use optional chaining or the
+   * `partialReload` guard instead.
+   */
+  private get requiredInternal(): ModuleLoader {
+    if (!this.internal) {
+      // Only reachable via a future caller that skipped the guard — fail fast.
+      throw new Error(LOADER_INTERNALS_ERROR)
+    }
+    return this.internal
+  }
 
   /**
    * Changes from externals will always trigger a full reload.
@@ -77,8 +105,8 @@ class Hmr extends Service {
 
   constructor(ctx: Context, public config: Hmr.Config) {
     super(ctx, 'hmr')
-    if (!this.ctx.loader.internal) {
-      throw new Error('--expose-internals is required for HMR service')
+    if (this.config.root.length && !this.ctx.loader.internal) {
+      throw new Error(LOADER_INTERNALS_ERROR)
     }
     this.internal = this.ctx.loader.internal
     this.baseDir = fileURLToPath(new URL(config.base || '.', ctx.baseUrl))
@@ -88,9 +116,12 @@ class Hmr extends Service {
    * Resolve a module specifier to a URL, compatible with Node 22-24.
    */
   private async _resolve(specifier: string, parentURL: string, attrs: ImportAttributes): Promise<ResolveResult> {
-    switch (this.internal.version) {
-      case 'v1': return await this.internal.resolve(specifier, parentURL, attrs)
-      case 'v2': return this.internal.resolveSync(parentURL, { specifier, attributes: attrs })
+    const internal = this.requiredInternal
+    const version = internal.version
+    switch (version) {
+      case 'v1': return await internal.resolve(specifier, parentURL, attrs)
+      case 'v2': return internal.resolveSync(parentURL, { specifier, attributes: attrs })
+      default: throw new Error(`unsupported loader internal version: ${String(version)}`)
     }
   }
 
@@ -105,51 +136,60 @@ class Hmr extends Service {
       this.ctx.logger.info('watching %o in %s', root, this.baseDir)
     }
 
-    const match = picomatch(ignored)
-    this.watcher = watch(root, {
-      ...this.config,
-      cwd: this.baseDir,
-      ignored: path => match(relative(this.baseDir, path)),
-    })
-
     // Collect externals: framework modules reachable from the main entry.
     // Changes to these files require a full process restart, not HMR.
-    const mainUrl = pathToFileURL(resolve(process.argv[1])).href
-    const mainJob = this.internal.loadCache.get(mainUrl)
-    if (mainJob) {
-      this.externals = await loadDependencies(mainJob)
+    // In watch-only mode (root: []) no module reload happens, so externals
+    // tracking is meaningless and loader internals may be unavailable.
+    if (this.config.root.length) {
+      // Track externals from the main entry. In embedded hosts the entry may
+      // be absent (process.argv[1] undefined); skip tracking instead of
+      // crashing — an empty externals set only disables full-reload detection.
+      const entry = process.argv[1]
+      const mainJob = entry && this.requiredInternal.loadCache.get(pathToFileURL(resolve(entry)).href)
+      this.externals = mainJob ? await loadDependencies(mainJob) : new Set()
     } else {
       this.externals = new Set()
     }
 
-    const partialReload = this.ctx.debounce(() => this.partialReload(), this.config.debounce)
+    // Only create a watcher when there are roots to watch; watch-only mode
+    // (root: []) never fires change events, so no watcher or handler is needed.
+    if (this.config.root.length) {
+      const match = picomatch(ignored)
+      this.watcher = watch(root, {
+        ...this.config,
+        cwd: this.baseDir,
+        ignored: path => match(relative(this.baseDir, path)),
+      })
 
-    this.watcher.on('change', async (path) => {
-      this.ctx.logger.debug('change detected at %C', path)
-      const filename = resolve(this.baseDir, path)
-      const url = pathToFileURL(filename).href
+      const partialReload = this.ctx.debounce(() => this.partialReload(), this.config.debounce)
 
-      // Full reload: the changed file is part of the framework
-      if (this.externals.has(url)) return loader.exit()
+      this.watcher.on('change', async (path) => {
+        this.ctx.logger.debug('change detected at %C', path)
+        const filename = resolve(this.baseDir, path)
+        const url = pathToFileURL(filename).href
 
-      // Partial reload: the file is in the ESM loadCache
-      // In Node 24, both CJS and ESM modules imported via import() end up
-      // in loadCache, so this check covers all module formats.
-      if (loader.internal!.loadCache.has(url)) {
-        this.stashed.add(url)
-        return partialReload()
-      }
+        // Full reload: the changed file is part of the framework
+        if (this.externals.has(url)) return loader.exit()
 
-      // Config reload: the file is a loader config file (e.g. cordis.yml)
-      for (const entry of this.ctx.loader.entries()) {
-        const include = entry.subtree as Include | undefined
-        if (include?.filename !== filename) continue
-        await include.refresh()
-        return
-      }
+        // Partial reload: the file is in the ESM loadCache
+        // In Node 24, both CJS and ESM modules imported via import() end up
+        // in loadCache, so this check covers all module formats.
+        if (this.internal?.loadCache.has(url)) {
+          this.stashed.add(url)
+          return partialReload()
+        }
+
+        // Config reload: the file is a loader config file (e.g. cordis.yml)
+        for (const entry of this.ctx.loader.entries()) {
+          const include = entry.subtree as Include | undefined
+          if (include?.filename !== filename) continue
+          await include.refresh()
+          return
+        }
 
-      this.ctx.emit('hmr/change', url)
-    })
+        this.ctx.emit('hmr/change', url)
+      })
+    }
   }
 
   // hide stack trace from HMR
@@ -158,7 +198,7 @@ class Hmr extends Service {
   ]
 
   async getLinked(url: string) {
-    const job = this.internal.loadCache.get(url)
+    const job = this.internal?.loadCache.get(url)
     if (!job) return []
     const linked = await job.linked
     return Array.prototype.map.call(linked, (job: ModuleJob) => job.url) as string[]
@@ -177,12 +217,10 @@ class Hmr extends Service {
     this.accepted = new Set(this.stashed)
     this.declined = new Set(this.externals)
 
-    const isExcluded = (url: string) => url.startsWith('node:') || url.includes('/node_modules/')
-
     await Promise.all([...this.stashed].map(async (url) => {
       const children = await this.getLinked(url)
       for (const child of children) {
-        if (this.accepted.has(child) || this.declined.has(child) || isExcluded(child)) continue
+        if (this.accepted.has(child) || this.declined.has(child) || isBuiltinOrExternal(child)) continue
         pending.push(child)
       }
     }))
@@ -194,7 +232,7 @@ class Hmr extends Service {
         const children = await this.getLinked(url)
         let isDeclined = true, isAccepted = false
         for (const child of children) {
-          if (this.declined.has(child) || isExcluded(child)) continue
+          if (this.declined.has(child) || isBuiltinOrExternal(child)) continue
           if (this.accepted.has(child)) {
             isAccepted = true
             break
@@ -227,6 +265,13 @@ class Hmr extends Service {
   }
 
   private async partialReload() {
+    // Defensive: partial reload is only reachable in module-reload mode where
+    // the constructor guarantees `internal`, but fail gracefully instead of a
+    // bare TypeError if the watcher ever fires without it.
+    if (!this.internal) {
+      this.ctx.logger.warn('partial reload skipped: loader internals unavailable')
+      return
+    }
     await this.analyzeChanges()
 
     const pending = new Map<ModuleJob, Plugin>()
@@ -245,7 +290,7 @@ class Hmr extends Service {
         try {
           const { url } = await this._resolve(name, baseUrl, {})
           if (this.declined.has(url)) continue
-          const job = this.internal.loadCache.get(url)
+          const job = this.requiredInternal.loadCache.get(url)
           const plugin = this.ctx.loader.unwrapExports(job?.module?.getNamespace())
           if (!job || !plugin) continue
           pending.set(job, plugin)
@@ -290,11 +335,12 @@ class Hmr extends Service {
     const esmBackup: Dict = Object.create(null)
     const cjsBackup: Dict = Object.create(null)
     const require = createRequire(import.meta.url)
+    const internal = this.requiredInternal
     for (const filename of this.accepted) {
       // Backup and clear ESM loadCache
-      const job = Map.prototype.get.call(this.internal.loadCache, filename)
+      const job = Map.prototype.get.call(internal.loadCache, filename)
       esmBackup[filename] = job
-      Map.prototype.delete.call(this.internal.loadCache, filename)
+      Map.prototype.delete.call(internal.loadCache, filename)
 
       // Backup and clear CJS Module._cache
       try {
@@ -310,7 +356,7 @@ class Hmr extends Service {
 
     const rollback = () => {
       for (const filename in esmBackup) {
-        Map.prototype.set.call(this.internal.loadCache, filename, esmBackup[filename])
+        Map.prototype.set.call(internal.loadCache, filename, esmBackup[filename])
       }
       for (const filepath in cjsBackup) {
         require.cache[filepath] = cjsBackup[filepath]

packages/hmr/tests/index.spec.ts

@@ -1,6 +1,8 @@
 import { Context, Fiber } from 'cordis'
 import Loader from '@cordisjs/plugin-loader'
 import Logger from '@cordisjs/plugin-logger-console'
+import Timer from '@cordisjs/plugin-timer'
+import Hmr from '@cordisjs/plugin-hmr'
 import { writeFileSync, readFileSync, unlinkSync } from 'node:fs'
 import { resolve } from 'node:path'
 import { expect, describe, it, beforeAll, afterAll, afterEach } from 'vitest'
@@ -72,6 +74,18 @@ async function createContext(configFile: string): Promise<{ ctx: Context; fiber:
   return { ctx, fiber }
 }
 
+// Helper: standalone context (Logger + Timer + Loader) for the watch-only
+// regression cases, which need direct control over the HMR config. Returns
+// the Loader fiber for disposal, matching the createContext pattern.
+async function createStandaloneContext(): Promise<{ ctx: Context; fiber: Fiber<Context> }> {
+  const ctx = new Context()
+  ctx.baseUrl = pathToFileURL(resolve(testDir) + '/').href
+  await ctx.plugin(Logger)
+  await ctx.plugin(Timer)
+  const fiber = await ctx.plugin(Loader)
+  return { ctx, fiber }
+}
+
 // Settle time after file restore, to let any triggered HMR finish
 const SETTLE_MS = 500
 
@@ -797,4 +811,57 @@ export function apply(ctx: Context) {
       expect(ctx.bail('hmr-test/get-value')).to.equal('stash-test-2')
     }, 10000)
   })
+
+  // ===== Watch-only without loader internals =====
+  // Regression: the constructor used to require `loader.internal`
+  // unconditionally, but watch-only mode (root: []) never touches module
+  // reload, so it must boot even when the native helper binding is missing.
+  describe('watch-only without loader internals', () => {
+    it('should start in watch-only mode without loader internals', async () => {
+      const { ctx, fiber } = await createStandaloneContext()
+      // Simulate a missing native helper binding: fromInternal() returns
+      // undefined in production when the addon cannot resolve.
+      ctx.loader.internal = undefined
+
+      await ctx.plugin(Hmr, { root: [], debounce: 100, ignored: [] })
+
+      expect(ctx.hmr).to.be.ok
+      // Watch-only mode must not create a watcher (guarded by root.length).
+      expect((ctx.hmr as unknown as { watcher?: unknown }).watcher).to.be.undefined
+      fiber?.dispose()
+      await new Promise(r => setTimeout(r, SETTLE_MS))
+    }, 10000)
+
+    it('should start in watch-only mode when loader internals are available', async (t) => {
+      const { ctx, fiber } = await createStandaloneContext()
+      // This case pins the "internal present × watch-only" cell. The repo test
+      // runner runs with --expose-internals, so the premise holds in CI; when
+      // the runner lacks the flag, skip instead of failing (the "internal
+      // missing" cell is already covered by the first case).
+      if (!ctx.loader.internal) {
+        fiber?.dispose()
+        t.skip()
+        return
+      }
+      expect(ctx.loader.internal).to.be.ok
+      await ctx.plugin(Hmr, { root: [], debounce: 100, ignored: [] })
+
+      expect(ctx.hmr).to.be.ok
+      // Watch-only mode must not create a watcher, regardless of internals.
+      expect((ctx.hmr as unknown as { watcher?: unknown }).watcher).to.be.undefined
+      fiber?.dispose()
+      await new Promise(r => setTimeout(r, SETTLE_MS))
+    }, 10000)
+
+    it('should still require loader internals for module reload', async () => {
+      const { ctx, fiber } = await createStandaloneContext()
+      ctx.loader.internal = undefined
+
+      await expect(ctx.plugin(Hmr, { root: ['.'], debounce: 100, ignored: [] }))
+        .rejects.toThrow(/loader internals/)
+
+      fiber?.dispose()
+      await new Promise(r => setTimeout(r, SETTLE_MS))
+    }, 10000)
+  })
 })