fix #581, close #582: auto-inject object defines

CHANGELOG.md

@@ -18,6 +18,12 @@
 
     This release fixes an edge case where files in ECMAScript module format that are converted to CommonJS format during bundling can generate exports to non-top-level symbols when code splitting is active. These files must be converted to CommonJS format if they are referenced by a `require()` call. When that happens, the symbols in that file are placed inside the CommonJS wrapper closure and are no longer top-level symbols. This means they should no longer be considered exportable for cross-chunk export generation due to code splitting. The result of this fix is that these cases no longer generate output files with module instantiation errors.
 
+* Allow `--define` with array and object literals ([#581](https://github.com/evanw/esbuild/issues/581))
+
+    The `--define` feature allows you to replace identifiers such as `DEBUG` with literal expressions such as `false`. This is valuable because the substitution can then participate in constant folding and dead code elimination. For example, `if (DEBUG) { ... }` could become `if (false) { ... }` which would then be completely removed in minified builds. However, doing this with compound literal expressions such as array and object literals is an anti-pattern because it could easily result in many copies of the same object in the output file.
+
+    This release adds support for array and object literals with `--define` anyway, but they work differently than other `--define` expressions. In this case a separate virtual file is created and configured to be injected into all files similar to how the `--inject` feature works. This means there is only at most one copy of the value in a given output file. However, these values do not participate in constant folding and dead code elimination, since the object can now potentially be mutated at run-time.
+
 ## 0.8.26
 
 * Ensure the current working directory remains unique per `startService()` call

internal/bundler/bundler.go

@@ -944,9 +944,51 @@ func (s *scanner) allocateSourceIndex(path logger.Path, kind cache.SourceIndexKi
 }
 
 func (s *scanner) preprocessInjectedFiles() {
-	injectedFiles := make([]config.InjectedFile, 0, len(s.options.InjectAbsPaths))
+	injectedFiles := make([]config.InjectedFile, 0, len(s.options.InjectedDefines)+len(s.options.InjectAbsPaths))
 	duplicateInjectedFiles := make(map[string]bool)
 	injectWaitGroup := sync.WaitGroup{}
+
+	// These are virtual paths that are generated for compound "--define" values.
+	// They are special-cased and are not available for plugins to intercept.
+	for _, define := range s.options.InjectedDefines {
+		// These should be unique by construction so no need to check for collisions
+		visitedKey := logger.Path{Text: fmt.Sprintf("<define:%s>", define.Name)}
+		sourceIndex := s.allocateSourceIndex(visitedKey, cache.SourceIndexNormal)
+		s.visited[visitedKey] = sourceIndex
+		source := logger.Source{
+			Index:          sourceIndex,
+			KeyPath:        visitedKey,
+			PrettyPath:     s.res.PrettyPath(visitedKey),
+			IdentifierName: js_ast.EnsureValidIdentifier(visitedKey.Text),
+		}
+
+		// The first "len(InjectedDefine)" injected files intentionally line up
+		// with the injected defines by index. The index will be used to import
+		// references to them in the parser.
+		injectedFiles = append(injectedFiles, config.InjectedFile{
+			Path:        visitedKey.Text,
+			SourceIndex: sourceIndex,
+			IsDefine:    true,
+		})
+
+		// Generate the file inline here since it has already been parsed
+		expr := js_ast.Expr{Data: define.Data}
+		ast := js_parser.LazyExportAST(s.log, source, js_parser.OptionsFromConfig(&s.options), expr, "")
+		result := parseResult{
+			ok: true,
+			file: file{
+				source:         source,
+				loader:         config.LoaderJSON,
+				repr:           &reprJS{ast: ast},
+				ignoreIfUnused: true,
+			},
+		}
+
+		// Append to the channel on a goroutine in case it blocks due to capacity
+		s.remaining++
+		go func() { s.resultChannel <- result }()
+	}
+
 	for _, absPath := range s.options.InjectAbsPaths {
 		prettyPath := s.res.PrettyPath(logger.Path{Text: absPath, Namespace: "file"})
 		lowerAbsPath := lowerCaseAbsPathForWindows(absPath)
@@ -976,6 +1018,7 @@ func (s *scanner) preprocessInjectedFiles() {
 			injectWaitGroup.Done()
 		}(i, prettyPath, resolveResult)
 	}
+
 	injectWaitGroup.Wait()
 	s.options.InjectedFiles = injectedFiles
 }

internal/bundler/linker.go

@@ -413,8 +413,7 @@ func newLinkerContext(
 			repr.meta = fileMeta{
 				cjsStyleExports: repr.ast.HasCommonJSFeatures() ||
 					(options.Mode == config.ModeBundle && repr.ast.ModuleScope.ContainsDirectEval) ||
-					(repr.ast.HasLazyExport && (c.options.Mode == config.ModePassThrough ||
-						(c.options.Mode == config.ModeConvertFormat && !c.options.OutputFormat.KeepES6ImportExportSyntax()))),
+					(repr.ast.HasLazyExport && c.options.Mode == config.ModeConvertFormat && !c.options.OutputFormat.KeepES6ImportExportSyntax()),
 				partMeta:                 make([]partMeta, len(repr.ast.Parts)),
 				resolvedExports:          resolvedExports,
 				isProbablyTypeScriptType: make(map[js_ast.Ref]bool),
@@ -451,14 +450,21 @@ func newLinkerContext(
 		file := &c.files[sourceIndex]
 		file.isEntryPoint = true
 
-		// Entry points with ES6 exports must generate an exports object when
-		// targeting non-ES6 formats. Note that the IIFE format only needs this
-		// when the global name is present, since that's the only way the exports
-		// can actually be observed externally.
-		if repr, ok := file.repr.(*reprJS); ok && repr.ast.HasES6Exports && (options.OutputFormat == config.FormatCommonJS ||
-			(options.OutputFormat == config.FormatIIFE && len(options.GlobalName) > 0)) {
-			repr.ast.UsesExportsRef = true
-			repr.meta.forceIncludeExportsForEntryPoint = true
+		if repr, ok := file.repr.(*reprJS); ok {
+			// Lazy exports default to CommonJS-style for the transform API
+			if repr.ast.HasLazyExport && c.options.Mode == config.ModePassThrough {
+				repr.meta.cjsStyleExports = true
+			}
+
+			// Entry points with ES6 exports must generate an exports object when
+			// targeting non-ES6 formats. Note that the IIFE format only needs this
+			// when the global name is present, since that's the only way the exports
+			// can actually be observed externally.
+			if repr.ast.HasES6Exports && (options.OutputFormat == config.FormatCommonJS ||
+				(options.OutputFormat == config.FormatIIFE && len(options.GlobalName) > 0)) {
+				repr.ast.UsesExportsRef = true
+				repr.meta.forceIncludeExportsForEntryPoint = true
+			}
 		}
 	}
 

internal/config/config.go

@@ -6,6 +6,7 @@ import (
 	"sync"
 
 	"github.com/evanw/esbuild/internal/compat"
+	"github.com/evanw/esbuild/internal/js_ast"
 	"github.com/evanw/esbuild/internal/logger"
 )
 
@@ -207,6 +208,7 @@ type Options struct {
 	OutputFormat       Format
 	PublicPath         string
 	InjectAbsPaths     []string
+	InjectedDefines    []InjectedDefine
 	InjectedFiles      []InjectedFile
 	Banner             string
 	Footer             string
@@ -222,10 +224,17 @@ type Options struct {
 	Stdin *StdinInfo
 }
 
+type InjectedDefine struct {
+	Source logger.Source
+	Data   js_ast.E
+	Name   string
+}
+
 type InjectedFile struct {
 	Path        string
 	SourceIndex uint32
 	Exports     []string
+	IsDefine    bool
 }
 
 var filterMutex sync.Mutex

internal/config/globals.go

@@ -113,8 +113,9 @@ var knownGlobals = [][]string{
 }
 
 type DefineArgs struct {
-	Loc        logger.Loc
-	FindSymbol func(logger.Loc, string) js_ast.Ref
+	Loc             logger.Loc
+	FindSymbol      func(logger.Loc, string) js_ast.Ref
+	SymbolForDefine func(int) js_ast.Ref
 }
 
 type DefineFunc func(DefineArgs) js_ast.E

internal/js_ast/js_ast.go

@@ -1654,6 +1654,10 @@ func GenerateNonUniqueNameFromPath(path string) string {
 		}
 	}
 
+	return EnsureValidIdentifier(base)
+}
+
+func EnsureValidIdentifier(base string) string {
 	// Convert it to an ASCII identifier. Note: If you change this to a non-ASCII
 	// identifier, you're going to potentially cause trouble with non-BMP code
 	// points in target environments that don't support bracketed Unicode escapes.

internal/js_parser/js_parser.go

@@ -57,6 +57,8 @@ type parser struct {
 	importMetaRef            js_ast.Ref
 	promiseRef               js_ast.Ref
 	findSymbolHelper         func(loc logger.Loc, name string) js_ast.Ref
+	symbolForDefineHelper    func(int) js_ast.Ref
+	injectedDefineSymbols    []js_ast.Ref
 	symbolUses               map[js_ast.Ref]js_ast.SymbolUse
 	declaredSymbols          []js_ast.DeclaredSymbol
 	runtimeImports           map[string]js_ast.Ref
@@ -9987,8 +9989,9 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
 
 func (p *parser) valueForDefine(loc logger.Loc, assignTarget js_ast.AssignTarget, isDeleteTarget bool, defineFunc config.DefineFunc) js_ast.Expr {
 	expr := js_ast.Expr{Loc: loc, Data: defineFunc(config.DefineArgs{
-		Loc:        loc,
-		FindSymbol: p.findSymbolHelper,
+		Loc:             loc,
+		FindSymbol:      p.findSymbolHelper,
+		SymbolForDefine: p.symbolForDefineHelper,
 	})}
 	if id, ok := expr.Data.(*js_ast.EIdentifier); ok {
 		return p.handleIdentifier(loc, assignTarget, isDeleteTarget, id)
@@ -10976,7 +10979,16 @@ func newParser(log logger.Log, source logger.Source, lexer js_lexer.Lexer, optio
 		namedExports:            make(map[string]js_ast.NamedExport),
 	}
 
-	p.findSymbolHelper = func(loc logger.Loc, name string) js_ast.Ref { return p.findSymbol(loc, name).ref }
+	p.findSymbolHelper = func(loc logger.Loc, name string) js_ast.Ref {
+		return p.findSymbol(loc, name).ref
+	}
+
+	p.symbolForDefineHelper = func(index int) js_ast.Ref {
+		ref := p.injectedDefineSymbols[index]
+		p.recordUsage(ref)
+		return ref
+	}
+
 	p.pushScopeForParsePass(js_ast.ScopeEntry, logger.Loc{Start: locModuleScope})
 
 	return p
@@ -11058,12 +11070,20 @@ func Parse(log logger.Log, source logger.Source, options Options) (result js_ast
 	for _, file := range p.options.injectedFiles {
 		exportsNoConflict := make([]string, 0, len(file.Exports))
 		symbols := make(map[string]js_ast.Ref)
-		for _, alias := range file.Exports {
-			if _, ok := p.moduleScope.Members[alias]; !ok {
-				ref := p.newSymbol(js_ast.SymbolOther, alias)
-				p.moduleScope.Members[alias] = js_ast.ScopeMember{Ref: ref}
-				symbols[alias] = ref
-				exportsNoConflict = append(exportsNoConflict, alias)
+		if file.IsDefine {
+			ref := p.newSymbol(js_ast.SymbolOther, js_ast.GenerateNonUniqueNameFromPath(file.Path))
+			p.moduleScope.Generated = append(p.moduleScope.Generated, ref)
+			symbols["default"] = ref
+			exportsNoConflict = append(exportsNoConflict, "default")
+			p.injectedDefineSymbols = append(p.injectedDefineSymbols, ref)
+		} else {
+			for _, alias := range file.Exports {
+				if _, ok := p.moduleScope.Members[alias]; !ok {
+					ref := p.newSymbol(js_ast.SymbolOther, alias)
+					p.moduleScope.Members[alias] = js_ast.ScopeMember{Ref: ref}
+					symbols[alias] = ref
+					exportsNoConflict = append(exportsNoConflict, alias)
+				}
 			}
 		}
 		before = p.generateImportStmt(file.Path, exportsNoConflict, file.SourceIndex, before, symbols)

pkg/api/api_impl.go

@@ -316,12 +316,14 @@ func validateJSX(log logger.Log, text string, name string) []string {
 	return parts
 }
 
-func validateDefines(log logger.Log, defines map[string]string, pureFns []string) *config.ProcessedDefines {
+func validateDefines(log logger.Log, defines map[string]string, pureFns []string) (*config.ProcessedDefines, []config.InjectedDefine) {
 	if len(defines) == 0 && len(pureFns) == 0 {
-		return nil
+		return nil, nil
 	}
 
 	rawDefines := make(map[string]config.DefineData)
+	valueToInject := make(map[string]config.InjectedDefine)
+	var definesToInject []string
 
 	for key, value := range defines {
 		// The key must be a dot-separated identifier list
@@ -359,9 +361,9 @@ func validateDefines(log logger.Log, defines map[string]string, pureFns []string
 			continue
 		}
 
-		// Only allow atoms for now
 		var fn config.DefineFunc
 		switch e := expr.Data.(type) {
+		// These values are inserted inline, and can participate in constant folding
 		case *js_ast.ENull:
 			fn = func(config.DefineArgs) js_ast.E { return &js_ast.ENull{} }
 		case *js_ast.EBoolean:
@@ -370,6 +372,13 @@ func validateDefines(log logger.Log, defines map[string]string, pureFns []string
 			fn = func(config.DefineArgs) js_ast.E { return &js_ast.EString{Value: e.Value} }
 		case *js_ast.ENumber:
 			fn = func(config.DefineArgs) js_ast.E { return &js_ast.ENumber{Value: e.Value} }
+
+		// These values are extracted into a shared symbol reference
+		case *js_ast.EArray, *js_ast.EObject:
+			definesToInject = append(definesToInject, key)
+			valueToInject[key] = config.InjectedDefine{Source: source, Data: e, Name: key}
+			continue
+
 		default:
 			log.AddError(nil, logger.Loc{}, fmt.Sprintf("Invalid define value: %q", value))
 			continue
@@ -378,6 +387,18 @@ func validateDefines(log logger.Log, defines map[string]string, pureFns []string
 		rawDefines[key] = config.DefineData{DefineFunc: fn}
 	}
 
+	// Sort injected defines for determinism, since the imports will be injected
+	// into every file in the order that we return them from this function
+	injectedDefines := make([]config.InjectedDefine, len(definesToInject))
+	sort.Strings(definesToInject)
+	for i, key := range definesToInject {
+		index := i // Capture this for the closure below
+		injectedDefines[i] = valueToInject[key]
+		rawDefines[key] = config.DefineData{DefineFunc: func(args config.DefineArgs) js_ast.E {
+			return &js_ast.EIdentifier{Ref: args.SymbolForDefine(index)}
+		}}
+	}
+
 	for _, key := range pureFns {
 		// The key must be a dot-separated identifier list
 		for _, part := range strings.Split(key, ".") {
@@ -396,7 +417,7 @@ func validateDefines(log logger.Log, defines map[string]string, pureFns []string
 	// Processing defines is expensive. Process them once here so the same object
 	// can be shared between all parsers we create using these arguments.
 	processed := config.ProcessDefines(rawDefines)
-	return &processed
+	return &processed, injectedDefines
 }
 
 func validatePath(log logger.Log, fs fs.FS, relPath string) string {
@@ -514,14 +535,16 @@ func rebuildImpl(
 	realFS := fs.RealFS()
 	jsFeatures, cssFeatures := validateFeatures(log, buildOpts.Target, buildOpts.Engines)
 	outJS, outCSS := validateOutputExtensions(log, buildOpts.OutExtensions)
+	defines, injectedDefines := validateDefines(log, buildOpts.Define, buildOpts.Pure)
 	options := config.Options{
 		UnsupportedJSFeatures:  jsFeatures,
 		UnsupportedCSSFeatures: cssFeatures,
 		JSX: config.JSXOptions{
 			Factory:  validateJSX(log, buildOpts.JSXFactory, "factory"),
 			Fragment: validateJSX(log, buildOpts.JSXFragment, "fragment"),
 		},
-		Defines:               validateDefines(log, buildOpts.Define, buildOpts.Pure),
+		Defines:               defines,
+		InjectedDefines:       injectedDefines,
 		Platform:              validatePlatform(buildOpts.Platform),
 		SourceMap:             validateSourceMap(buildOpts.Sourcemap),
 		ExcludeSourcesContent: buildOpts.SourcesContent == SourcesContentExclude,
@@ -766,11 +789,13 @@ func transformImpl(input string, transformOpts TransformOptions) TransformResult
 
 	// Convert and validate the transformOpts
 	jsFeatures, cssFeatures := validateFeatures(log, transformOpts.Target, transformOpts.Engines)
+	defines, injectedDefines := validateDefines(log, transformOpts.Define, transformOpts.Pure)
 	options := config.Options{
 		UnsupportedJSFeatures:   jsFeatures,
 		UnsupportedCSSFeatures:  cssFeatures,
 		JSX:                     jsx,
-		Defines:                 validateDefines(log, transformOpts.Define, transformOpts.Pure),
+		Defines:                 defines,
+		InjectedDefines:         injectedDefines,
 		SourceMap:               validateSourceMap(transformOpts.Sourcemap),
 		ExcludeSourcesContent:   transformOpts.SourcesContent == SourcesContentExclude,
 		OutputFormat:            validateFormat(transformOpts.Format),

scripts/js-api-tests.js

@@ -171,6 +171,26 @@ let buildTests = {
     assert.strictEqual(require(output).result, 123)
   },
 
+  async defineObject({ esbuild, testDir }) {
+    const input = path.join(testDir, 'in.js');
+    const output = path.join(testDir, 'out.js')
+    await writeFileAsync(input, 'export default {abc, xyz}')
+    await esbuild.build({
+      entryPoints: [input],
+      outfile: output,
+      format: 'cjs',
+      bundle: true,
+      define: {
+        abc: '["a", "b", "c"]',
+        xyz: '{"x": 1, "y": 2, "z": 3}',
+      },
+    })
+    assert.deepStrictEqual(require(output).default, {
+      abc: ['a', 'b', 'c'],
+      xyz: { x: 1, y: 2, z: 3 },
+    })
+  },
+
   async inject({ esbuild, testDir }) {
     const input = path.join(testDir, 'in.js');
     const inject = path.join(testDir, 'inject.js')
@@ -1879,6 +1899,12 @@ let transformTests = {
     assert.strictEqual(code, `console.log("production");\n`)
   },
 
+  async defineArray({ service }) {
+    const define = { 'process.env.NODE_ENV': '[1,2,3]', 'something.else': '[2,3,4]' }
+    const { code } = await service.transform(`console.log(process.env.NODE_ENV)`, { define })
+    assert.strictEqual(code, `var define_process_env_NODE_ENV_default = [1, 2, 3];\nconsole.log(define_process_env_NODE_ENV_default);\n`)
+  },
+
   async defineWarning({ service }) {
     const define = { 'process.env.NODE_ENV': 'production' }
     const { code, warnings } = await service.transform(`console.log(process.env.NODE_ENV)`, { define })