fix #2231: log about indirect require usage

Author: evanw

CHANGELOG.md

@@ -20,6 +20,25 @@
     // New output (with --minify)
     ```
 
+* Add log messages for indirect `require` references ([#2231](https://github.com/evanw/esbuild/issues/2231))
+
+    A long time ago esbuild used to warn about indirect uses of `require` because they break esbuild's ability to analyze the dependencies of the code and cause dependencies to not be bundled, resulting in a potentially broken bundle. However, this warning was removed because many people wanted the warning to be removed. Some packages have code that uses `require` like this but on a code path that isn't used at run-time, so their code still happens to work even though the bundle is incomplete. For example, the following code will _not_ bundle `bindings`:
+
+    ```js
+    // Prevent React Native packager from seeing modules required with this
+    const nodeRequire = require;
+
+    function getRealmConstructor(environment) {
+      switch (environment) {
+        case "node.js":
+        case "electron":
+          return nodeRequire("bindings")("realm.node").Realm;
+      }
+    }
+    ```
+
+    Version 0.11.11 of esbuild removed this warning, which means people no longer have a way to know at compile time whether their bundle is broken in this way. Now that esbuild has custom log message levels, this warning can be added back in a way that should make both people happy. With this release, there is now a log message for this that defaults to the `debug` log level, which normally isn't visible. You can either do `--log-override:indirect-require=warning` to make this log message a warning (and therefore visible) or use `--log-level=debug` to see this and all other `debug` log messages.
+
 ## 0.14.42
 
 * Fix a parser hang on invalid CSS ([#2276](https://github.com/evanw/esbuild/issues/2276))

internal/bundler/bundler_default_test.go

@@ -6288,3 +6288,33 @@ func TestMangleQuotedPropsMinifySyntax(t *testing.T) {
 		},
 	})
 }
+
+func TestIndirectRequireMessage(t *testing.T) {
+	loader_suite.expectBundled(t, bundled{
+		files: map[string]string{
+			"/array.js":  `let x = [require]`,
+			"/assign.js": `require = x`,
+			"/ident.js":  `let x = require`,
+
+			// These shouldn't log anything: https://github.com/evanw/esbuild/issues/812
+			"/dot.js":   `let x = require.cache`,
+			"/index.js": `let x = require[cache]`,
+		},
+		entryPaths: []string{
+			"/array.js",
+			"/assign.js",
+			"/dot.js",
+			"/ident.js",
+			"/index.js",
+		},
+		options: config.Options{
+			Mode:         config.ModeBundle,
+			AbsOutputDir: "/out",
+		},
+		debugLogs: true,
+		expectedScanLog: `array.js: DEBUG: Indirect calls to "require" will not be bundled
+assign.js: DEBUG: Indirect calls to "require" will not be bundled
+ident.js: DEBUG: Indirect calls to "require" will not be bundled
+`,
+	})
+}

internal/bundler/snapshots/snapshots_loader.txt

@@ -40,6 +40,24 @@ a:after {
   content: "entry2";
 }
 
+================================================================================
+TestIndirectRequireMessage
+---------- /out/array.js ----------
+
+---------- /out/assign.js ----------
+// assign.js
+__require = x;
+
+---------- /out/dot.js ----------
+// dot.js
+var x = __require.cache;
+
+---------- /out/ident.js ----------
+
+---------- /out/index.js ----------
+// index.js
+var x = __require[cache];
+
 ================================================================================
 TestJSXPreserveCapitalLetter
 ---------- /out.js ----------

internal/js_parser/js_parser.go

@@ -143,12 +143,13 @@ type parser struct {
 	// The visit pass binds identifiers to declared symbols, does constant
 	// folding, substitutes compile-time variable definitions, and lowers certain
 	// syntactic constructs as appropriate.
-	stmtExprValue js_ast.E
-	callTarget    js_ast.E
-	templateTag   js_ast.E
-	deleteTarget  js_ast.E
-	loopBody      js_ast.S
-	moduleScope   *js_ast.Scope
+	stmtExprValue    js_ast.E
+	callTarget       js_ast.E
+	dotOrIndexTarget js_ast.E
+	templateTag      js_ast.E
+	deleteTarget     js_ast.E
+	loopBody         js_ast.S
+	moduleScope      *js_ast.Scope
 
 	// This helps recognize the "await import()" pattern. When this is present,
 	// warnings about non-string import paths will be omitted inside try blocks.
@@ -12619,6 +12620,7 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
 			}
 		}
 
+		p.dotOrIndexTarget = e.Target.Data
 		target, out := p.visitExprInOut(e.Target, exprIn{
 			hasChainParent: e.OptionalChain == js_ast.OptionalChainContinue,
 		})
@@ -12703,6 +12705,7 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
 			}
 		}
 
+		p.dotOrIndexTarget = e.Target.Data
 		target, out := p.visitExprInOut(e.Target, exprIn{
 			hasChainParent: e.OptionalChain == js_ast.OptionalChainContinue,
 		})
@@ -14234,6 +14237,11 @@ func (p *parser) handleIdentifier(loc logger.Loc, e *js_ast.EIdentifier, opts id
 
 	// Swap references to the global "require" function with our "__require" stub
 	if ref == p.requireRef && !opts.isCallTarget {
+		if p.options.mode == config.ModeBundle && p.source.Index != runtime.SourceIndex && e != p.dotOrIndexTarget {
+			p.log.AddID(logger.MsgID_JS_IndirectRequire, logger.Debug, &p.tracker, js_lexer.RangeOfIdentifier(p.source, loc),
+				"Indirect calls to \"require\" will not be bundled")
+		}
+
 		return p.valueToSubstituteForRequire(loc)
 	}
 

internal/logger/msg_ids.go

@@ -25,6 +25,7 @@ const (
 	MsgID_JS_EqualsNewObject
 	MsgID_JS_HTMLCommentInJS
 	MsgID_JS_ImpossibleTypeof
+	MsgID_JS_IndirectRequire
 	MsgID_JS_PrivateNameWillThrow
 	MsgID_JS_SemicolonAfterReturn
 	MsgID_JS_SuspiciousBooleanNot
@@ -110,6 +111,8 @@ func StringToMsgIDs(str string, logLevel LogLevel, overrides map[MsgID]LogLevel)
 		overrides[MsgID_JS_HTMLCommentInJS] = logLevel
 	case "impossible-typeof":
 		overrides[MsgID_JS_ImpossibleTypeof] = logLevel
+	case "indirect-require":
+		overrides[MsgID_JS_IndirectRequire] = logLevel
 	case "private-name-will-throw":
 		overrides[MsgID_JS_PrivateNameWillThrow] = logLevel
 	case "semicolon-after-return":
@@ -216,6 +219,8 @@ func MsgIDToString(id MsgID) string {
 		return "html-comment-in-js"
 	case MsgID_JS_ImpossibleTypeof:
 		return "impossible-typeof"
+	case MsgID_JS_IndirectRequire:
+		return "indirect-require"
 	case MsgID_JS_PrivateNameWillThrow:
 		return "private-name-will-throw"
 	case MsgID_JS_SemicolonAfterReturn: