remove cross-file variable splicing (#638)

CHANGELOG.md

@@ -20,9 +20,13 @@
 
     * Minification now takes advantage of the left-associativity of certain operators. This means `a && (b && c)` turns into `a && b && c`.
 
-* Fix issues with source maps ([#638][https://github.com/evanw/esbuild/issues/638])
+* Fix issues with nested source maps ([#638][https://github.com/evanw/esbuild/issues/638])
 
-    * Generated source maps were incorrect when an input file had a nested source map (i.e. contained a valid `//# sourceMappingURL=` comment) and the input source map had more than one source file. This regression was introduced by an optimization in version 0.8.25 that parallelizes the generation of certain internal source map data structures. The index into the generated `sources` array was incorrectly incremented by 1 for every input file instead of by the number of sources in the input source map. This issue has been fixed and now has test coverage.
+    A nested source map happens when an input file has a valid `//# sourceMappingURL=` comment that points to a valid source map file. In that case, esbuild will read that source map and use it to map back to the original source code from the generated file. This only happens if you enable source map generation in esbuild via `--sourcemap`. This release fixes the following issues:
+
+    * Generated source maps were incorrect when an input file had a nested source map and the input source map had more than one source file. This regression was introduced by an optimization in version 0.8.25 that parallelizes the generation of certain internal source map data structures. The index into the generated `sources` array was incorrectly incremented by 1 for every input file instead of by the number of sources in the input source map. This issue has been fixed and now has test coverage.
+
+    * Generated source maps were incorrect when an input file had a nested source map, the file starts with a local variable, the previous file ends with a local variable of that same type, and the input source map is missing a mapping at the start of the file. An optimization was added in version 0.7.18 that splices together local variable declarations from separate files when they end up adjacent to each other in the generated output file (i.e. `var a=0;var b=2;` becomes `var a=0,b=2;` when `a` and `b` are in separate files). The source map splicing was expecting a mapping at the start of the file and that isn't necessarily the case when using nested source maps. The optimization has been disabled for now to fix source map generation, and this specific case has test coverage.
 
 ## 0.8.29
 

internal/bundler/linker.go

@@ -3160,12 +3160,6 @@ type compileResultJS struct {
 
 	sourceIndex uint32
 
-	// This is used during minification to merge local declarations across files
-	startsWithLocal         *js_ast.LocalKind
-	endsWithLocal           *js_ast.LocalKind
-	shouldMergeLocalsBefore bool
-	shouldMergeLocalsAfter  bool
-
 	// This is the line and column offset since the previous JavaScript string
 	// or the start of the file if this is the first JavaScript string.
 	generatedOffset lineColumnOffset
@@ -3314,14 +3308,6 @@ func (c *linkerContext) generateCodeForFileInChunkJS(
 		PrintResult: js_printer.Print(tree, c.symbols, r, printOptions),
 		sourceIndex: partRange.sourceIndex,
 	}
-	if len(stmts) > 0 {
-		if local, ok := stmts[0].Data.(*js_ast.SLocal); ok {
-			result.startsWithLocal = &local.Kind
-		}
-		if local, ok := stmts[len(stmts)-1].Data.(*js_ast.SLocal); ok {
-			result.endsWithLocal = &local.Kind
-		}
-	}
 
 	// Write this separately as the entry point tail so it can be split off
 	// from the main entry point code. This is sometimes required to deal with
@@ -3712,7 +3698,7 @@ func (repr *chunkReprJS) generate(c *linkerContext, chunk *chunkInfo) func([]ast
 			metaOrder = make([]string, 0, len(compileResults))
 			metaByteCount = make(map[string]int, len(compileResults))
 		}
-		for i, compileResult := range compileResults {
+		for _, compileResult := range compileResults {
 			isRuntime := compileResult.sourceIndex == runtime.SourceIndex
 			for text := range compileResult.ExtractedComments {
 				if !commentSet[text] {
@@ -3750,53 +3736,18 @@ func (repr *chunkReprJS) generate(c *linkerContext, chunk *chunkInfo) func([]ast
 				prevComment = compileResult.sourceIndex
 			}
 
-			js := compileResult.JS
-
-			// Merge leading and trailing local declarations together. So a file
-			// ending in "var a=1;" followed by a file starting with "var b=2;" would
-			// become "var a=1,b=2;" for better minification.
-			if c.options.MangleSyntax && compileResult.endsWithLocal != nil && i+1 < len(compileResults) {
-				if next := compileResults[i+1]; next.startsWithLocal != nil && *compileResult.endsWithLocal == *next.startsWithLocal {
-					compileResult.shouldMergeLocalsAfter = true
-					if n := len(js); js[n-1] == ';' {
-						js = js[:n-1]
-					} else if js[n-1] == '\n' && js[n-2] == ';' {
-						js[n-2] = '\n'
-						js = js[:n-1]
-					} else {
-						panic("Internal error")
-					}
-				}
-			}
-			if c.options.MangleSyntax && compileResult.startsWithLocal != nil && i > 0 {
-				if prev := compileResults[i-1]; prev.endsWithLocal != nil && *compileResult.startsWithLocal == *prev.endsWithLocal {
-					compileResult.shouldMergeLocalsBefore = true
-					bytes := make([]byte, 0, compileResult.FirstDeclByteOffset)
-					for js[len(bytes)] == ' ' {
-						bytes = append(bytes, ' ')
-					}
-					bytes = append(bytes, ',')
-					if !c.options.RemoveWhitespace {
-						bytes = append(bytes, ' ')
-					}
-					prevOffset.advanceBytes(bytes)
-					j.AddBytes(bytes)
-					js = js[compileResult.FirstDeclByteOffset:]
-				}
-			}
-
 			// Don't include the runtime in source maps
 			if isRuntime {
-				prevOffset.advanceString(string(js))
-				j.AddBytes(js)
+				prevOffset.advanceString(string(compileResult.JS))
+				j.AddBytes(compileResult.JS)
 			} else {
 				// Save the offset to the start of the stored JavaScript
 				compileResult.generatedOffset = prevOffset
-				j.AddBytes(js)
+				j.AddBytes(compileResult.JS)
 
 				// Ignore empty source map chunks
 				if compileResult.SourceMapChunk.ShouldIgnore {
-					prevOffset.advanceBytes(js)
+					prevOffset.advanceBytes(compileResult.JS)
 				} else {
 					prevOffset = lineColumnOffset{}
 
@@ -3811,10 +3762,10 @@ func (repr *chunkReprJS) generate(c *linkerContext, chunk *chunkInfo) func([]ast
 					// Accumulate file sizes since a given file may be split into multiple parts
 					path := c.files[compileResult.sourceIndex].source.PrettyPath
 					if count, ok := metaByteCount[path]; ok {
-						metaByteCount[path] = count + len(js)
+						metaByteCount[path] = count + len(compileResult.JS)
 					} else {
 						metaOrder = append(metaOrder, path)
-						metaByteCount[path] = len(js)
+						metaByteCount[path] = len(compileResult.JS)
 					}
 				}
 			}
@@ -4397,28 +4348,14 @@ func (c *linkerContext) generateSourceMapForChunk(
 			startState.GeneratedColumn += prevColumnOffset
 		}
 
-		buffer := chunk.Buffer
-
-		// Adjust the source map if we stripped off the leading variable
-		// declaration keyword (i.e. var/let/const) to splice files together
-		if result.shouldMergeLocalsBefore {
-			buffer = js_printer.RemovePrefixFromSourceMapChunk(buffer, int(result.FirstDeclSourceMapOffset))
-		}
-
 		// Append the precomputed source map chunk
-		js_printer.AppendSourceMapChunk(&j, prevEndState, startState, buffer)
+		js_printer.AppendSourceMapChunk(&j, prevEndState, startState, chunk.Buffer)
 
 		// Generate the relative offset to start from next time
 		prevEndState = chunk.EndState
 		prevEndState.SourceIndex += sourcesIndex
 		prevColumnOffset = chunk.FinalGeneratedColumn
 
-		// Adjust the source map if we stripped off the trailing semicolon
-		// after the last variable declaration statement when splicing
-		if result.shouldMergeLocalsAfter {
-			prevColumnOffset = chunk.FinalLocalSemi
-		}
-
 		// If this was all one line, include the column offset from the start
 		if prevEndState.GeneratedLine == 0 {
 			prevEndState.GeneratedColumn += startState.GeneratedColumn

internal/bundler/snapshots/snapshots_default.txt

@@ -1392,7 +1392,7 @@ console.log(shared_default);
 ================================================================================
 TestMinifiedBundleCommonJS
 ---------- /out.js ----------
-var n=e(r=>{r.foo=function(){return 123}}),u=e((j,t)=>{t.exports={test:!0}}),{foo:c}=n();console.log(c(),u());
+var n=e(r=>{r.foo=function(){return 123}});var u=e((j,t)=>{t.exports={test:!0}});var{foo:c}=n();console.log(c(),u());
 
 ================================================================================
 TestMinifiedBundleES6

internal/bundler/snapshots/snapshots_ts.txt

@@ -271,7 +271,7 @@ console.log(a, b, c, d, e, real);
 ================================================================================
 TestTSMinifiedBundleCommonJS
 ---------- /out.js ----------
-var n=e(r=>{r.foo=function(){return 123}}),u=e((j,t)=>{t.exports={test:!0}}),{foo:c}=n();console.log(c(),u());
+var n=e(r=>{r.foo=function(){return 123}});var u=e((j,t)=>{t.exports={test:!0}});var{foo:c}=n();console.log(c(),u());
 
 ================================================================================
 TestTSMinifiedBundleES6

internal/js_printer/js_printer.go

@@ -86,59 +86,6 @@ func AppendSourceMapChunk(j *Joiner, prevEndState SourceMapState, startState Sou
 	j.AddBytes(sourceMap)
 }
 
-// Rewrite the source map to remove everything before "offset" and have the
-// generated position start from (0, 0) at that point. This is used when
-// erasing the variable declaration keyword from the start of a file.
-func RemovePrefixFromSourceMapChunk(buffer []byte, offset int) []byte {
-	// Avoid an out-of-bounds array access if the offset is 0. This doesn't
-	// happen normally but can happen if a source is remapped from another
-	// source map that is missing mappings. This is the case with some file
-	// in the "ant-design" project.
-	if offset == 0 {
-		return buffer
-	}
-
-	state := SourceMapState{}
-	i := 0
-
-	// Accumulate mappings before the break point
-	for i < offset {
-		if buffer[i] == ';' {
-			i++
-			continue
-		}
-
-		var si, ol, oc int
-		_, i = sourcemap.DecodeVLQ(buffer, i)
-		si, i = sourcemap.DecodeVLQ(buffer, i)
-		ol, i = sourcemap.DecodeVLQ(buffer, i)
-		oc, i = sourcemap.DecodeVLQ(buffer, i)
-		state.SourceIndex += si
-		state.OriginalLine += ol
-		state.OriginalColumn += oc
-
-		if i < len(buffer) && buffer[i] == ',' {
-			i++
-		}
-	}
-
-	// Rewrite the mapping at the break point
-	var si, ol, oc int
-	_, i = sourcemap.DecodeVLQ(buffer, i)
-	si, i = sourcemap.DecodeVLQ(buffer, i)
-	ol, i = sourcemap.DecodeVLQ(buffer, i)
-	oc, i = sourcemap.DecodeVLQ(buffer, i)
-	state.SourceIndex += si
-	state.OriginalLine += ol
-	state.OriginalColumn += oc
-
-	// Splice it in front of the buffer assuming it's big enough
-	first := appendMapping(nil, 0, SourceMapState{}, state)
-	buffer = buffer[i-len(first):]
-	copy(buffer, first)
-	return buffer
-}
-
 func appendMapping(buffer []byte, lastByte byte, prevState SourceMapState, currentState SourceMapState) []byte {
 	// Put commas in between mappings
 	if lastByte != 0 && lastByte != ';' && lastByte != '"' {
@@ -532,12 +479,6 @@ type printer struct {
 	hasPrevState        bool
 	lineOffsetTables    []LineOffsetTable
 
-	// For splicing adjacent files together
-	finalLocalSemi           int
-	firstDeclByteOffset      uint32
-	firstDeclSourceMapOffset uint32
-	hasDecl                  bool
-
 	// This is a workaround for a bug in the popular "source-map" library:
 	// https://github.com/mozilla/source-map/issues/261. The library will
 	// sometimes return null when querying a source map unless every line
@@ -868,27 +809,16 @@ func (p *printer) printIdentifierUTF16(name []uint16) {
 	}
 }
 
-func (p *printer) addSourceMappingForDecl(loc logger.Loc) {
-	if !p.hasDecl {
-		p.hasDecl = true
-		p.firstDeclByteOffset = uint32(len(p.js))
-		p.firstDeclSourceMapOffset = uint32(len(p.sourceMap))
-		p.prevLoc.Start = -1 // Force a new source mapping to be generated
-		p.addSourceMapping(loc)
-	}
-}
-
 func (p *printer) printBinding(binding js_ast.Binding) {
+	p.addSourceMapping(binding.Loc)
+
 	switch b := binding.Data.(type) {
 	case *js_ast.BMissing:
 
 	case *js_ast.BIdentifier:
-		p.printSpaceBeforeIdentifier()
-		p.addSourceMappingForDecl(binding.Loc)
 		p.printSymbol(b.Ref)
 
 	case *js_ast.BArray:
-		p.addSourceMappingForDecl(binding.Loc)
 		p.print("[")
 		if len(b.Items) > 0 {
 			if !b.IsSingleLine {
@@ -933,7 +863,6 @@ func (p *printer) printBinding(binding js_ast.Binding) {
 		p.print("]")
 
 	case *js_ast.BObject:
-		p.addSourceMappingForDecl(binding.Loc)
 		p.print("{")
 		if len(b.Properties) > 0 {
 			if !b.IsSingleLine {
@@ -2333,8 +2262,6 @@ func (p *printer) printDeclStmt(isExport bool, keyword string, decls []js_ast.De
 		p.print("export ")
 	}
 	p.printDecls(keyword, decls, 0)
-	p.updateGeneratedLineAndColumn()
-	p.finalLocalSemi = p.generatedColumn
 	p.printSemicolonAfterStatement()
 }
 
@@ -3105,10 +3032,6 @@ type SourceMapChunk struct {
 	// we'll need to know how many characters were in the last line we generated.
 	FinalGeneratedColumn int
 
-	// Like "FinalGeneratedColumn" but for the generated column of the last
-	// semicolon for a "SLocal" statement.
-	FinalLocalSemi int
-
 	ShouldIgnore bool
 }
 
@@ -3121,10 +3044,6 @@ type PrintResult struct {
 	SourceMapChunk SourceMapChunk
 
 	ExtractedComments map[string]bool
-
-	// These are used when stripping off the leading variable declaration
-	FirstDeclByteOffset      uint32
-	FirstDeclSourceMapOffset uint32
 }
 
 func Print(tree js_ast.AST, symbols js_ast.SymbolMap, r renamer.Renamer, options Options) PrintResult {
@@ -3172,10 +3091,7 @@ func Print(tree js_ast.AST, symbols js_ast.SymbolMap, r renamer.Renamer, options
 			Buffer:               p.sourceMap,
 			EndState:             p.prevState,
 			FinalGeneratedColumn: p.generatedColumn,
-			FinalLocalSemi:       p.finalLocalSemi,
 			ShouldIgnore:         p.shouldIgnoreSourceMap(),
 		},
-		FirstDeclByteOffset:      p.firstDeclByteOffset,
-		FirstDeclSourceMapOffset: p.firstDeclSourceMapOffset,
 	}
 }

scripts/verify-source-map.js

@@ -241,6 +241,7 @@ const testCaseComplex = {
 }
 
 const toSearchComplex = {
+  '[object Array]': '../../node_modules/fuse.js/dist/webpack:/src/helpers/is_array.js',
   'Score average:': '../../node_modules/fuse.js/dist/webpack:/src/index.js',
   '0123456789': '../../node_modules/object-assign/index.js',
   'forceUpdate': '../../node_modules/react/cjs/react.production.min.js',