Skip to content

Commit c0309fa

Browse files
sandersnsandersn
committed
Fix crash when checking module exports for export=
Also make maxNodeModuleJsDepth default to 0 so that incorrect tsconfigs now let the compiler spend less time compiling JS that is found in node_modules (especially since most people will already have the d.ts and want ignore the JS anyway). jsconfig still defaults to 2.
1 parent 93de502 commit c0309fa

11 files changed

+123
-7
lines changed

‎src/compiler/checker.ts

+4-2
Original file line numberDiff line numberDiff line change
@@ -1023,8 +1023,8 @@ namespace ts {
10231023
}
10241024
}
10251025

1026-
function getDeclarationOfAliasSymbol(symbol: Symbol): Declaration {
1027-
return findMap(symbol.declarations, d => isAliasSymbolDeclaration(d) ? d : undefined);
1026+
function getDeclarationOfAliasSymbol(symbol: Symbol): Declaration | undefined {
1027+
return forEach(symbol.declarations, d => isAliasSymbolDeclaration(d) ? d : undefined);
10281028
}
10291029

10301030
function getTargetOfImportEqualsDeclaration(node: ImportEqualsDeclaration): Symbol {
@@ -1191,6 +1191,7 @@ namespace ts {
11911191
if (!links.target) {
11921192
links.target = resolvingSymbol;
11931193
const node = getDeclarationOfAliasSymbol(symbol);
1194+
Debug.assert(!!node);
11941195
const target = getTargetOfAliasDeclaration(node);
11951196
if (links.target === resolvingSymbol) {
11961197
links.target = target || unknownSymbol;
@@ -1226,6 +1227,7 @@ namespace ts {
12261227
if (!links.referenced) {
12271228
links.referenced = true;
12281229
const node = getDeclarationOfAliasSymbol(symbol);
1230+
Debug.assert(!!node);
12291231
if (node.kind === SyntaxKind.ExportAssignment) {
12301232
// export default <symbol>
12311233
checkExpressionCached((<ExportAssignment>node).expression);

‎src/compiler/commandLineParser.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -885,7 +885,7 @@ namespace ts {
885885
function convertCompilerOptionsFromJsonWorker(jsonOptions: any,
886886
basePath: string, errors: Diagnostic[], configFileName?: string): CompilerOptions {
887887

888-
const options: CompilerOptions = getBaseFileName(configFileName) === "jsconfig.json" ? { allowJs: true } : {};
888+
const options: CompilerOptions = getBaseFileName(configFileName) === "jsconfig.json" ? { allowJs: true, maxNodeModuleJsDepth: 2 } : {};
889889
convertOptionsFromJson(optionDeclarations, jsonOptions, basePath, options, Diagnostics.Unknown_compiler_option_0, errors);
890890
return options;
891891
}

‎src/compiler/program.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -1101,7 +1101,7 @@ namespace ts {
11011101
// - This calls resolveModuleNames, and then calls findSourceFile for each resolved module.
11021102
// As all these operations happen - and are nested - within the createProgram call, they close over the below variables.
11031103
// The current resolution depth is tracked by incrementing/decrementing as the depth first search progresses.
1104-
const maxNodeModulesJsDepth = typeof options.maxNodeModuleJsDepth === "number" ? options.maxNodeModuleJsDepth : 2;
1104+
const maxNodeModulesJsDepth = typeof options.maxNodeModuleJsDepth === "number" ? options.maxNodeModuleJsDepth : 0;
11051105
let currentNodeModulesDepth = 0;
11061106

11071107
// If a module has some of its imports skipped due to being at the depth limit under node_modules, then track

‎src/harness/unittests/convertCompilerOptionsFromJson.ts

+6-2
Original file line numberDiff line numberDiff line change
@@ -403,6 +403,7 @@ namespace ts {
403403
{
404404
compilerOptions: <CompilerOptions>{
405405
allowJs: true,
406+
maxNodeModuleJsDepth: 2,
406407
module: ModuleKind.CommonJS,
407408
target: ScriptTarget.ES5,
408409
noImplicitAny: false,
@@ -429,6 +430,7 @@ namespace ts {
429430
{
430431
compilerOptions: <CompilerOptions>{
431432
allowJs: false,
433+
maxNodeModuleJsDepth: 2,
432434
module: ModuleKind.CommonJS,
433435
target: ScriptTarget.ES5,
434436
noImplicitAny: false,
@@ -450,7 +452,8 @@ namespace ts {
450452
{
451453
compilerOptions:
452454
{
453-
allowJs: true
455+
allowJs: true,
456+
maxNodeModuleJsDepth: 2
454457
},
455458
errors: [{
456459
file: undefined,
@@ -469,7 +472,8 @@ namespace ts {
469472
{
470473
compilerOptions:
471474
{
472-
allowJs: true
475+
allowJs: true,
476+
maxNodeModuleJsDepth: 2
473477
},
474478
errors: <Diagnostic[]>[]
475479
}

‎tests/baselines/reference/errorForConflictingExportEqualsValue.errors.txt

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
tests/cases/compiler/errorForConflictingExportEqualsValue.ts(2,1): error TS2309: An export assignment cannot be used in a module with other exported elements.
2+
3+
4+
==== tests/cases/compiler/errorForConflictingExportEqualsValue.ts (1 errors) ====
5+
export var x;
6+
export = {};
7+
~~~~~~~~~~~~
8+
!!! error TS2309: An export assignment cannot be used in a module with other exported elements.
9+

‎tests/baselines/reference/errorForConflictingExportEqualsValue.js

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
//// [errorForConflictingExportEqualsValue.ts]
2+
export var x;
3+
export = {};
4+
5+
6+
//// [errorForConflictingExportEqualsValue.js]
7+
"use strict";
8+
module.exports = {};

‎tests/baselines/reference/maxNodeModuleJsDepthDefaultsToZero.errors.txt

+26
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
c:/root/index.ts(4,5): error TS2339: Property 'y' does not exist on type 'typeof "shortid"'.
2+
3+
4+
==== c:/root/index.ts (1 errors) ====
5+
/// <reference path="c:/root/typings/index.d.ts" />
6+
import * as foo from "shortid";
7+
foo.x // found in index.d.ts
8+
foo.y // ignored from shortid/index.ts
9+
~
10+
!!! error TS2339: Property 'y' does not exist on type 'typeof "shortid"'.
11+
12+
13+
==== c:/root/node_modules/shortid/node_modules/z/index.js (0 errors) ====
14+
// z will not be found because maxNodeModulesJsDepth: 0
15+
module.exports = { z: 'no' };
16+
17+
==== c:/root/node_modules/shortid/index.js (0 errors) ====
18+
var z = require('z');
19+
var y = { y: 'foo' };
20+
module.exports = y;
21+
22+
==== c:/root/typings/index.d.ts (0 errors) ====
23+
declare module "shortid" {
24+
export var x: number;
25+
}
26+

‎tests/baselines/reference/maxNodeModuleJsDepthDefaultsToZero.trace.json

+28
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
[
2+
"======== Resolving module 'shortid' from 'c:/root/index.ts'. ========",
3+
"Explicitly specified module resolution kind: 'NodeJs'.",
4+
"Loading module 'shortid' from 'node_modules' folder.",
5+
"File 'c:/root/node_modules/shortid.ts' does not exist.",
6+
"File 'c:/root/node_modules/shortid.tsx' does not exist.",
7+
"File 'c:/root/node_modules/shortid.d.ts' does not exist.",
8+
"File 'c:/root/node_modules/shortid.js' does not exist.",
9+
"File 'c:/root/node_modules/shortid.jsx' does not exist.",
10+
"File 'c:/root/node_modules/shortid/package.json' does not exist.",
11+
"File 'c:/root/node_modules/shortid/index.ts' does not exist.",
12+
"File 'c:/root/node_modules/shortid/index.tsx' does not exist.",
13+
"File 'c:/root/node_modules/shortid/index.d.ts' does not exist.",
14+
"File 'c:/root/node_modules/shortid/index.js' exist - use it as a name resolution result.",
15+
"File 'c:/root/node_modules/@types/shortid.ts' does not exist.",
16+
"File 'c:/root/node_modules/@types/shortid.tsx' does not exist.",
17+
"File 'c:/root/node_modules/@types/shortid.d.ts' does not exist.",
18+
"File 'c:/root/node_modules/@types/shortid.js' does not exist.",
19+
"File 'c:/root/node_modules/@types/shortid.jsx' does not exist.",
20+
"File 'c:/root/node_modules/@types/shortid/package.json' does not exist.",
21+
"File 'c:/root/node_modules/@types/shortid/index.ts' does not exist.",
22+
"File 'c:/root/node_modules/@types/shortid/index.tsx' does not exist.",
23+
"File 'c:/root/node_modules/@types/shortid/index.d.ts' does not exist.",
24+
"File 'c:/root/node_modules/@types/shortid/index.js' does not exist.",
25+
"File 'c:/root/node_modules/@types/shortid/index.jsx' does not exist.",
26+
"Resolving real path for 'c:/root/node_modules/shortid/index.js', result 'c:/root/node_modules/shortid/index.js'",
27+
"======== Module name 'shortid' was successfully resolved to 'c:/root/node_modules/shortid/index.js'. ========"
28+
]

‎tests/cases/compiler/errorForConflictingExportEqualsValue.ts

+2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
export var x;
2+
export = {};

‎tests/cases/compiler/maxNodeModuleJsDepthDefaultsToZero.ts

+36
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
// @module: commonjs
2+
// @moduleResolution: node
3+
// @allowJs: true
4+
// @traceResolution: true
5+
// @noEmit: true
6+
7+
// @filename: c:/root/tsconfig.json
8+
{
9+
"compileOnSave": true,
10+
"compilerOptions": {
11+
"module": "commonjs",
12+
"moduleResolution": "node",
13+
"outDir": "bin"
14+
},
15+
"exclude": [ "node_modules" ]
16+
}
17+
// @filename: c:/root/node_modules/shortid/node_modules/z/index.js
18+
// z will not be found because maxNodeModulesJsDepth: 0
19+
module.exports = { z: 'no' };
20+
21+
// @filename: c:/root/node_modules/shortid/index.js
22+
var z = require('z');
23+
var y = { y: 'foo' };
24+
module.exports = y;
25+
26+
// @filename: c:/root/typings/index.d.ts
27+
declare module "shortid" {
28+
export var x: number;
29+
}
30+
31+
// @filename: c:/root/index.ts
32+
/// <reference path="c:/root/typings/index.d.ts" />
33+
import * as foo from "shortid";
34+
foo.x // found in index.d.ts
35+
foo.y // ignored from shortid/index.ts
36+

‎tests/cases/projects/NodeModulesSearch/importHigher/tsconfig.json

+2-1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
"compilerOptions": {
33
"allowJs": true,
44
"declaration": false,
5-
"moduleResolution": "node"
5+
"moduleResolution": "node",
6+
"maxNodeModuleJsDepth": 2
67
}
78
}

0 commit comments

Comments
 (0)