Skip to content

Commit

Permalink
data loaders from plugins are not side-effect free
Browse files Browse the repository at this point in the history
  • Loading branch information
@evanw
evanw committed Mar 9, 2021
1 parent e0f7312 commit 48306f2
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 40 deletions.
95 changes: 55 additions & 40 deletions internal/bundler/bundler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
},
}

Expand Down Expand Up @@ -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

Expand All @@ -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

Expand All @@ -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

Expand All @@ -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

Expand All @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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",
Expand Down
33 changes: 33 additions & 0 deletions scripts/plugin-tests.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down

0 comments on commit 48306f2

Please sign in to comment.