fix #3115: pass through unknown js directives

CHANGELOG.md

@@ -2,6 +2,23 @@
 
 ## Unreleased
 
+* Avoid removing unrecognized directives from the directive prologue when minifying ([#3115](https://github.com/evanw/esbuild/issues/3115))
+
+    The [directive prologue](https://262.ecma-international.org/6.0/#sec-directive-prologues-and-the-use-strict-directive) in JavaScript is a sequence of top-level string expressions that come before your code. The only directives that JavaScript engines currently recognize are `use strict` and sometimes `use asm`. However, the people behind React have made up their own directive for their own custom dialect of JavaScript. Previously esbuild only preserved the `use strict` directive when minifying, although you could still write React JavaScript with esbuild using something like `--banner:js="'your directive here';"`. With this release, you can now put arbitrary directives in the entry point and esbuild will preserve them in its minified output:
+
+    ```js
+    // Original code
+    'use wtf'; console.log(123)
+
+    // Old output (with --minify)
+    console.log(123);
+
+    // New output (with --minify)
+    "use wtf";console.log(123);
+    ```
+
+    Note that this means esbuild will no longer remove certain stray top-level strings when minifying. This behavior is an intentional change because these stray top-level strings are actually part of the directive prologue, and could potentially have semantics assigned to them (as was the case with React).
+
 * Improved minification of binary shift operators
 
     With this release, esbuild's minifier will now evaluate the `<<` and `>>>` operators if the resulting code would be shorter:

internal/bundler_tests/bundler_dce_test.go

@@ -3645,3 +3645,105 @@ func TestTreeShakingJSWithAssociatedCSSUnusedNestedImportSideEffectsFalseOnlyJS(
 		},
 	})
 }
+
+func TestPreserveDirectivesMinifyPassThrough(t *testing.T) {
+	dce_suite.expectBundled(t, bundled{
+		files: map[string]string{
+			"/entry.js": `
+				//! 1
+				'use 1'
+				//! 2
+				'use 2'
+				//! 3
+				'use 3'
+				entry()
+				//! 4
+				'use 4'
+				//! 5
+				'use 5'
+				//! 6
+				'use 6'
+			`,
+		},
+		entryPaths: []string{"/entry.js"},
+		options: config.Options{
+			Mode:          config.ModePassThrough,
+			AbsOutputFile: "/out.js",
+			MinifySyntax:  true,
+		},
+	})
+}
+
+func TestPreserveDirectivesMinifyIIFE(t *testing.T) {
+	dce_suite.expectBundled(t, bundled{
+		files: map[string]string{
+			"/entry.js": `
+				//! 1
+				'use 1'
+				//! 2
+				'use 2'
+				//! 3
+				'use 3'
+				entry()
+				//! 4
+				'use 4'
+				//! 5
+				'use 5'
+				//! 6
+				'use 6'
+			`,
+		},
+		entryPaths: []string{"/entry.js"},
+		options: config.Options{
+			Mode:          config.ModeConvertFormat,
+			OutputFormat:  config.FormatIIFE,
+			AbsOutputFile: "/out.js",
+			MinifySyntax:  true,
+		},
+	})
+}
+
+func TestPreserveDirectivesMinifyBundle(t *testing.T) {
+	dce_suite.expectBundled(t, bundled{
+		files: map[string]string{
+			"/entry.js": `
+				//! 1
+				'use 1'
+				//! 2
+				'use 2'
+				//! 3
+				'use 3'
+				entry()
+				//! 4
+				'use 4'
+				//! 5
+				'use 5'
+				//! 6
+				'use 6'
+				import "./nested.js"
+			`,
+			"/nested.js": `
+				//! A
+				'use A'
+				//! B
+				'use B'
+				//! C
+				'use C'
+				nested()
+				//! D
+				'use D'
+				//! E
+				'use E'
+				//! F
+				'use F'
+			`,
+		},
+		entryPaths: []string{"/entry.js"},
+		options: config.Options{
+			Mode:          config.ModeBundle,
+			OutputFormat:  config.FormatIIFE,
+			AbsOutputFile: "/out.js",
+			MinifySyntax:  true,
+		},
+	})
+}

internal/bundler_tests/snapshots/snapshots_dce.txt

@@ -197,6 +197,7 @@ var ns;
 
 ---------- /out/directive-before.js ----------
 function nested() {
+  "directive";
   x = [1, 1];
 }
 
@@ -1505,6 +1506,62 @@ console.log("hello");
 // Users/user/project/src/entry.js
 console.log("unused import");
 
+================================================================================
+TestPreserveDirectivesMinifyBundle
+---------- /out.js ----------
+"use 1";
+"use 2";
+"use 3";
+(() => {
+  // nested.js
+  //! A
+  //! B
+  //! C
+  nested();
+  //! D
+  //! E
+  //! F
+
+  // entry.js
+  //! 1
+  //! 2
+  //! 3
+  entry();
+  //! 4
+  //! 5
+  //! 6
+})();
+
+================================================================================
+TestPreserveDirectivesMinifyIIFE
+---------- /out.js ----------
+"use 1";
+"use 2";
+"use 3";
+(() => {
+  //! 1
+  //! 2
+  //! 3
+  entry();
+  //! 4
+  //! 5
+  //! 6
+})();
+
+================================================================================
+TestPreserveDirectivesMinifyPassThrough
+---------- /out.js ----------
+"use 1";
+"use 2";
+"use 3";
+//! 1
+//! 2
+//! 3
+entry();
+//! 4
+//! 5
+//! 6
+
 ================================================================================
 TestPureCallsWithSpread
 ---------- /out.js ----------

internal/bundler_tests/snapshots/snapshots_default.txt

@@ -5896,7 +5896,7 @@ TestUseStrictDirectiveBundleIssue1837
 ================================================================================
 TestUseStrictDirectiveMinifyNoBundle
 ---------- /out.js ----------
-"use strict";a,b;
+"use strict";"use loose";a,b;
 
 ================================================================================
 TestVarRelocatingBundle

internal/js_ast/js_ast.go

@@ -1841,9 +1841,9 @@ type AST struct {
 	// This is internal-only data used for the implementation of Yarn PnP
 	ManifestForYarnPnP Expr
 
-	Hashbang  string
-	Directive string
-	URLForCSS string
+	Hashbang   string
+	Directives []string
+	URLForCSS  string
 
 	// Note: If you're in the linker, do not use this map directly. This map is
 	// filled in by the parser and is considered immutable. For performance reasons,

internal/js_parser/js_parser.go

@@ -8181,15 +8181,6 @@ func (p *parser) visitStmts(stmts []js_ast.Stmt, kind stmtsKind) []js_ast.Stmt {
 	return p.mangleStmts(visited, kind)
 }
 
-func isDirectiveSupported(s *js_ast.SDirective) bool {
-	// When minifying, strip all directives other than "use strict" since
-	// that should be the only one that is ever really used by engines in
-	// practice. We don't support "use asm" even though that's also
-	// technically used in practice because the rest of our minifier would
-	// likely cause asm.js code to fail validation anyway.
-	return helpers.UTF16EqualsString(s.Value, "use strict")
-}
-
 func (p *parser) mangleStmts(stmts []js_ast.Stmt, kind stmtsKind) []js_ast.Stmt {
 	// Remove inlined constants now that we know whether any of these statements
 	// contained a direct eval() or not. This can't be done earlier when we
@@ -8314,11 +8305,6 @@ func (p *parser) mangleStmts(stmts []js_ast.Stmt, kind stmtsKind) []js_ast.Stmt
 			// Strip empty statements
 			continue
 
-		case *js_ast.SDirective:
-			if !isDirectiveSupported(s) {
-				continue
-			}
-
 		case *js_ast.SLocal:
 			// Merge adjacent local statements
 			if len(result) > 0 {
@@ -15786,26 +15772,54 @@ func Parse(log logger.Log, source logger.Source, options Options) (result js_ast
 	p.prepareForVisitPass()
 
 	// Insert a "use strict" directive if "alwaysStrict" is active
-	directive := ""
+	var directives []string
 	if tsAlwaysStrict := p.options.tsAlwaysStrict; tsAlwaysStrict != nil && tsAlwaysStrict.Value {
-		directive = "use strict"
+		directives = append(directives, "use strict")
 	}
 
-	// Strip off a leading "use strict" directive when not bundling
-	for i, stmt := range stmts {
-		switch s := stmt.Data.(type) {
-		case *js_ast.SComment:
-			continue
-		case *js_ast.SDirective:
-			if !isDirectiveSupported(s) {
+	// Strip off all leading directives
+	{
+		totalCount := 0
+		keptCount := 0
+
+		for _, stmt := range stmts {
+			switch s := stmt.Data.(type) {
+			case *js_ast.SComment:
+				stmts[keptCount] = stmt
+				keptCount++
+				totalCount++
+				continue
+
+			case *js_ast.SDirective:
+				if p.isStrictMode() && s.LegacyOctalLoc.Start > 0 {
+					p.markStrictModeFeature(legacyOctalEscape, p.source.RangeOfLegacyOctalEscape(s.LegacyOctalLoc), "")
+				}
+				directive := helpers.UTF16ToString(s.Value)
+
+				// Remove duplicate directives
+				found := false
+				for _, existing := range directives {
+					if existing == directive {
+						found = true
+						break
+					}
+				}
+				if !found {
+					directives = append(directives, directive)
+				}
+
+				// Remove this directive from the statement list
+				totalCount++
 				continue
 			}
-			directive = helpers.UTF16ToString(s.Value)
 
-			// Remove this directive from the statement list
-			stmts = append(stmts[:i], stmts[i+1:]...)
+			// Stop when the directive prologue ends
+			break
+		}
+
+		if keptCount < totalCount {
+			stmts = append(stmts[:keptCount], stmts[totalCount:]...)
 		}
-		break
 	}
 
 	// Add an empty part for the namespace export that we can fill in later
@@ -15985,7 +15999,7 @@ func Parse(log logger.Log, source logger.Source, options Options) (result js_ast
 	// Pop the module scope to apply the "ContainsDirectEval" rules
 	p.popScope()
 
-	result = p.toAST(before, parts, after, hashbang, directive)
+	result = p.toAST(before, parts, after, hashbang, directives)
 	result.SourceMapComment = p.lexer.SourceMappingURL
 	return
 }
@@ -16016,7 +16030,7 @@ func LazyExportAST(log logger.Log, source logger.Source, options Options, expr j
 	}
 	p.symbolUses = nil
 
-	ast := p.toAST(nil, []js_ast.Part{nsExportPart, part}, nil, "", "")
+	ast := p.toAST(nil, []js_ast.Part{nsExportPart, part}, nil, "", nil)
 	ast.HasLazyExport = true
 	return ast
 }
@@ -16399,7 +16413,7 @@ func sortedKeysOfMapStringLocRef(in map[string]js_ast.LocRef) []string {
 	return keys
 }
 
-func (p *parser) toAST(before, parts, after []js_ast.Part, hashbang string, directive string) js_ast.AST {
+func (p *parser) toAST(before, parts, after []js_ast.Part, hashbang string, directives []string) js_ast.AST {
 	// Insert an import statement for any runtime imports we generated
 	if len(p.runtimeImports) > 0 && !p.options.omitRuntimeForTests {
 		keys := sortedKeysOfMapStringLocRef(p.runtimeImports)
@@ -16603,7 +16617,7 @@ func (p *parser) toAST(before, parts, after []js_ast.Part, hashbang string, dire
 		ModuleRef:                       p.moduleRef,
 		WrapperRef:                      wrapperRef,
 		Hashbang:                        hashbang,
-		Directive:                       directive,
+		Directives:                      directives,
 		NamedImports:                    p.namedImports,
 		NamedExports:                    p.namedExports,
 		TSEnums:                         p.tsEnums,

internal/js_parser/js_parser_test.go

@@ -4329,10 +4329,10 @@ func TestMangleUnused(t *testing.T) {
 	expectPrintedNormalAndMangle(t, "true", "true;\n", "")
 	expectPrintedNormalAndMangle(t, "123", "123;\n", "")
 	expectPrintedNormalAndMangle(t, "123n", "123n;\n", "")
-	expectPrintedNormalAndMangle(t, "'abc'", "\"abc\";\n", "")        // Technically a directive, not a string expression
-	expectPrintedNormalAndMangle(t, "0; 'abc'", "0;\n\"abc\";\n", "") // Actually a string expression
-	expectPrintedNormalAndMangle(t, "'abc'; 'use strict'", "\"use strict\";\n\"abc\";\n", "\"use strict\";\n")
-	expectPrintedNormalAndMangle(t, "function f() { 'abc'; 'use strict' }", "function f() {\n  \"abc\";\n  \"use strict\";\n}\n", "function f() {\n  \"use strict\";\n}\n")
+	expectPrintedNormalAndMangle(t, "'abc'", "\"abc\";\n", "\"abc\";\n") // Technically a directive, not a string expression
+	expectPrintedNormalAndMangle(t, "0; 'abc'", "0;\n\"abc\";\n", "")    // Actually a string expression
+	expectPrintedNormalAndMangle(t, "'abc'; 'use strict'", "\"abc\";\n\"use strict\";\n", "\"abc\";\n\"use strict\";\n")
+	expectPrintedNormalAndMangle(t, "function f() { 'abc'; 'use strict' }", "function f() {\n  \"abc\";\n  \"use strict\";\n}\n", "function f() {\n  \"abc\";\n  \"use strict\";\n}\n")
 	expectPrintedNormalAndMangle(t, "this", "this;\n", "")
 	expectPrintedNormalAndMangle(t, "/regex/", "/regex/;\n", "")
 	expectPrintedNormalAndMangle(t, "(function() {})", "(function() {\n});\n", "")

internal/js_printer/js_printer.go

@@ -4608,9 +4608,9 @@ func Print(tree js_ast.AST, symbols js_ast.SymbolMap, r renamer.Renamer, options
 	}
 
 	// Add the top-level directive if present
-	if tree.Directive != "" {
+	for _, directive := range tree.Directives {
 		p.printIndent()
-		p.printQuotedUTF8(tree.Directive, options.ASCIIOnly)
+		p.printQuotedUTF8(directive, options.ASCIIOnly)
 		p.print(";")
 		p.printNewline()
 	}