Merge pull request microsoft#11370 from Microsoft/requireAsFunctionInExternalModule

sheetalkamat · web-flow · commit 28452d5a7b7a · 2016-10-17T16:37:03.000-07:00
Do not treat local resolution of require call as external module
diff --git a/src/compiler/binder.ts b/src/compiler/binder.ts
@@ -2072,7 +2072,9 @@ namespace ts {
         function setCommonJsModuleIndicator(node: Node) {
             if (!file.commonJsModuleIndicator) {
                 file.commonJsModuleIndicator = node;
-                bindSourceFileAsExternalModule();
+                if (!file.externalModuleIndicator) {
+                    bindSourceFileAsExternalModule();
+                }
             }
         }
 
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -12791,7 +12791,10 @@ namespace ts {
             }
 
             // In JavaScript files, calls to any identifier 'require' are treated as external module imports
-            if (isInJavaScriptFile(node) && isRequireCall(node, /*checkArgumentIsStringLiteral*/true)) {
+            if (isInJavaScriptFile(node) &&
+                isRequireCall(node, /*checkArgumentIsStringLiteral*/true) &&
+                // Make sure require is not a local function
+                !resolveName(node.expression, (<Identifier>node.expression).text, SymbolFlags.Value, /*nameNotFoundMessage*/ undefined, /*nameArg*/ undefined)) {
                 return resolveExternalModuleTypeByLiteral(<StringLiteral>node.arguments[0]);
             }
 
diff --git a/tests/baselines/reference/requireAsFunctionInExternalModule.js b/tests/baselines/reference/requireAsFunctionInExternalModule.js
@@ -0,0 +1,37 @@
+//// [tests/cases/compiler/requireAsFunctionInExternalModule.ts] ////
+
+//// [c.js]
+export default function require(a) { }
+export function has(a) { return true }
+
+//// [m.js]
+import require, { has } from "./c"
+export function hello() { }
+if (has('ember-debug')) {
+    require('ember-debug');
+}
+
+//// [m2.ts]
+import { hello } from "./m";
+hello();
+
+
+//// [c.js]
+"use strict";
+function require(a) { }
+exports.__esModule = true;
+exports["default"] = require;
+function has(a) { return true; }
+exports.has = has;
+//// [m.js]
+"use strict";
+var c_1 = require("./c");
+function hello() { }
+exports.hello = hello;
+if (c_1.has('ember-debug')) {
+    c_1["default"]('ember-debug');
+}
+//// [m2.js]
+"use strict";
+var m_1 = require("./m");
+m_1.hello();
diff --git a/tests/baselines/reference/requireAsFunctionInExternalModule.symbols b/tests/baselines/reference/requireAsFunctionInExternalModule.symbols
@@ -0,0 +1,31 @@
+=== tests/cases/compiler/c.js ===
+export default function require(a) { }
+>require : Symbol(require, Decl(c.js, 0, 0))
+>a : Symbol(a, Decl(c.js, 0, 32))
+
+export function has(a) { return true }
+>has : Symbol(has, Decl(c.js, 0, 38))
+>a : Symbol(a, Decl(c.js, 1, 20))
+
+=== tests/cases/compiler/m.js ===
+import require, { has } from "./c"
+>require : Symbol(require, Decl(m.js, 0, 6))
+>has : Symbol(has, Decl(m.js, 0, 17))
+
+export function hello() { }
+>hello : Symbol(hello, Decl(m.js, 0, 34))
+
+if (has('ember-debug')) {
+>has : Symbol(has, Decl(m.js, 0, 17))
+
+    require('ember-debug');
+>require : Symbol(require, Decl(m.js, 0, 6))
+}
+
+=== tests/cases/compiler/m2.ts ===
+import { hello } from "./m";
+>hello : Symbol(hello, Decl(m2.ts, 0, 8))
+
+hello();
+>hello : Symbol(hello, Decl(m2.ts, 0, 8))
+
diff --git a/tests/baselines/reference/requireAsFunctionInExternalModule.types b/tests/baselines/reference/requireAsFunctionInExternalModule.types
@@ -0,0 +1,37 @@
+=== tests/cases/compiler/c.js ===
+export default function require(a) { }
+>require : (a: any) => void
+>a : any
+
+export function has(a) { return true }
+>has : (a: any) => boolean
+>a : any
+>true : true
+
+=== tests/cases/compiler/m.js ===
+import require, { has } from "./c"
+>require : (a: any) => void
+>has : (a: any) => boolean
+
+export function hello() { }
+>hello : () => void
+
+if (has('ember-debug')) {
+>has('ember-debug') : boolean
+>has : (a: any) => boolean
+>'ember-debug' : "ember-debug"
+
+    require('ember-debug');
+>require('ember-debug') : void
+>require : (a: any) => void
+>'ember-debug' : "ember-debug"
+}
+
+=== tests/cases/compiler/m2.ts ===
+import { hello } from "./m";
+>hello : () => void
+
+hello();
+>hello() : void
+>hello : () => void
+
diff --git a/tests/cases/compiler/requireAsFunctionInExternalModule.ts b/tests/cases/compiler/requireAsFunctionInExternalModule.ts
@@ -0,0 +1,16 @@
+// @allowjs: true
+// @outDir: dist
+// @Filename: c.js
+export default function require(a) { }
+export function has(a) { return true }
+
+// @Filename: m.js
+import require, { has } from "./c"
+export function hello() { }
+if (has('ember-debug')) {
+    require('ember-debug');
+}
+
+// @Filename: m2.ts
+import { hello } from "./m";
+hello();

Original file line number	Diff line number	Diff line change
`@@ -2072,7 +2072,9 @@ namespace ts {`
`2072`	`2072`	`function setCommonJsModuleIndicator(node: Node) {`
`2073`	`2073`	`if (!file.commonJsModuleIndicator) {`
`2074`	`2074`	`file.commonJsModuleIndicator = node;`
`2075`		`- bindSourceFileAsExternalModule();`
	`2075`	`+ if (!file.externalModuleIndicator) {`
	`2076`	`+ bindSourceFileAsExternalModule();`
	`2077`	`+ }`
`2076`	`2078`	`}`
`2077`	`2079`	`}`
`2078`	`2080`
Original file line number	Diff line number	Diff line change
`@@ -12791,7 +12791,10 @@ namespace ts {`
`12791`	`12791`	`}`
`12792`	`12792`
`12793`	`12793`	`// In JavaScript files, calls to any identifier 'require' are treated as external module imports`
`12794`		`- if (isInJavaScriptFile(node) && isRequireCall(node, /checkArgumentIsStringLiteral/true)) {`
	`12794`	`+ if (isInJavaScriptFile(node) &&`
	`12795`	`+ isRequireCall(node, /checkArgumentIsStringLiteral/true) &&`
	`12796`	`+ // Make sure require is not a local function`
	`12797`	`+ !resolveName(node.expression, (<Identifier>node.expression).text, SymbolFlags.Value, /nameNotFoundMessage/ undefined, /nameArg/ undefined)) {`
`12795`	`12798`	`return resolveExternalModuleTypeByLiteral(<StringLiteral>node.arguments[0]);`
`12796`	`12799`	`}`
`12797`	`12800`