Fix exported type resolution in commonjs (microsoft#24495)

sandersn · web-flow · commit cdfa63aa40f7 · 2018-05-30T14:12:38.000-07:00
* Fix resolution of exported types in commonjs It is fine to resolve the types of exported classes in ES6: ```js export class C { } var c = new C() ``` But not for commonjs exported classes: ```js module.exports.C = class { } var c = new C() // should error ``` Fixes microsoft#24492 * All jsdoc type aliases are available locally in commonjs modules * Check that location isSourceFile before commonJsModuleIndicator
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -1323,8 +1323,14 @@ namespace ts {
                             }
                         }
 
+                        // ES6 exports are also visible locally (except for 'default'), but commonjs exports are not (except typedefs)
                         if (name !== InternalSymbolName.Default && (result = lookup(moduleExports, name, meaning & SymbolFlags.ModuleMember))) {
-                            break loop;
+                            if (isSourceFile(location) && location.commonJsModuleIndicator && !result.declarations.some(isJSDocTypeAlias)) {
+                                result = undefined;
+                            }
+                            else {
+                                break loop;
+                            }
                         }
                         break;
                     case SyntaxKind.EnumDeclaration:
diff --git a/tests/baselines/reference/exportPropertyAssignmentNameResolution.errors.txt b/tests/baselines/reference/exportPropertyAssignmentNameResolution.errors.txt
@@ -0,0 +1,9 @@
+tests/cases/conformance/salsa/bug24492.js(2,5): error TS2304: Cannot find name 'D'.
+
+
+==== tests/cases/conformance/salsa/bug24492.js (1 errors) ====
+    module.exports.D = class { }
+    new D()
+        ~
+!!! error TS2304: Cannot find name 'D'.
+    
diff --git a/tests/baselines/reference/exportPropertyAssignmentNameResolution.symbols b/tests/baselines/reference/exportPropertyAssignmentNameResolution.symbols
@@ -0,0 +1,8 @@
+=== tests/cases/conformance/salsa/bug24492.js ===
+module.exports.D = class { }
+>module.exports : Symbol(D, Decl(bug24492.js, 0, 0))
+>module : Symbol(module)
+>D : Symbol(D, Decl(bug24492.js, 0, 0))
+
+new D()
+
diff --git a/tests/baselines/reference/exportPropertyAssignmentNameResolution.types b/tests/baselines/reference/exportPropertyAssignmentNameResolution.types
@@ -0,0 +1,14 @@
+=== tests/cases/conformance/salsa/bug24492.js ===
+module.exports.D = class { }
+>module.exports.D = class { } : typeof D
+>module.exports.D : any
+>module.exports : any
+>module : any
+>exports : any
+>D : any
+>class { } : typeof D
+
+new D()
+>new D() : any
+>D : any
+
diff --git a/tests/cases/conformance/salsa/exportPropertyAssignmentNameResolution.ts b/tests/cases/conformance/salsa/exportPropertyAssignmentNameResolution.ts
@@ -0,0 +1,6 @@
+// @noEmit: true
+// @allowJs: true
+// @checkJs: true
+// @Filename: bug24492.js
+module.exports.D = class { }
+new D()
diff --git a/tests/cases/fourslash/refactorConvertToEs6Module_export_named.ts b/tests/cases/fourslash/refactorConvertToEs6Module_export_named.ts
@@ -21,9 +21,7 @@ verify.codeFix({
     description: "Convert to ES6 module",
     newFileContent:
 `export function f() {}
-const _C = class {
-};
-export { _C as C };
+export class C {}
 export const x = 0;
 export function a1() {}
 export function a2() { return 0; }
diff --git a/tests/cases/fourslash/refactorConvertToEs6Module_export_namedClassExpression.ts b/tests/cases/fourslash/refactorConvertToEs6Module_export_namedClassExpression.ts
@@ -10,12 +10,6 @@
 verify.codeFix({
     description: "Convert to ES6 module",
     newFileContent:
-`const _C = class E {
-    static instance = new E();
-};
-export { _C as C };
-const _D = class D {
-    static instance = new D();
-};
-export { _D as D };`,
+`export const C = class E { static instance = new E(); }
+export class D { static instance = new D(); }`,
 });
diff --git a/tests/cases/fourslash/refactorConvertToEs6Module_expressionToDeclaration.ts b/tests/cases/fourslash/refactorConvertToEs6Module_expressionToDeclaration.ts
@@ -4,13 +4,17 @@
 // @target: esnext
 
 // @Filename: /a.js
+////var C = {};
+////console.log(C);
 ////exports.f = async function* f(p) { p; }
 ////exports.C = class C extends D { m() {} }
 
 verify.codeFix({
     description: "Convert to ES6 module",
     newFileContent:
-`export async function* f(p) { p; }
+`var C = {};
+console.log(C);
+export async function* f(p) { p; }
 const _C = class C extends D {
     m() { }
 };

Original file line number	Diff line number	Diff line change
`@@ -1323,8 +1323,14 @@ namespace ts {`
`1323`	`1323`	`}`
`1324`	`1324`	`}`
`1325`	`1325`
	`1326`	`+ // ES6 exports are also visible locally (except for 'default'), but commonjs exports are not (except typedefs)`
`1326`	`1327`	`if (name !== InternalSymbolName.Default && (result = lookup(moduleExports, name, meaning & SymbolFlags.ModuleMember))) {`
`1327`		`- break loop;`
	`1328`	`+ if (isSourceFile(location) && location.commonJsModuleIndicator && !result.declarations.some(isJSDocTypeAlias)) {`
	`1329`	`+ result = undefined;`
	`1330`	`+ }`
	`1331`	`+ else {`
	`1332`	`+ break loop;`
	`1333`	`+ }`
`1328`	`1334`	`}`
`1329`	`1335`	`break;`
`1330`	`1336`	`case SyntaxKind.EnumDeclaration:`