Stop indirect call warning for 'require.cache' for CommonJS modules

CHANGELOG.md

@@ -1,5 +1,15 @@
 # Changelog
 
+## Unreleased
+
+* Omit warning about `require.someProperty` when targeting CommonJS ([#812](https://github.com/evanw/esbuild/issues/812))
+
+    The `require.cache` property allows introspecting the state of the `require` cache, generally without affecting what is imported/bundled.
+
+    Since esbuild's static analyzer only detects direct calls to `require`, it currently warns about uses of `require` in any situation other than a direct call since that means the value is "escaping" the analyzer. This is meant to detect and warn about indirect calls such as `['fs', 'path'].map(require)`.
+
+    However, this warning is not relevant when accessing a property off of the `require` object such as `require.cache` because a property access does not result in capturing the value of `require`. Now a warning is no longer generated for `require.someProperty` when the output format is `cjs`. This allows for the use of features such as `require.cache` and `require.extensions`.
+
 ## 0.8.46
 
 * Fix minification of `.0` in CSS ([#804](https://github.com/evanw/esbuild/issues/804))

internal/bundler/bundler_default_test.go

@@ -1185,6 +1185,51 @@ func TestRequireWithoutCallInsideTry(t *testing.T) {
 	})
 }
 
+func TestRequirePropertyAccessCommonJS(t *testing.T) {
+	default_suite.expectBundled(t, bundled{
+		files: map[string]string{
+			"/entry.js": `
+				// These shouldn't warn
+				console.log(Object.keys(require.cache))
+				console.log(Object.keys(require.extensions))
+				delete require.cache['fs']
+				delete require.extensions['.json']
+			`,
+		},
+		entryPaths: []string{"/entry.js"},
+		options: config.Options{
+			Mode:          config.ModeBundle,
+			OutputFormat:  config.FormatCommonJS,
+			AbsOutputFile: "/out.js",
+		},
+	})
+}
+
+func TestRequirePropertyAccessES6(t *testing.T) {
+	default_suite.expectBundled(t, bundled{
+		files: map[string]string{
+			"/entry.js": `
+				// These shouldn't warn
+				console.log(Object.keys(require.cache))
+				console.log(Object.keys(require.extensions))
+				delete require.cache['fs']
+				delete require.extensions['.json']
+			`,
+		},
+		entryPaths: []string{"/entry.js"},
+		options: config.Options{
+			Mode:          config.ModeBundle,
+			OutputFormat:  config.FormatESModule,
+			AbsOutputFile: "/out.js",
+		},
+		expectedScanLog: `entry.js: warning: Indirect calls to "require" will not be bundled (surround with a try/catch to silence this warning)
+entry.js: warning: Indirect calls to "require" will not be bundled (surround with a try/catch to silence this warning)
+entry.js: warning: Indirect calls to "require" will not be bundled (surround with a try/catch to silence this warning)
+entry.js: warning: Indirect calls to "require" will not be bundled (surround with a try/catch to silence this warning)
+`,
+	})
+}
+
 // Test a workaround for code using "await import()"
 func TestAwaitImportInsideTry(t *testing.T) {
 	default_suite.expectBundled(t, bundled{
@@ -3793,12 +3838,13 @@ func TestConstWithLetNoMangle(t *testing.T) {
 	})
 }
 
-func TestRequireMainCommonJS(t *testing.T) {
+func TestRequireMainCacheCommonJS(t *testing.T) {
 	default_suite.expectBundled(t, bundled{
 		files: map[string]string{
 			"/entry.js": `
 				console.log('is main:', require.main === module)
 				console.log(require('./is-main'))
+				console.log('cache:', require.cache);
 			`,
 			"/is-main.js": `
 				module.exports = require.main === module
@@ -3813,11 +3859,12 @@ func TestRequireMainCommonJS(t *testing.T) {
 	})
 }
 
-func TestRequireMainIIFE(t *testing.T) {
+func TestRequireMainCacheIIFE(t *testing.T) {
 	default_suite.expectBundled(t, bundled{
 		files: map[string]string{
 			"/entry.js": `
 				console.log('is main:', require.main === module)
+				console.log('cache:', require.cache);
 			`,
 		},
 		entryPaths: []string{"/entry.js"},
@@ -3827,6 +3874,7 @@ func TestRequireMainIIFE(t *testing.T) {
 			OutputFormat:  config.FormatIIFE,
 		},
 		expectedScanLog: `entry.js: warning: Indirect calls to "require" will not be bundled (surround with a try/catch to silence this warning)
+entry.js: warning: Indirect calls to "require" will not be bundled (surround with a try/catch to silence this warning)
 `,
 	})
 }

internal/bundler/snapshots/snapshots_default.txt

@@ -1944,7 +1944,7 @@ var require_test = __commonJS((exports, module) => {
 console.log(require_test());
 
 ================================================================================
-TestRequireMainCommonJS
+TestRequireMainCacheCommonJS
 ---------- /out.js ----------
 // is-main.js
 var require_is_main = __commonJS((exports2, module2) => {
@@ -1954,14 +1954,16 @@ var require_is_main = __commonJS((exports2, module2) => {
 // entry.js
 console.log("is main:", require.main === module);
 console.log(require_is_main());
+console.log("cache:", require.cache);
 
 ================================================================================
-TestRequireMainIIFE
+TestRequireMainCacheIIFE
 ---------- /out.js ----------
 (() => {
   // entry.js
   var require_entry = __commonJS((exports, module) => {
     console.log("is main:", require.main === module);
+    console.log("cache:", require.cache);
   });
   require_entry();
 })();
@@ -1986,6 +1988,24 @@ var src_default = 123;
 // Users/user/project/src/dir/entry.js
 console.log(src_default);
 
+================================================================================
+TestRequirePropertyAccessCommonJS
+---------- /out.js ----------
+// entry.js
+console.log(Object.keys(require.cache));
+console.log(Object.keys(require.extensions));
+delete require.cache["fs"];
+delete require.extensions[".json"];
+
+================================================================================
+TestRequirePropertyAccessES6
+---------- /out.js ----------
+// entry.js
+console.log(Object.keys(require.cache));
+console.log(Object.keys(require.extensions));
+delete require.cache["fs"];
+delete require.extensions[".json"];
+
 ================================================================================
 TestRequireResolve
 ---------- /out.js ----------

internal/js_parser/js_parser.go

@@ -206,10 +206,10 @@ type parser struct {
 	// warnings about non-string import paths will be omitted inside try blocks.
 	awaitTarget js_ast.E
 
-	// This helps recognize the "require.main" pattern. If this pattern is
+	// This helps recognize the "require.someProperty" pattern. If this pattern is
 	// present and the output format is CommonJS, we avoid generating a warning
 	// about an unbundled use of "require".
-	cjsDotMainTarget js_ast.E
+	cjsDotTarget js_ast.E
 
 	// This helps recognize calls to "require.resolve()" which may become
 	// ERequireResolve expressions.
@@ -10392,9 +10392,9 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
 			}
 		}
 
-		// Pattern-match "require.main" from node
-		if p.options.outputFormat == config.FormatCommonJS && e.Name == "main" {
-			p.cjsDotMainTarget = e.Target.Data
+		// This helps us pattern-match "require.someProperty" when targeting CommonJS
+		if p.options.outputFormat == config.FormatCommonJS {
+			p.cjsDotTarget = e.Target.Data
 		}
 
 		isCallTarget := e == p.callTarget
@@ -11156,8 +11156,11 @@ func (p *parser) handleIdentifier(loc logger.Loc, assignTarget js_ast.AssignTarg
 		}
 	}
 
-	// Warn about uses of "require" other than a direct call
-	if ref == p.requireRef && e != p.callTarget && e != p.typeofTarget && e != p.cjsDotMainTarget && p.fnOrArrowDataVisit.tryBodyCount == 0 {
+	// Warn about uses of "require" other than a direct "require()" call, a
+	// "typeof require" expression, or a "require.someProperty" access. But
+	// suppress warnings inside a try body block since presumably the try/catch
+	// is there to handle run-time failures due to indirect require calls.
+	if ref == p.requireRef && e != p.callTarget && e != p.typeofTarget && e != p.cjsDotTarget && p.fnOrArrowDataVisit.tryBodyCount == 0 {
 		// "typeof require == 'function' && require"
 		if e == p.typeofRequireEqualsFnTarget {
 			// Become "false" in the browser and "require" in node