add strict option for nullish coalescing

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## Unreleased
+
+* Add the `--strict` option
+
+    This currently only affects the transform for the `??` nullish coalescing operator. In loose mode (the default), `a ?? b` is transformed to `a != null ? a : b`. This works fine in all cases except when `a` is the special object `document.all`. In strict mode, `a ?? b` is transformed to `a !== null && a !== void 0 ? a : b` which works correctly with `document.all`. Enable `--strict` if you need to use `document.all` with the `??` operator.
+
 ## 0.5.8
 
 * Transform async functions ([#137](https://github.com/evanw/esbuild/issues/137))

README.md

@@ -113,6 +113,18 @@ These syntax features are conditionally transformed for older browsers depending
 | [Private static fields](https://github.com/tc39/proposal-static-class-features)     | `esnext`                             | `class { static #x }`      |
 | [Logical assignment operators](https://github.com/tc39/proposal-logical-assignment) | `esnext`                             | `a ??= b`                  |
 
+<details>
+<summary>Syntax transform caveats (click to expand)</summary><br>
+
+* **Nullish coalescing correctness**
+
+    By default `a ?? b` is transformed into `a != null ? a : b`, which works because `a != null` is only false if `a` is `null` or `undefined`. However, there's exactly one obscure edge case where this doesn't work. For legacy reasons, the value of `document.all` is special-cased such that `document.all != null` is false. If you need to use this value with the nullish coalescing operator, you should enable `--strict` transforms so `a ?? b` becomes `a !== null && a !== void 0 ? a : b` instead, which works correctly with `document.all`. The strict transform isn't done by default because it causes code bloat for an obscure edge case that shouldn't matter in modern code.
+
+* **Private member performance**
+
+    This transform uses `WeakMap` and `WeakSet` to preserve the privacy properties of this feature, similar to the corresponding transforms in the Babel and TypeScript compilers. Most modern JavaScript engines (V8, JavaScriptCore, and SpiderMonkey but not ChakraCore) may not have good performance characteristics for large `WeakMap` and `WeakSet` objects. Creating many instances of classes with private fields or private methods with this syntax transform active may cause a lot of overhead for the garbage collector. This is because modern engines (other than ChakraCore) store weak values in an actual map object instead of as hidden properties on the keys themselves, and large map objects can cause performance issues with garbage collection. See [this reference](https://github.com/tc39/ecma262/issues/1657#issuecomment-518916579) for more information.
+</details>
+
 These syntax features are currently always passed through un-transformed:
 
 | Syntax transform                                                                    | Unsupported when `--target` is below | Example                     |
@@ -299,6 +311,7 @@ Advanced options:
   --log-level=...           Disable logging (info, warning, error, silent)
   --resolve-extensions=...  A comma-separated list of implicit extensions
   --metafile=...            Write metadata about the build to a JSON file
+  --strict                  Transforms handle edge cases but have more overhead
 
   --trace=...           Write a CPU trace to this file
   --cpuprofile=...      Write a CPU profile to this file

cmd/esbuild/main.go

@@ -49,6 +49,7 @@ Advanced options:
   --log-level=...           Disable logging (info, warning, error, silent)
   --resolve-extensions=...  A comma-separated list of implicit extensions
   --metafile=...            Write metadata about the build to a JSON file
+  --strict                  Transforms handle edge cases but have more overhead
 
   --trace=...           Write a CPU trace to this file
   --cpuprofile=...      Write a CPU profile to this file

internal/parser/parser.go

@@ -7749,8 +7749,8 @@ func (p *parser) visitExprInOut(expr ast.Expr, in exprIn) (ast.Expr, exprOut) {
 				return e.Right, exprOut{}
 
 			default:
-				if p.Target < ES2020 {
-					return p.lowerNullishCoalescing(expr.Loc, e), exprOut{}
+				if p.Target < nullishCoalescingTarget {
+					return p.lowerNullishCoalescing(expr.Loc, e.Left, e.Right), exprOut{}
 				}
 			}
 
@@ -8932,13 +8932,26 @@ const (
 	PlatformNode
 )
 
+type StrictOptions struct {
+	// Loose:  "a ?? b" => "a != null ? a : b"
+	// Strict: "a ?? b" => "a !== null && a !== void 0 ? a : b"
+	//
+	// The disadvantage of strictness here is code bloat. The only observable
+	// difference between the two is when the left operand is the bizarre legacy
+	// value "document.all". This value is special-cased in the standard for
+	// legacy reasons such that "document.all != null" is false even though it's
+	// not "null" or "undefined".
+	NullishCoalescing bool
+}
+
 type ParseOptions struct {
 	// true: imports are scanned and bundled along with the file
 	// false: imports are left alone and the file is passed through as-is
 	IsBundling bool
 
-	Defines      *ProcessedDefines
 	MangleSyntax bool
+	Strict       StrictOptions
+	Defines      *ProcessedDefines
 	TS           TypeScriptOptions
 	JSX          JSXOptions
 	Target       LanguageTarget

internal/parser/parser_lower.go

@@ -13,6 +13,7 @@ import (
 
 const asyncAwaitTarget = ES2017
 const objectPropertyBindingTarget = ES2018
+const nullishCoalescingTarget = ES2020
 const privateNameTarget = ESNext
 
 type futureSyntax uint8
@@ -462,20 +463,12 @@ func (p *parser) lowerExponentiationAssignmentOperator(loc ast.Loc, e *ast.EBina
 
 func (p *parser) lowerNullishCoalescingAssignmentOperator(loc ast.Loc, e *ast.EBinary) ast.Expr {
 	if target, privateLoc, private := p.extractPrivateIndex(e.Left); private != nil {
-		if p.Target < ES2020 {
+		if p.Target < nullishCoalescingTarget {
 			// "a.#b ??= c" => "(_a = __privateGet(a, #b)) != null ? _a : __privateSet(a, #b, c)"
 			targetFunc, targetWrapFunc := p.captureValueWithPossibleSideEffects(loc, 2, target)
-			testFunc, testWrapFunc := p.captureValueWithPossibleSideEffects(loc, 2,
-				p.lowerPrivateGet(targetFunc(), privateLoc, private))
-			return testWrapFunc(targetWrapFunc(ast.Expr{loc, &ast.EIf{
-				Test: ast.Expr{loc, &ast.EBinary{
-					Op:    ast.BinOpLooseNe,
-					Left:  testFunc(),
-					Right: ast.Expr{loc, &ast.ENull{}},
-				}},
-				Yes: testFunc(),
-				No:  p.lowerPrivateSet(targetFunc(), privateLoc, private, e.Right),
-			}}))
+			left := p.lowerPrivateGet(targetFunc(), privateLoc, private)
+			right := p.lowerPrivateSet(targetFunc(), privateLoc, private, e.Right)
+			return targetWrapFunc(p.lowerNullishCoalescing(loc, left, right))
 		}
 
 		// "a.#b ??= c" => "__privateGet(a, #b) ?? __privateSet(a, #b, c)"
@@ -488,17 +481,12 @@ func (p *parser) lowerNullishCoalescingAssignmentOperator(loc ast.Loc, e *ast.EB
 	}
 
 	return p.lowerAssignmentOperator(e.Left, func(a ast.Expr, b ast.Expr) ast.Expr {
-		if p.Target < ES2020 {
+		if p.Target < nullishCoalescingTarget {
 			// "a ??= b" => "(_a = a) != null ? _a : a = b"
-			testFunc, testWrapFunc := p.captureValueWithPossibleSideEffects(a.Loc, 2, a)
-			return testWrapFunc(ast.Expr{loc, &ast.EIf{
-				Test: ast.Expr{loc, &ast.EBinary{
-					Op:    ast.BinOpLooseNe,
-					Left:  testFunc(),
-					Right: ast.Expr{loc, &ast.ENull{}},
-				}},
-				Yes: testFunc(),
-				No:  ast.Expr{loc, &ast.EBinary{Op: ast.BinOpAssign, Left: b, Right: e.Right}},
+			return p.lowerNullishCoalescing(loc, a, ast.Expr{loc, &ast.EBinary{
+				Op:    ast.BinOpAssign,
+				Left:  b,
+				Right: e.Right,
 			}})
 		}
 
@@ -534,18 +522,41 @@ func (p *parser) lowerLogicalAssignmentOperator(loc ast.Loc, e *ast.EBinary, op
 	})
 }
 
-func (p *parser) lowerNullishCoalescing(loc ast.Loc, e *ast.EBinary) ast.Expr {
-	// "a ?? b" => "a != null ? a : b"
-	// "a() ?? b()" => "_ = a(), _ != null ? _ : b"
-	leftFunc, wrapFunc := p.captureValueWithPossibleSideEffects(loc, 2, e.Left)
-	return wrapFunc(ast.Expr{e.Right.Loc, &ast.EIf{
-		ast.Expr{loc, &ast.EBinary{
-			ast.BinOpLooseNe,
-			leftFunc(),
-			ast.Expr{loc, &ast.ENull{}},
+func (p *parser) lowerNullishCoalescing(loc ast.Loc, left ast.Expr, right ast.Expr) ast.Expr {
+	if p.Strict.NullishCoalescing {
+		// "x ?? y" => "x !== null && x !== void 0 ? x : y"
+		// "x() ?? y()" => "_a = x(), _a !== null && _a !== void 0 ? _a : y"
+		leftFunc, wrapFunc := p.captureValueWithPossibleSideEffects(loc, 3, left)
+		return wrapFunc(ast.Expr{loc, &ast.EIf{
+			Test: ast.Expr{loc, &ast.EBinary{
+				Op: ast.BinOpLogicalAnd,
+				Left: ast.Expr{loc, &ast.EBinary{
+					Op:    ast.BinOpStrictNe,
+					Left:  leftFunc(),
+					Right: ast.Expr{loc, &ast.ENull{}},
+				}},
+				Right: ast.Expr{loc, &ast.EBinary{
+					Op:    ast.BinOpStrictNe,
+					Left:  leftFunc(),
+					Right: ast.Expr{loc, &ast.EUndefined{}},
+				}},
+			}},
+			Yes: leftFunc(),
+			No:  right,
+		}})
+	}
+
+	// "x ?? y" => "x != null ? x : y"
+	// "x() ?? y()" => "_a = x(), _a != null ? _a : y"
+	leftFunc, wrapFunc := p.captureValueWithPossibleSideEffects(loc, 2, left)
+	return wrapFunc(ast.Expr{loc, &ast.EIf{
+		Test: ast.Expr{loc, &ast.EBinary{
+			Op:    ast.BinOpLooseNe,
+			Left:  leftFunc(),
+			Right: ast.Expr{loc, &ast.ENull{}},
 		}},
-		leftFunc(),
-		e.Right,
+		Yes: leftFunc(),
+		No:  right,
 	}})
 }
 

internal/parser/parser_test.go

@@ -110,6 +110,36 @@ func expectPrintedTarget(t *testing.T, target LanguageTarget, contents string, e
 	})
 }
 
+func expectPrintedTargetStrict(t *testing.T, target LanguageTarget, contents string, expected string) {
+	t.Run(contents, func(t *testing.T) {
+		log := logging.NewDeferLog()
+		ast, ok := Parse(log, logging.Source{
+			Index:        0,
+			AbsolutePath: "<stdin>",
+			PrettyPath:   "<stdin>",
+			Contents:     contents,
+		}, ParseOptions{
+			Target: target,
+			Strict: StrictOptions{
+				NullishCoalescing: true,
+			},
+		})
+		msgs := log.Done()
+		text := ""
+		for _, msg := range msgs {
+			if msg.Kind != logging.Warning {
+				text += msg.String(logging.StderrOptions{}, logging.TerminalInfo{})
+			}
+		}
+		assertEqual(t, text, "")
+		if !ok {
+			t.Fatal("Parse error")
+		}
+		js := printer.Print(ast, printer.PrintOptions{}).JS
+		assertEqual(t, string(js), expected)
+	})
+}
+
 func expectParseErrorJSX(t *testing.T, contents string, expected string) {
 	t.Run(contents, func(t *testing.T) {
 		log := logging.NewDeferLog()
@@ -1535,29 +1565,41 @@ func TestLowerFunctionArgumentScope(t *testing.T) {
 
 func TestLowerNullishCoalescing(t *testing.T) {
 	expectPrintedTarget(t, ES2020, "a ?? b", "a ?? b;\n")
+	expectPrintedTargetStrict(t, ES2020, "a ?? b", "a ?? b;\n")
 
 	expectPrintedTarget(t, ES2019, "a ?? b", "a != null ? a : b;\n")
 	expectPrintedTarget(t, ES2019, "a() ?? b()", "var _a;\n(_a = a()) != null ? _a : b();\n")
 	expectPrintedTarget(t, ES2019, "function foo() { if (x) { a() ?? b() ?? c() } }",
 		"function foo() {\n  var _a, _b;\n  if (x) {\n    (_b = (_a = a()) != null ? _a : b()) != null ? _b : c();\n  }\n}\n")
 	expectPrintedTarget(t, ES2019, "() => a ?? b", "() => a != null ? a : b;\n")
 	expectPrintedTarget(t, ES2019, "() => a() ?? b()", "() => {\n  var _a;\n  return (_a = a()) != null ? _a : b();\n};\n")
+
+	expectPrintedTargetStrict(t, ES2019, "a ?? b", "a !== null && a !== void 0 ? a : b;\n")
+	expectPrintedTargetStrict(t, ES2019, "a() ?? b()", "var _a;\n(_a = a()) !== null && _a !== void 0 ? _a : b();\n")
 }
 
 func TestLowerNullishCoalescingAssign(t *testing.T) {
 	expectPrintedTarget(t, ESNext, "a ??= b", "a ??= b;\n")
+	expectPrintedTargetStrict(t, ESNext, "a ??= b", "a ??= b;\n")
+	expectPrintedTargetStrict(t, ESNext, "a.b ??= c", "a.b ??= c;\n")
 
 	expectPrintedTarget(t, ES2019, "a ??= b", "a != null ? a : a = b;\n")
 	expectPrintedTarget(t, ES2019, "a.b ??= c", "var _a;\n(_a = a.b) != null ? _a : a.b = c;\n")
 	expectPrintedTarget(t, ES2019, "a().b ??= c", "var _a, _b;\n(_b = (_a = a()).b) != null ? _b : _a.b = c;\n")
 	expectPrintedTarget(t, ES2019, "a[b] ??= c", "var _a;\n(_a = a[b]) != null ? _a : a[b] = c;\n")
 	expectPrintedTarget(t, ES2019, "a()[b()] ??= c", "var _a, _b, _c;\n(_c = (_a = a())[_b = b()]) != null ? _c : _a[_b] = c;\n")
 
+	expectPrintedTargetStrict(t, ES2019, "a ??= b", "a !== null && a !== void 0 ? a : a = b;\n")
+	expectPrintedTargetStrict(t, ES2019, "a.b ??= c", "var _a;\n(_a = a.b) !== null && _a !== void 0 ? _a : a.b = c;\n")
+
 	expectPrintedTarget(t, ES2020, "a ??= b", "a ?? (a = b);\n")
 	expectPrintedTarget(t, ES2020, "a.b ??= c", "a.b ?? (a.b = c);\n")
 	expectPrintedTarget(t, ES2020, "a().b ??= c", "var _a;\n(_a = a()).b ?? (_a.b = c);\n")
 	expectPrintedTarget(t, ES2020, "a[b] ??= c", "a[b] ?? (a[b] = c);\n")
 	expectPrintedTarget(t, ES2020, "a()[b()] ??= c", "var _a, _b;\n(_a = a())[_b = b()] ?? (_a[_b] = c);\n")
+
+	expectPrintedTargetStrict(t, ES2020, "a ??= b", "a ?? (a = b);\n")
+	expectPrintedTargetStrict(t, ES2020, "a.b ??= c", "a.b ?? (a.b = c);\n")
 }
 
 func TestLowerLogicalAssign(t *testing.T) {

lib/api-common.ts

@@ -2,6 +2,7 @@ import * as types from "./api-types";
 
 function pushCommonFlags(flags: string[], options: types.CommonOptions, isTTY: boolean, logLevelDefault: types.LogLevel): void {
   if (options.target) flags.push(`--target=${options.target}`);
+  if (options.strict) flags.push(`--strict${options.strict === true ? '' : `=${options.strict.join(',')}`}`);
 
   if (options.minify) flags.push('--minify');
   if (options.minifySyntax) flags.push('--minify-syntax');

lib/api-types.ts

@@ -3,10 +3,12 @@ export declare type Platform = 'browser' | 'node';
 export declare type Format = 'iife' | 'cjs' | 'esm';
 export declare type Loader = 'js' | 'jsx' | 'ts' | 'tsx' | 'json' | 'text' | 'base64' | 'file' | 'dataurl';
 export declare type LogLevel = 'info' | 'warning' | 'error' | 'silent';
+export declare type Strict = 'nullish-coalescing';
 
 export interface CommonOptions {
   sourcemap?: boolean | 'inline' | 'external';
   target?: Target;
+  strict?: boolean | Strict[];
 
   minify?: boolean;
   minifyWhitespace?: boolean;

pkg/api/api.go

@@ -160,6 +160,10 @@ const (
 	LogLevelError
 )
 
+type StrictOptions struct {
+	NullishCoalescing bool
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // Build API
 
@@ -170,6 +174,7 @@ type BuildOptions struct {
 
 	Sourcemap SourceMap
 	Target    Target
+	Strict    StrictOptions
 
 	MinifyWhitespace  bool
 	MinifyIdentifiers bool
@@ -219,6 +224,7 @@ type TransformOptions struct {
 
 	Sourcemap SourceMap
 	Target    Target
+	Strict    StrictOptions
 
 	MinifyWhitespace  bool
 	MinifyIdentifiers bool

pkg/api/api_impl.go

@@ -108,6 +108,12 @@ func validateTarget(value Target) parser.LanguageTarget {
 	}
 }
 
+func validateStrict(value StrictOptions) parser.StrictOptions {
+	return parser.StrictOptions{
+		NullishCoalescing: value.NullishCoalescing,
+	}
+}
+
 func validateLoader(value Loader) bundler.Loader {
 	switch value {
 	case LoaderJS:
@@ -315,6 +321,7 @@ func buildImpl(options BuildOptions) BuildResult {
 	realFS := fs.RealFS()
 	parseOptions := parser.ParseOptions{
 		Target:       validateTarget(options.Target),
+		Strict:       validateStrict(options.Strict),
 		MangleSyntax: options.MinifySyntax,
 		JSX: parser.JSXOptions{
 			Factory:  validateJSX(log, options.JSXFactory, "factory"),
@@ -440,6 +447,7 @@ func transformImpl(input string, options TransformOptions) TransformResult {
 	// Convert and validate the options
 	parseOptions := parser.ParseOptions{
 		Target:       validateTarget(options.Target),
+		Strict:       validateStrict(options.Strict),
 		MangleSyntax: options.MinifySyntax,
 		JSX: parser.JSXOptions{
 			Factory:  validateJSX(log, options.JSXFactory, "factory"),

pkg/cli/cli_impl.go

@@ -172,6 +172,35 @@ func parseOptionsImpl(osArgs []string, buildOpts *api.BuildOptions, transformOpt
 				transformOpts.Target = target
 			}
 
+		case arg == "--strict":
+			value := api.StrictOptions{
+				NullishCoalescing: true,
+			}
+			if buildOpts != nil {
+				buildOpts.Strict = value
+			} else {
+				transformOpts.Strict = value
+			}
+
+		case strings.HasPrefix(arg, "--strict="):
+			value := api.StrictOptions{}
+			parts := arg[len("--strict="):]
+			if parts != "" {
+				for _, part := range strings.Split(parts, ",") {
+					switch part {
+					case "nullish-coalescing":
+						value.NullishCoalescing = true
+					default:
+						return fmt.Errorf("Invalid strict value: %q (valid: nullish-coalescing)", part)
+					}
+				}
+			}
+			if buildOpts != nil {
+				buildOpts.Strict = value
+			} else {
+				transformOpts.Strict = value
+			}
+
 		case strings.HasPrefix(arg, "--platform=") && buildOpts != nil:
 			value := arg[len("--platform="):]
 			switch value {