Fix test and prevent CJS require from resolving

Author: andrewbranch

src/compiler/binder.ts

@@ -82,6 +82,7 @@ import {
     getContainingClass,
     getEffectiveContainerForJSDocTemplateTag,
     getElementOrPropertyAccessName,
+    getEmitModuleResolutionKind,
     getEmitScriptTarget,
     getEnclosingBlockScopeContainer,
     getErrorSpanForNode,
@@ -227,6 +228,7 @@ import {
     ModifierFlags,
     ModuleBlock,
     ModuleDeclaration,
+    ModuleResolutionKind,
     Mutable,
     NamespaceExportDeclaration,
     Node,
@@ -3583,6 +3585,7 @@ function createBinder(): (file: SourceFile, options: CompilerOptions) => void {
         if (!isBindingPattern(node.name)) {
             const possibleVariableDecl = node.kind === SyntaxKind.VariableDeclaration ? node : node.parent.parent;
             if (isInJSFile(node) &&
+                getEmitModuleResolutionKind(options) !== ModuleResolutionKind.Hybrid &&
                 isVariableDeclarationInitializedToBareOrAccessedRequire(possibleVariableDecl) &&
                 !getJSDocTypeTag(node) &&
                 !(getCombinedModifierFlags(node) & ModifierFlags.Export)

src/compiler/checker.ts

@@ -4505,6 +4505,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
             if (
                 namespace.valueDeclaration &&
                 isInJSFile(namespace.valueDeclaration) &&
+                getEmitModuleResolutionKind(compilerOptions) !== ModuleResolutionKind.Hybrid &&
                 isVariableDeclaration(namespace.valueDeclaration) &&
                 namespace.valueDeclaration.initializer &&
                 isCommonJsRequire(namespace.valueDeclaration.initializer)
@@ -33373,7 +33374,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
         }
 
         // In JavaScript files, calls to any identifier 'require' are treated as external module imports
-        if (isInJSFile(node) && isCommonJsRequire(node)) {
+        if (isInJSFile(node) && getEmitModuleResolutionKind(compilerOptions) !== ModuleResolutionKind.Hybrid && isCommonJsRequire(node)) {
             return resolveExternalModuleTypeByLiteral(node.arguments![0] as StringLiteral);
         }
 
@@ -44099,7 +44100,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
                 // 4). type A = import("./f/*gotToDefinitionHere*/oo")
                 if ((isExternalModuleImportEqualsDeclaration(node.parent.parent) && getExternalModuleImportEqualsDeclarationExpression(node.parent.parent) === node) ||
                     ((node.parent.kind === SyntaxKind.ImportDeclaration || node.parent.kind === SyntaxKind.ExportDeclaration) && (node.parent as ImportDeclaration).moduleSpecifier === node) ||
-                    ((isInJSFile(node) && isRequireCall(node.parent, /*checkArgumentIsStringLiteralLike*/ false)) || isImportCall(node.parent)) ||
+                    ((isInJSFile(node) && getEmitModuleResolutionKind(compilerOptions) !== ModuleResolutionKind.Hybrid && isRequireCall(node.parent, /*checkArgumentIsStringLiteralLike*/ false)) || isImportCall(node.parent)) ||
                     (isLiteralTypeNode(node.parent) && isLiteralImportTypeNode(node.parent.parent) && node.parent.parent.argument === node.parent)
                 ) {
                     return resolveExternalModuleName(node, node as LiteralExpression, ignoreErrors);

src/compiler/program.ts

@@ -3065,7 +3065,10 @@ export function createProgram(rootNamesOrOptions: readonly string[] | CreateProg
         for (const node of file.statements) {
             collectModuleReferences(node, /*inAmbientModule*/ false);
         }
-        if ((file.flags & NodeFlags.PossiblyContainsDynamicImport) || isJavaScriptFile) {
+
+        // `require` has no special meaning in `--moduleResolution hybrid`
+        const shouldProcessRequires = isJavaScriptFile && getEmitModuleResolutionKind(options) !== ModuleResolutionKind.Hybrid;
+        if ((file.flags & NodeFlags.PossiblyContainsDynamicImport) || shouldProcessRequires) {
             collectDynamicImportOrRequireCalls(file);
         }
 
@@ -3127,7 +3130,7 @@ export function createProgram(rootNamesOrOptions: readonly string[] | CreateProg
             const r = /import|require/g;
             while (r.exec(file.text) !== null) { // eslint-disable-line no-null/no-null
                 const node = getNodeAtPosition(file, r.lastIndex);
-                if (isJavaScriptFile && isRequireCall(node, /*checkArgumentIsStringLiteralLike*/ true)) {
+                if (shouldProcessRequires && isRequireCall(node, /*checkArgumentIsStringLiteralLike*/ true)) {
                     setParentRecursive(node, /*incremental*/ false); // we need parent data on imports before the program is fully bound, so we ensure it's set here
                     imports = append(imports, node.arguments[0]);
                 }

tests/baselines/reference/hybridSyntaxRestrictions.errors.txt

@@ -1,7 +1,21 @@
+error TS2468: Cannot find global value 'Promise'.
 /main.ts(2,1): error TS5099: Import assignment is not allowed when 'moduleResolution' is set to 'hybrid'. Consider using 'import * as ns from "mod"', 'import {a} from "mod"', 'import d from "mod"', or another module format instead.
 /main.ts(3,1): error TS5100: Export assignment cannot be used when 'moduleResolution' is set to 'hybrid'. Consider using 'export default' or another module format instead.
+/mainJs.js(2,1): error TS2712: A dynamic import call in ES5/ES3 requires the 'Promise' constructor.  Make sure you have a declaration for the 'Promise' constructor or include 'ES2015' in your '--lib' option.
 
 
+!!! error TS2468: Cannot find global value 'Promise'.
+==== /node_modules/@types/node/index.d.ts (0 errors) ====
+    declare var require: (...args: any[]) => any;
+    
+==== /mainJs.js (1 errors) ====
+    import {} from "./a";
+    import("./a");
+    ~~~~~~~~~~~~~
+!!! error TS2712: A dynamic import call in ES5/ES3 requires the 'Promise' constructor.  Make sure you have a declaration for the 'Promise' constructor or include 'ES2015' in your '--lib' option.
+    const _ = require("./a"); // No resolution
+    _.a; // any
+    
 ==== /main.ts (2 errors) ====
     import {} from "./a";
     import _ = require("./a"); // Error
@@ -12,13 +26,6 @@
 !!! error TS5100: Export assignment cannot be used when 'moduleResolution' is set to 'hybrid'. Consider using 'export default' or another module format instead.
     export {};
 
-==== /node_modules/@types/node/index.d.ts (0 errors) ====
-    declare var require: (...args: any[]) => any;
-    
 ==== /a.ts (0 errors) ====
-    export {};
-    
-==== /mainJs.js (0 errors) ====
-    import {} from "./a";
-    const _ = require("./a"); // No resolution
+    export const a = "a";
 

tests/baselines/reference/hybridSyntaxRestrictions.js

@@ -3,23 +3,33 @@
 //// [index.d.ts]
 declare var require: (...args: any[]) => any;
 
-//// [a.ts]
-export {};
-
 //// [mainJs.js]
 import {} from "./a";
+import("./a");
 const _ = require("./a"); // No resolution
+_.a; // any
 
 //// [main.ts]
 import {} from "./a";
 import _ = require("./a"); // Error
 export = {}; // Error
 export {};
 
+//// [a.ts]
+export const a = "a";
+
 
 //// [a.js]
 "use strict";
 exports.__esModule = true;
+exports.a = void 0;
+exports.a = "a";
+//// [mainJs.js]
+"use strict";
+exports.__esModule = true;
+Promise.resolve().then(function () { return require("./a"); });
+var _ = require("./a"); // No resolution
+_.a; // any
 //// [main.js]
 "use strict";
 module.exports = {};

tests/baselines/reference/hybridSyntaxRestrictions.symbols

@@ -1,3 +1,20 @@
+=== /node_modules/@types/node/index.d.ts ===
+declare var require: (...args: any[]) => any;
+>require : Symbol(require, Decl(index.d.ts, 0, 11))
+>args : Symbol(args, Decl(index.d.ts, 0, 22))
+
+=== /mainJs.js ===
+import {} from "./a";
+import("./a");
+>"./a" : Symbol("/a", Decl(a.ts, 0, 0))
+
+const _ = require("./a"); // No resolution
+>_ : Symbol(_, Decl(mainJs.js, 2, 5))
+>require : Symbol(require, Decl(index.d.ts, 0, 11))
+
+_.a; // any
+>_ : Symbol(_, Decl(mainJs.js, 2, 5))
+
 === /main.ts ===
 import {} from "./a";
 import _ = require("./a"); // Error
@@ -6,12 +23,7 @@ import _ = require("./a"); // Error
 export = {}; // Error
 export {};
 
-=== /node_modules/@types/node/index.d.ts ===
-declare var require: (...args: any[]) => any;
->require : Symbol(require, Decl(index.d.ts, 0, 11))
->args : Symbol(args, Decl(index.d.ts, 0, 22))
-
 === /a.ts ===
-
-export {};
+export const a = "a";
+>a : Symbol(a, Decl(a.ts, 0, 12))
 

tests/baselines/reference/hybridSyntaxRestrictions.types

@@ -1,3 +1,25 @@
+=== /node_modules/@types/node/index.d.ts ===
+declare var require: (...args: any[]) => any;
+>require : (...args: any[]) => any
+>args : any[]
+
+=== /mainJs.js ===
+import {} from "./a";
+import("./a");
+>import("./a") : Promise<typeof import("/a")>
+>"./a" : "./a"
+
+const _ = require("./a"); // No resolution
+>_ : any
+>require("./a") : any
+>require : (...args: any[]) => any
+>"./a" : "./a"
+
+_.a; // any
+>_.a : any
+>_ : any
+>a : any
+
 === /main.ts ===
 import {} from "./a";
 import _ = require("./a"); // Error
@@ -8,12 +30,8 @@ export = {}; // Error
 
 export {};
 
-=== /node_modules/@types/node/index.d.ts ===
-declare var require: (...args: any[]) => any;
->require : (...args: any[]) => any
->args : any[]
-
 === /a.ts ===
-
-export {};
+export const a = "a";
+>a : "a"
+>"a" : "a"
 

tests/cases/conformance/moduleResolution/hybrid/hybridSyntaxRestrictions.ts

@@ -6,15 +6,17 @@
 // @Filename: /node_modules/@types/node/index.d.ts
 declare var require: (...args: any[]) => any;
 
-// @Filename: /a.ts
-export {};
-
 // @Filename: /mainJs.js
 import {} from "./a";
+import("./a");
 const _ = require("./a"); // No resolution
+_.a; // any
 
 // @Filename: /main.ts
 import {} from "./a";
 import _ = require("./a"); // Error
 export = {}; // Error
 export {};
+
+// @Filename: /a.ts
+export const a = "a";