fix #512: omit warnings for require() inside try

CHANGELOG.md

@@ -6,6 +6,10 @@
 
     The parsing of TypeScript's `import name =` syntax should now match the official TypeScript parser. Previously esbuild incorrectly allowed any kind of expression after the equals sign. Now you can only use either a sequence of identifiers separated by periods or a call to the `require` function with a string literal.
 
+* Do not report warnings about `require()` inside `try` ([#512](https://github.com/evanw/esbuild/issues/512))
+
+    This release no longer reports warnings about un-bundled calls to `require()` if they are within a `try` block statement. Presumably the try/catch statement is there to handle the potential run-time error from the unbundled `require()` call failing, so the potential failure is expected and not worth warning about.
+
 ## 0.8.5
 
 * Direct `eval()` now causes the module to be considered CommonJS ([#175](https://github.com/evanw/esbuild/pull/175))

internal/bundler/bundler_default_test.go

@@ -815,6 +815,15 @@ func TestRequireAndDynamicImportInvalidTemplate(t *testing.T) {
 				require(` + "`./${b}`" + `)
 				import(tag` + "`./b`" + `)
 				import(` + "`./${b}`" + `)
+
+				// Try/catch should silence this warning for require()
+				try {
+					require(tag` + "`./b`" + `)
+					require(` + "`./${b}`" + `)
+					import(tag` + "`./b`" + `)
+					import(` + "`./${b}`" + `)
+				} catch {
+				}
 			`,
 		},
 		entryPaths: []string{"/entry.js"},
@@ -826,6 +835,8 @@ func TestRequireAndDynamicImportInvalidTemplate(t *testing.T) {
 /entry.js: warning: This call to "require" will not be bundled because the argument is not a string literal
 /entry.js: warning: This dynamic import will not be bundled because the argument is not a string literal
 /entry.js: warning: This dynamic import will not be bundled because the argument is not a string literal
+/entry.js: warning: This dynamic import will not be bundled because the argument is not a string literal
+/entry.js: warning: This dynamic import will not be bundled because the argument is not a string literal
 `,
 	})
 }
@@ -836,6 +847,13 @@ func TestRequireBadArgumentCount(t *testing.T) {
 			"/entry.js": `
 				require()
 				require("a", "b")
+
+				// Try/catch should silence this warning
+				try {
+					require()
+					require("a", "b")
+				} catch {
+				}
 			`,
 		},
 		entryPaths: []string{"/entry.js"},

internal/bundler/snapshots/snapshots_default.txt

@@ -1586,13 +1586,25 @@ require(tag`./b`);
 require(`./${b}`);
 import(tag`./b`);
 import(`./${b}`);
+try {
+  require(tag`./b`);
+  require(`./${b}`);
+  import(tag`./b`);
+  import(`./${b}`);
+} catch {
+}
 
 ================================================================================
 TestRequireBadArgumentCount
 ---------- /out.js ----------
 // /entry.js
 require();
 require("a", "b");
+try {
+  require();
+  require("a", "b");
+} catch {
+}
 
 ================================================================================
 TestRequireChildDirCommonJS

internal/js_parser/js_parser.go

@@ -9203,13 +9203,14 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
 		// Track calls to require() so we can use them while bundling
 		if p.Mode != config.ModePassThrough {
 			if id, ok := e.Target.Data.(*js_ast.EIdentifier); ok && id.Ref == p.requireRef {
+				// Heuristic: omit warnings inside try/catch blocks because presumably
+				// the try/catch statement is there to handle the potential run-time
+				// error from the unbundled require() call failing.
+				omitWarnings := p.fnOrArrowDataVisit.tryBodyCount != 0
+
 				if p.Mode == config.ModeBundle {
 					// There must be one argument
-					if len(e.Args) != 1 {
-						r := js_lexer.RangeOfIdentifier(p.source, e.Target.Loc)
-						p.log.AddRangeWarning(&p.source, r, fmt.Sprintf(
-							"This call to \"require\" will not be bundled because it has %d arguments", len(e.Args)))
-					} else {
+					if len(e.Args) == 1 {
 						arg := e.Args[0]
 
 						// The argument must be a string
@@ -9230,11 +9231,17 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
 							return js_ast.Expr{Loc: expr.Loc, Data: &js_ast.ERequire{ImportRecordIndex: importRecordIndex}}, exprOut{}
 						}
 
+						if !omitWarnings {
+							r := js_lexer.RangeOfIdentifier(p.source, e.Target.Loc)
+							p.log.AddRangeWarning(&p.source, r,
+								"This call to \"require\" will not be bundled because the argument is not a string literal")
+						}
+					} else if !omitWarnings {
 						r := js_lexer.RangeOfIdentifier(p.source, e.Target.Loc)
-						p.log.AddRangeWarning(&p.source, r,
-							"This call to \"require\" will not be bundled because the argument is not a string literal")
+						p.log.AddRangeWarning(&p.source, r, fmt.Sprintf(
+							"This call to \"require\" will not be bundled because it has %d arguments", len(e.Args)))
 					}
-				} else if p.OutputFormat == config.FormatESModule {
+				} else if p.OutputFormat == config.FormatESModule && !omitWarnings {
 					r := js_lexer.RangeOfIdentifier(p.source, e.Target.Loc)
 					p.log.AddRangeWarning(&p.source, r, "Converting \"require\" to \"esm\" is currently not supported")
 				}