From 48306f211098a24af9c9b377f328d90aba6b341b Mon Sep 17 00:00:00 2001 From: Evan Wallace Date: Mon, 8 Mar 2021 19:30:58 -0800 Subject: [PATCH] data loaders from plugins are not side-effect free --- internal/bundler/bundler.go | 95 +++++++++++++++++++++---------------- scripts/plugin-tests.js | 33 +++++++++++++ 2 files changed, 88 insertions(+), 40 deletions(-) diff --git a/internal/bundler/bundler.go b/internal/bundler/bundler.go index 79155e30014..1a5adb590fd 100644 --- a/internal/bundler/bundler.go +++ b/internal/bundler/bundler.go @@ -65,11 +65,19 @@ type file struct { isEntryPoint bool // If true, this file was listed as not having side effects by a package.json - // file in one of our containing directories with a "sideEffects" field. + // file in one of our containing directories with a "sideEffects" field, or + // this was a data-oriented loader (e.g. "text") that is known to not have + // side effects. ignoreIfUnused bool + // If this is true and this file is imported without any import names, issue + // a warning. This is different than the flag above because imports of files + // generated by plugins do not cause a warning (since running the plugin is + // a side effect). + warnIfUnused bool + // This is optional additional information about "ignoreIfUnused" for errors - ignoreIfUnusedData *resolver.IgnoreIfUnusedData + warnIfUnusedData *resolver.IgnoreIfUnusedData } type fileRepr interface { @@ -126,22 +134,22 @@ type Bundle struct { } type parseArgs struct { - fs fs.FS - log logger.Log - res resolver.Resolver - caches *cache.CacheSet - keyPath logger.Path - prettyPath string - sourceIndex uint32 - importSource *logger.Source - ignoreIfUnused bool - ignoreIfUnusedData *resolver.IgnoreIfUnusedData - importPathRange logger.Range - pluginData interface{} - options config.Options - results chan parseResult - inject chan config.InjectedFile - skipResolve bool + fs fs.FS + log logger.Log + res resolver.Resolver + caches *cache.CacheSet + keyPath logger.Path + prettyPath string + sourceIndex uint32 + importSource *logger.Source + ignoreIfUnused bool + warnIfUnusedData *resolver.IgnoreIfUnusedData + importPathRange logger.Range + pluginData interface{} + options config.Options + results chan parseResult + inject chan config.InjectedFile + skipResolve bool } type parseResult struct { @@ -214,8 +222,9 @@ func parseFile(args parseArgs) { pluginData: pluginData, // Record information from "sideEffects" in "package.json" - ignoreIfUnused: args.ignoreIfUnused, - ignoreIfUnusedData: args.ignoreIfUnusedData, + ignoreIfUnused: args.ignoreIfUnused, + warnIfUnused: args.ignoreIfUnused, + warnIfUnusedData: args.warnIfUnusedData, }, } @@ -257,6 +266,7 @@ func parseFile(args parseArgs) { expr, ok := args.caches.JSONCache.Parse(args.log, source, js_parser.JSONOptions{}) ast := js_parser.LazyExportAST(args.log, source, js_parser.OptionsFromConfig(&args.options), expr, "") result.file.ignoreIfUnused = true + result.file.warnIfUnused = pluginName == "" result.file.repr = &reprJS{ast: ast} result.ok = ok @@ -266,6 +276,7 @@ func parseFile(args parseArgs) { ast := js_parser.LazyExportAST(args.log, source, js_parser.OptionsFromConfig(&args.options), expr, "") ast.URLForCSS = "data:text/plain;base64," + encoded result.file.ignoreIfUnused = true + result.file.warnIfUnused = pluginName == "" result.file.repr = &reprJS{ast: ast} result.ok = true @@ -276,6 +287,7 @@ func parseFile(args parseArgs) { ast := js_parser.LazyExportAST(args.log, source, js_parser.OptionsFromConfig(&args.options), expr, "") ast.URLForCSS = "data:" + mimeType + ";base64," + encoded result.file.ignoreIfUnused = true + result.file.warnIfUnused = pluginName == "" result.file.repr = &reprJS{ast: ast} result.ok = true @@ -285,6 +297,7 @@ func parseFile(args parseArgs) { ast := js_parser.LazyExportAST(args.log, source, js_parser.OptionsFromConfig(&args.options), expr, "__toBinary") ast.URLForCSS = "data:application/octet-stream;base64," + encoded result.file.ignoreIfUnused = true + result.file.warnIfUnused = pluginName == "" result.file.repr = &reprJS{ast: ast} result.ok = true @@ -296,6 +309,7 @@ func parseFile(args parseArgs) { ast := js_parser.LazyExportAST(args.log, source, js_parser.OptionsFromConfig(&args.options), expr, "") ast.URLForCSS = url result.file.ignoreIfUnused = true + result.file.warnIfUnused = pluginName == "" result.file.repr = &reprJS{ast: ast} result.ok = true @@ -326,6 +340,7 @@ func parseFile(args parseArgs) { ast := js_parser.LazyExportAST(args.log, source, js_parser.OptionsFromConfig(&args.options), expr, "") ast.URLForCSS = publicPath result.file.ignoreIfUnused = true + result.file.warnIfUnused = pluginName == "" result.file.repr = &reprJS{ast: ast} result.ok = true @@ -1006,22 +1021,22 @@ func (s *scanner) maybeParseFile( } go parseFile(parseArgs{ - fs: s.fs, - log: s.log, - res: s.res, - caches: s.caches, - keyPath: path, - prettyPath: prettyPath, - sourceIndex: sourceIndex, - importSource: importSource, - ignoreIfUnused: resolveResult.IgnorePrimaryIfUnused != nil, - ignoreIfUnusedData: resolveResult.IgnorePrimaryIfUnused, - importPathRange: importPathRange, - pluginData: pluginData, - options: optionsClone, - results: s.resultChannel, - inject: inject, - skipResolve: skipResolve, + fs: s.fs, + log: s.log, + res: s.res, + caches: s.caches, + keyPath: path, + prettyPath: prettyPath, + sourceIndex: sourceIndex, + importSource: importSource, + ignoreIfUnused: resolveResult.IgnorePrimaryIfUnused != nil, + warnIfUnusedData: resolveResult.IgnorePrimaryIfUnused, + importPathRange: importPathRange, + pluginData: pluginData, + options: optionsClone, + results: s.resultChannel, + inject: inject, + skipResolve: skipResolve, }) return sourceIndex @@ -1416,16 +1431,16 @@ func (s *scanner) processScannedFiles() []file { // Don't include this module for its side effects if it can be // considered to have no side effects if record.WasOriginallyBareImport && !s.options.IgnoreDCEAnnotations { - if otherFile := &s.results[record.SourceIndex.GetIndex()].file; otherFile.ignoreIfUnused { + if otherFile := &s.results[record.SourceIndex.GetIndex()].file; otherFile.warnIfUnused { var notes []logger.MsgData - if otherFile.ignoreIfUnusedData != nil { + if otherFile.warnIfUnusedData != nil { var text string - if otherFile.ignoreIfUnusedData.IsSideEffectsArrayInJSON { + if otherFile.warnIfUnusedData.IsSideEffectsArrayInJSON { text = "It was excluded from the \"sideEffects\" array in the enclosing \"package.json\" file" } else { text = "\"sideEffects\" is false in the enclosing \"package.json\" file" } - notes = append(notes, logger.RangeData(otherFile.ignoreIfUnusedData.Source, otherFile.ignoreIfUnusedData.Range, text)) + notes = append(notes, logger.RangeData(otherFile.warnIfUnusedData.Source, otherFile.warnIfUnusedData.Range, text)) } s.log.AddRangeWarningWithNotes(&result.file.source, record.Range, fmt.Sprintf("Ignoring this import because %q was marked as having no side effects", diff --git a/scripts/plugin-tests.js b/scripts/plugin-tests.js index a5c024eafde..298fd3a0e54 100644 --- a/scripts/plugin-tests.js +++ b/scripts/plugin-tests.js @@ -1820,6 +1820,39 @@ let pluginTests = { } assert.strictEqual(resolveKind, 'url-token') }, + + async warnIfUnusedNoWarning({ esbuild }) { + const build = await esbuild.build({ + entryPoints: ['entry'], + bundle: true, + write: false, + logLevel: 'silent', + plugins: [{ + name: 'plugin', + setup(build) { + build.onResolve({ filter: /.*/ }, args => { + if (args.importer === '') return { path: args.path, namespace: 'entry' } + else return { path: args.path, namespace: 'bare-import' } + }) + build.onLoad({ filter: /.*/, namespace: 'entry' }, () => { + return { + contents: ` + import "base64" + import "binary" + import "dataurl" + import "json" + import "text" + `, + } + }) + build.onLoad({ filter: /.*/, namespace: 'bare-import' }, args => { + return { contents: `[1, 2, 3]`, loader: args.path } + }) + }, + }], + }) + assert.strictEqual(build.warnings.length, 0) + }, } async function main() {