From 94f09ea521d966f9223c002943ef840be5703aa0 Mon Sep 17 00:00:00 2001
From: Evan Wallace <evan.exe@gmail.com>
Date: Sun, 30 Jun 2024 12:25:43 -0400
Subject: [PATCH] fix #3790: warn about incorrect `onResolve` plugin

---
 CHANGELOG.md            |  4 ++++
 pkg/api/api_impl.go     | 27 +++++++++++++++++++++++++++
 scripts/plugin-tests.js | 19 +++++++++++++++++++
 3 files changed, 50 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 41feb076023..581145a28cc 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -32,6 +32,10 @@
 
     The ECMAScript 2024 specification was just approved, so it has been added to esbuild as a possible compilation target. You can read more about the features that it adds here: [https://2ality.com/2024/06/ecmascript-2024.html](https://2ality.com/2024/06/ecmascript-2024.html). The only addition that's relevant for esbuild is the regular expression `/v` flag. With `--target=es2024`, regular expressions that use the `/v` flag will now be passed through untransformed instead of being transformed into a call to `new RegExp`.
 
+* Warn about `onResolve` plugins not setting a path ([#3790](https://github.com/evanw/esbuild/issues/3790))
+
+    Plugins that return values from `onResolve` without resolving the path (i.e. without setting either `path` or `external: true`) will now cause a warning. This is because esbuild only uses return values from `onResolve` if it successfully resolves the path, and it's not good for invalid input to be silently ignored.
+
 ## 0.21.5
 
 * Fix `Symbol.metadata` on classes without a class decorator ([#3781](https://github.com/evanw/esbuild/issues/3781))
diff --git a/pkg/api/api_impl.go b/pkg/api/api_impl.go
index ba120affa52..291dcd20e51 100644
--- a/pkg/api/api_impl.go
+++ b/pkg/api/api_impl.go
@@ -1927,6 +1927,33 @@ func (impl *pluginImpl) onResolve(options OnResolveOptions, callback func(OnReso
 
 			// Convert log messages
 			result.Msgs = convertErrorsAndWarningsToInternal(response.Errors, response.Warnings)
+
+			// Warn if the plugin returned things without resolving the path
+			if response.Path == "" && !response.External {
+				var what string
+				if response.Namespace != "" {
+					what = "namespace"
+				} else if response.Suffix != "" {
+					what = "suffix"
+				} else if response.PluginData != nil {
+					what = "pluginData"
+				} else if response.WatchFiles != nil {
+					what = "watchFiles"
+				} else if response.WatchDirs != nil {
+					what = "watchDirs"
+				}
+				if what != "" {
+					path := "path"
+					if logger.API == logger.GoAPI {
+						what = strings.Title(what)
+						path = strings.Title(path)
+					}
+					result.Msgs = append(result.Msgs, logger.Msg{
+						Kind: logger.Warning,
+						Data: logger.MsgData{Text: fmt.Sprintf("Returning %q doesn't do anything when %q is empty", what, path)},
+					})
+				}
+			}
 			return
 		},
 	})
diff --git a/scripts/plugin-tests.js b/scripts/plugin-tests.js
index c0bb9bd2edd..0a4400f2d28 100644
--- a/scripts/plugin-tests.js
+++ b/scripts/plugin-tests.js
@@ -2148,6 +2148,25 @@ let pluginTests = {
     assert.deepStrictEqual({ ...esbuildFromBuild }, { ...esbuild })
   },
 
+  async onResolveSuffixWithoutPath({ esbuild, testDir }) {
+    const input = path.join(testDir, 'in.js')
+    await writeFileAsync(input, `works()`)
+    const result = await esbuild.build({
+      entryPoints: [input],
+      logLevel: 'silent',
+      write: false,
+      plugins: [{
+        name: 'the-plugin',
+        setup(build) {
+          build.onResolve({ filter: /.*/ }, () => ({ suffix: '?just suffix without path' }))
+        },
+      }],
+    })
+    assert.strictEqual(result.warnings.length, 1)
+    assert.strictEqual(result.warnings[0].text, `Returning "suffix" doesn't do anything when "path" is empty`)
+    assert.strictEqual(result.warnings[0].pluginName, 'the-plugin')
+  },
+
   async onResolveInvalidPathSuffix({ esbuild }) {
     try {
       await esbuild.build({