fix(esbuild,bun): keep empty results, filter by id before reading, normalize hooks once

plans/README.md

@@ -18,7 +18,7 @@ were confirmed by reading the cited code.
 | 001 | [Test harness: cold-start build, fixture script, error-checking webpack/rspack runners, honest loader filter tests](./001-test-harness.md) | P1 | M | — | DONE |
 | 002 | [rspack loader parity: `raw` loaders, `buildEnd` on `processAssets`](./002-rspack-loader-parity.md) | P1 | S | 001 | DONE |
 | 003 | [webpack: `resolveId` errors reach the callback; `emitFile` emits exact bytes](./003-webpack-resolveid-errors-and-emitfile-bytes.md) | P1 | S | 001 | TODO |
-| 004 | [esbuild/Bun hot path: empty results, read-after-filter, normalise once, scoped watch files, dead map branch, Bun setup crash](./004-esbuild-bun-hot-path.md) | P1 | M | — | TODO |
+| 004 | [esbuild/Bun hot path: empty results, read-after-filter, normalise once, scoped watch files, dead map branch, Bun setup crash](./004-esbuild-bun-hot-path.md) | P1 | M | — | DONE |
 | 005 | [Farm hook contract: `load`→null, `transform`→string, `resolveId` importer/isEntry/sideEffects](./005-farm-hook-contract.md) | P1 | S | — | TODO |
 | 006 | [Filter glob base escaping, `mergeSourceMaps` empty-map guard, direct `src/utils` tests](./006-filter-cwd-escaping-and-utils-tests.md) | P2 | S | — | TODO |
 | 007 | [Lifecycle coverage: `watchChange`, `buildEnd`/`emitFile` on rspack/esbuild/bun, fixture sourcemaps](./007-lifecycle-hook-coverage.md) | P2 | M | 001, 002 | TODO |

src/bun/index.ts

@@ -72,7 +72,7 @@ export function getBunPlugin<UserOptions = Record<string, never>>(
           }))
 
         const transformHooks = plugins
-          .filter(plugin => plugin.transform || plugin.transformInclude)
+          .filter(plugin => plugin.transform)
           .map(plugin => ({
             plugin,
             ...normalizeObjectHook('transform', plugin.transform!),
@@ -195,18 +195,18 @@ export function getBunPlugin<UserOptions = Record<string, never>>(
             }
           }
 
-          if (!hasResult && namespace === 'file' && transformHooks.length > 0) {
+          // Decide by id first, so a file no transform hook wants is never read
+          const namespaceTransformHooks = (namespace === 'file'
+            ? transformHooks
+            : transformHooks.filter(h => h.plugin.name === namespace))
+            .filter(({ plugin, idFilter }) => (!plugin.transformInclude || plugin.transformInclude(id)) && idFilter(id))
+
+          if (!hasResult && namespace === 'file' && namespaceTransformHooks.length > 0) {
             code = await Bun.file(id).text()
           }
 
           if (code !== undefined) {
-            const namespaceTransformHooks = namespace === 'file'
-              ? transformHooks
-              : transformHooks.filter(h => h.plugin.name === namespace)
-
             for (const { plugin, handler, filter } of namespaceTransformHooks) {
-              if (plugin.transformInclude && !plugin.transformInclude(id))
-                continue
               if (!filter(id, code))
                 continue
 

src/esbuild/index.ts

@@ -1,4 +1,3 @@
-import type { RawSourceMap } from '@jridgewell/remapping'
 import type { OnLoadOptions, OnLoadResult, PluginBuild } from 'esbuild'
 import type { SourceMap } from 'rollup'
 import type {
@@ -14,7 +13,6 @@ import { version as unpluginVersion } from '../../package.json'
 import { normalizeObjectHook } from '../utils/filter'
 import { toArray } from '../utils/general'
 import {
-  combineSourcemaps,
   createBuildContext,
   createPluginContext,
   guessLoader,
@@ -97,7 +95,7 @@ export function getEsbuildPlugin<UserOptions = Record<string, never>>(
 
             if (onLoadCb)
               result = await onLoadCb!(args)
-            if (result?.contents)
+            if (result?.contents != null)
               break
           }
 
@@ -113,7 +111,7 @@ export function getEsbuildPlugin<UserOptions = Record<string, never>>(
                 ...result,
                 ...args,
                 async getContents() {
-                  if (result?.contents)
+                  if (result?.contents != null)
                     return result.contents as string
 
                   if (fsContentsCached)
@@ -143,12 +141,12 @@ export function getEsbuildPlugin<UserOptions = Record<string, never>>(
               }
 
               const _result = await onTransformCb(newArgs)
-              if (_result?.contents)
+              if (_result?.contents != null)
                 result = _result
             }
           }
 
-          if (result?.contents)
+          if (result?.contents != null)
             return result
         })
       }
@@ -190,6 +188,7 @@ function buildSetup() {
       }
 
       if (plugin.resolveId) {
+        const { handler, filter } = normalizeObjectHook('resolveId', plugin.resolveId)
         onResolve({ filter: onResolveFilter }, async (args) => {
           const id = args.path
           if (initialOptions.external?.includes(id)) {
@@ -199,7 +198,6 @@ function buildSetup() {
             return
           }
 
-          const { handler, filter } = normalizeObjectHook('resolveId', plugin.resolveId!)
           if (!filter(id))
             return
 
@@ -239,8 +237,8 @@ function buildSetup() {
       }
 
       if (plugin.load) {
+        const { handler, filter } = normalizeObjectHook('load', plugin.load)
         onLoad({ filter: onLoadFilter }, async (args) => {
-          const { handler, filter } = normalizeObjectHook('load', plugin.load!)
           const id = args.path + (args.suffix || '') // compat for #427
 
           if (plugin.loadInclude && !plugin.loadInclude(id))
@@ -284,12 +282,13 @@ function buildSetup() {
       }
 
       if (plugin.transform) {
+        const { handler, filter, idFilter } = normalizeObjectHook('transform', plugin.transform)
         onTransform({ filter: onLoadFilter }, async (args) => {
-          const { handler, filter } = normalizeObjectHook('transform', plugin.transform!)
-
           const id = args.path + (args.suffix || '')
           if (plugin.transformInclude && !plugin.transformInclude(id))
             return
+          if (!idFilter(id))
+            return
           let code = await args.getContents()
           if (!filter(id, code))
             return
@@ -304,28 +303,12 @@ function buildSetup() {
           }
           else if (typeof result === 'object' && result !== null) {
             code = result.code
-            // if we already got sourcemap from `load()`,
-            // combine the two sourcemaps
-            if (map && result.map) {
-              map = combineSourcemaps(args.path, [
-                result.map === 'string'
-                  ? JSON.parse(result.map)
-                  : result.map as RawSourceMap,
-                map as RawSourceMap,
-              ]) as SourceMap
-            }
-            else {
-              if (typeof result.map === 'string') {
-                map = JSON.parse(result.map)
-              }
-              else {
-                // otherwise, we always keep the last one, even if it's empty
-                map = result.map as SourceMap
-              }
-            }
+            map = typeof result.map === 'string'
+              ? JSON.parse(result.map)
+              : result.map as SourceMap
           }
 
-          if (code) {
+          if (code !== undefined) {
             if (map)
               code = processCodeWithSourceMap(map, code)
             return {

src/esbuild/utils.ts

@@ -133,7 +133,6 @@ export function combineSourcemaps(
 }
 
 export function createBuildContext(build: PluginBuild): UnpluginBuildContext {
-  const watchFiles: string[] = []
   const { initialOptions } = build
   return {
     parse,
@@ -152,7 +151,7 @@ export function createBuildContext(build: PluginBuild): UnpluginBuildContext {
       }
     },
     getWatchFiles() {
-      return watchFiles
+      return []
     },
     getNativeBuildContext() {
       return { framework: 'esbuild', build }
@@ -172,11 +171,16 @@ export function createPluginContext(context: UnpluginBuildContext): {
     warn(message) { warnings.push(normalizeMessage(message)) },
   }
 
+  // Each hook call reports only the files it added, so esbuild does not tie every module to every watched file
+  const watchFiles: string[] = []
   const mixedContext: UnpluginContext & UnpluginBuildContext = {
     ...context,
     ...pluginContext,
     addWatchFile(id: string) {
-      context.getWatchFiles().push(id)
+      watchFiles.push(id)
+    },
+    getWatchFiles() {
+      return watchFiles
     },
   }
 

src/utils/filter.ts

@@ -132,13 +132,12 @@ function createFilterForId(filter: StringFilter | undefined): PluginFilter | und
 }
 
 function createFilterForTransform(
-  idFilter: StringFilter | undefined,
+  idFilterFunction: PluginFilter | undefined,
   codeFilter: StringFilter | undefined,
 ): TransformHookFilter | undefined {
-  if (!idFilter && !codeFilter)
-    return
-  const idFilterFunction = createIdFilter(idFilter)
   const codeFilterFunction = createCodeFilter(codeFilter)
+  if (!idFilterFunction && !codeFilterFunction)
+    return
   return (id, code) => {
     let fallback = true
     if (idFilterFunction) {
@@ -166,34 +165,29 @@ export function normalizeObjectHook<T extends (...args: any[]) => any>(
 export function normalizeObjectHook<T extends (...args: any[]) => any>(
   name: 'transform',
   hook: Hook<T, HookFilter>,
-): { handler: T, filter: TransformHookFilter }
+): { handler: T, filter: TransformHookFilter, idFilter: PluginFilter }
 export function normalizeObjectHook<T extends (...args: any[]) => any>(
   name: 'resolveId' | 'load' | 'transform',
   hook: Hook<T, ResolveIdHookFilter | LoadHookFilter | HookFilter>,
 ): {
   handler: T
   filter: PluginFilter | TransformHookFilter
+  idFilter?: PluginFilter
 } {
-  let handler: T
-  let filter: PluginFilter | TransformHookFilter | undefined
+  const matchAll = () => true
+  if (typeof hook === 'function')
+    return { handler: hook, filter: matchAll, idFilter: matchAll }
 
-  if (typeof hook === 'function') {
-    handler = hook
-  }
-  else {
-    handler = hook.handler
-    const hookFilter = hook.filter
-    if (name === 'resolveId' || name === 'load') {
-      filter = createFilterForId(hookFilter?.id)
-    }
-    else {
-      const transformFilter = hookFilter as HookFilter | undefined
-      filter = createFilterForTransform(transformFilter?.id, transformFilter?.code)
-    }
-  }
+  const { handler, filter: hookFilter } = hook
+  if (name === 'resolveId' || name === 'load')
+    return { handler, filter: createFilterForId(hookFilter?.id) || matchAll }
 
+  const transformFilter = hookFilter as HookFilter | undefined
+  const idFilter = createIdFilter(transformFilter?.id)
   return {
     handler,
-    filter: filter || (() => true),
+    filter: createFilterForTransform(idFilter, transformFilter?.code) || matchAll,
+    // The id part alone, so callers can skip reading a module's code when the id already rules it out
+    idFilter: idFilter || matchAll,
   }
 }

test/unit-tests/bun/index.test.ts

@@ -48,4 +48,19 @@ describe.skipIf(typeof Bun === 'undefined')('bun plugin', () => {
     expect(bunPlugin.name).toBe('unplugin-host:plugin-1:plugin-2')
     expect(bunPlugin.setup).toBeDefined()
   })
+
+  it('ignores transformInclude without a transform hook', async () => {
+    const unplugin = createUnplugin(() => ({
+      name: 'include-only',
+      transformInclude: () => true,
+    }))
+
+    const build = {
+      config: {},
+      onStart: () => {},
+      onResolve: () => {},
+      onLoad: () => {},
+    }
+    await expect(unplugin.bun().setup(build as any)).resolves.toBeUndefined()
+  })
 })

test/unit-tests/esbuild/fixtures/empty-result/dep.js

@@ -0,0 +1 @@
+export const dep = 'ORIGINAL_DEP'

test/unit-tests/esbuild/fixtures/empty-result/entry.js

@@ -0,0 +1,3 @@
+import { dep } from './dep.js'
+
+console.log(dep)

test/unit-tests/esbuild/fixtures/empty-result/virtual-entry.js

@@ -0,0 +1,4 @@
+import { dep } from './dep.js'
+import 'virtual:empty'
+
+console.log(dep)