Skip to content

Commit

Permalink
fix #3790: warn about incorrect onResolve plugin
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Jun 30, 2024
1 parent ab9b002 commit 94f09ea
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 0 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
27 changes: 27 additions & 0 deletions pkg/api/api_impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
},
})
Expand Down
19 changes: 19 additions & 0 deletions scripts/plugin-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down

0 comments on commit 94f09ea

Please sign in to comment.