Skip to content

Commit b988bc9

Browse files
author
Andy
authored
Merge pull request #10303 from Microsoft/not_needed_types
Allow `"typings": null`
2 parents 9ac13ab + df739fd commit b988bc9

File tree

8 files changed

+84
-51
lines changed

8 files changed

+84
-51
lines changed

src/compiler/program.ts

+44-41
Original file line numberOriginal file lineDiff line numberDiff line change
@@ -119,49 +119,31 @@ namespace ts {
119
}
119
}
120

120

121
function tryReadTypesSection(packageJsonPath: string, baseDirectory: string, state: ModuleResolutionState): string {
121
function tryReadTypesSection(packageJsonPath: string, baseDirectory: string, state: ModuleResolutionState): string {
122-
let jsonContent: { typings?: string, types?: string, main?: string };
122+
const jsonContent = readJson(packageJsonPath, state.host);
123-
try {
123+
124-
const jsonText = state.host.readFile(packageJsonPath);
124+
function tryReadFromField(fieldName: string) {
125-
jsonContent = jsonText ? <{ typings?: string, types?: string, main?: string }>JSON.parse(jsonText) : {};
125+
if (hasProperty(jsonContent, fieldName)) {
126-
}
126+
const typesFile = (<any>jsonContent)[fieldName];
127-
catch (e) {
127+
if (typeof typesFile === "string") {
128-
// gracefully handle if readFile fails or returns not JSON
128+
const typesFilePath = normalizePath(combinePaths(baseDirectory, typesFile));
129-
jsonContent = {};
129+
if (state.traceEnabled) {
130-
}
130+
trace(state.host, Diagnostics.package_json_has_0_field_1_that_references_2, fieldName, typesFile, typesFilePath);
131-
131+
}
132-
let typesFile: string;
132+
return typesFilePath;
133-
let fieldName: string;
134-
// first try to read content of 'typings' section (backward compatibility)
135-
if (jsonContent.typings) {
136-
if (typeof jsonContent.typings === "string") {
137-
fieldName = "typings";
138-
typesFile = jsonContent.typings;
139-
}
140-
else {
141-
if (state.traceEnabled) {
142-
trace(state.host, Diagnostics.Expected_type_of_0_field_in_package_json_to_be_string_got_1, "typings", typeof jsonContent.typings);
143
}
133
}
144-
}
134+
else {
145-
}
135+
if (state.traceEnabled) {
146-
// then read 'types'
136+
trace(state.host, Diagnostics.Expected_type_of_0_field_in_package_json_to_be_string_got_1, fieldName, typeof typesFile);
147-
if (!typesFile && jsonContent.types) {
137+
}
148-
if (typeof jsonContent.types === "string") {
149-
fieldName = "types";
150-
typesFile = jsonContent.types;
151-
}
152-
else {
153-
if (state.traceEnabled) {
154-
trace(state.host, Diagnostics.Expected_type_of_0_field_in_package_json_to_be_string_got_1, "types", typeof jsonContent.types);
155
}
138
}
156
}
139
}
157
}
140
}
158-
if (typesFile) {
141+
159-
const typesFilePath = normalizePath(combinePaths(baseDirectory, typesFile));
142+
const typesFilePath = tryReadFromField("typings") || tryReadFromField("types");
160-
if (state.traceEnabled) {
143+
if (typesFilePath) {
161-
trace(state.host, Diagnostics.package_json_has_0_field_1_that_references_2, fieldName, typesFile, typesFilePath);
162-
}
163
return typesFilePath;
144
return typesFilePath;
164
}
145
}
146+
165
// Use the main module for inferring types if no types package specified and the allowJs is set
147
// Use the main module for inferring types if no types package specified and the allowJs is set
166
if (state.compilerOptions.allowJs && jsonContent.main && typeof jsonContent.main === "string") {
148
if (state.compilerOptions.allowJs && jsonContent.main && typeof jsonContent.main === "string") {
167
if (state.traceEnabled) {
149
if (state.traceEnabled) {
@@ -173,6 +155,17 @@ namespace ts {
173
return undefined;
155
return undefined;
174
}
156
}
175

157

158+
function readJson(path: string, host: ModuleResolutionHost): { typings?: string, types?: string, main?: string } {
159+
try {
160+
const jsonText = host.readFile(path);
161+
return jsonText ? JSON.parse(jsonText) : {};
162+
}
163+
catch (e) {
164+
// gracefully handle if readFile fails or returns not JSON
165+
return {};
166+
}
167+
}
168+
176
const typeReferenceExtensions = [".d.ts"];
169
const typeReferenceExtensions = [".d.ts"];
177

170

178
function getEffectiveTypeRoots(options: CompilerOptions, host: ModuleResolutionHost) {
171
function getEffectiveTypeRoots(options: CompilerOptions, host: ModuleResolutionHost) {
@@ -717,7 +710,7 @@ namespace ts {
717
}
710
}
718

711

719
function loadNodeModuleFromDirectory(extensions: string[], candidate: string, failedLookupLocation: string[], onlyRecordFailures: boolean, state: ModuleResolutionState): string {
712
function loadNodeModuleFromDirectory(extensions: string[], candidate: string, failedLookupLocation: string[], onlyRecordFailures: boolean, state: ModuleResolutionState): string {
720-
const packageJsonPath = combinePaths(candidate, "package.json");
713+
const packageJsonPath = pathToPackageJson(candidate);
721
const directoryExists = !onlyRecordFailures && directoryProbablyExists(candidate, state.host);
714
const directoryExists = !onlyRecordFailures && directoryProbablyExists(candidate, state.host);
722
if (directoryExists && state.host.fileExists(packageJsonPath)) {
715
if (directoryExists && state.host.fileExists(packageJsonPath)) {
723
if (state.traceEnabled) {
716
if (state.traceEnabled) {
@@ -747,6 +740,10 @@ namespace ts {
747
return loadModuleFromFile(combinePaths(candidate, "index"), extensions, failedLookupLocation, !directoryExists, state);
740
return loadModuleFromFile(combinePaths(candidate, "index"), extensions, failedLookupLocation, !directoryExists, state);
748
}
741
}
749

742

743+
function pathToPackageJson(directory: string): string {
744+
return combinePaths(directory, "package.json");
745+
}
746+
750
function loadModuleFromNodeModulesFolder(moduleName: string, directory: string, failedLookupLocations: string[], state: ModuleResolutionState): string {
747
function loadModuleFromNodeModulesFolder(moduleName: string, directory: string, failedLookupLocations: string[], state: ModuleResolutionState): string {
751
const nodeModulesFolder = combinePaths(directory, "node_modules");
748
const nodeModulesFolder = combinePaths(directory, "node_modules");
752
const nodeModulesFolderExists = directoryProbablyExists(nodeModulesFolder, state.host);
749
const nodeModulesFolderExists = directoryProbablyExists(nodeModulesFolder, state.host);
@@ -1070,15 +1067,21 @@ namespace ts {
1070
}
1067
}
1071

1068

1072
// Walk the primary type lookup locations
1069
// Walk the primary type lookup locations
1073-
let result: string[] = [];
1070+
const result: string[] = [];
1074
if (host.directoryExists && host.getDirectories) {
1071
if (host.directoryExists && host.getDirectories) {
1075
const typeRoots = getEffectiveTypeRoots(options, host);
1072
const typeRoots = getEffectiveTypeRoots(options, host);
1076
if (typeRoots) {
1073
if (typeRoots) {
1077
for (const root of typeRoots) {
1074
for (const root of typeRoots) {
1078
if (host.directoryExists(root)) {
1075
if (host.directoryExists(root)) {
1079
for (const typeDirectivePath of host.getDirectories(root)) {
1076
for (const typeDirectivePath of host.getDirectories(root)) {
1080-
// Return just the type directive names
1077+
const normalized = normalizePath(typeDirectivePath);
1081-
result = result.concat(getBaseFileName(normalizePath(typeDirectivePath)));
1078+
const packageJsonPath = pathToPackageJson(combinePaths(root, normalized));
1079+
// tslint:disable-next-line:no-null-keyword
1080+
const isNotNeededPackage = host.fileExists(packageJsonPath) && readJson(packageJsonPath, host).typings === null;
1081+
if (!isNotNeededPackage) {
1082+
// Return just the type directive names
1083+
result.push(getBaseFileName(normalized));
1084+
}
1082
}
1085
}
1083
}
1086
}
1084
}
1087
}

src/harness/compilerRunner.ts

+1-1
Original file line numberOriginal file lineDiff line numberDiff line change
@@ -52,7 +52,7 @@ class CompilerBaselineRunner extends RunnerBase {
52
private makeUnitName(name: string, root: string) {
52
private makeUnitName(name: string, root: string) {
53
const path = ts.toPath(name, root, (fileName) => Harness.Compiler.getCanonicalFileName(fileName));
53
const path = ts.toPath(name, root, (fileName) => Harness.Compiler.getCanonicalFileName(fileName));
54
const pathStart = ts.toPath(Harness.IO.getCurrentDirectory(), "", (fileName) => Harness.Compiler.getCanonicalFileName(fileName));
54
const pathStart = ts.toPath(Harness.IO.getCurrentDirectory(), "", (fileName) => Harness.Compiler.getCanonicalFileName(fileName));
55-
return path.replace(pathStart, "/");
55+
return pathStart ? path.replace(pathStart, "/") : path;
56
};
56
};
57

57

58
public checkTestCodeOutput(fileName: string) {
58
public checkTestCodeOutput(fileName: string) {

src/harness/fourslash.ts

+10-9
Original file line numberOriginal file lineDiff line numberDiff line change
@@ -2395,13 +2395,14 @@ ${code}
2395
// Comment line, check for global/file @options and record them
2395
// Comment line, check for global/file @options and record them
2396
const match = optionRegex.exec(line.substr(2));
2396
const match = optionRegex.exec(line.substr(2));
2397
if (match) {
2397
if (match) {
2398-
const fileMetadataNamesIndex = fileMetadataNames.indexOf(match[1]);
2398+
const [key, value] = match.slice(1);
2399+
const fileMetadataNamesIndex = fileMetadataNames.indexOf(key);
2399
if (fileMetadataNamesIndex === -1) {
2400
if (fileMetadataNamesIndex === -1) {
2400
// Check if the match is already existed in the global options
2401
// Check if the match is already existed in the global options
2401-
if (globalOptions[match[1]] !== undefined) {
2402+
if (globalOptions[key] !== undefined) {
2402-
throw new Error("Global Option : '" + match[1] + "' is already existed");
2403+
throw new Error(`Global option '${key}' already exists`);
2403
}
2404
}
2404-
globalOptions[match[1]] = match[2];
2405+
globalOptions[key] = value;
2405
}
2406
}
2406
else {
2407
else {
2407
if (fileMetadataNamesIndex === fileMetadataNames.indexOf(metadataOptionNames.fileName)) {
2408
if (fileMetadataNamesIndex === fileMetadataNames.indexOf(metadataOptionNames.fileName)) {
@@ -2416,12 +2417,12 @@ ${code}
2416
resetLocalData();
2417
resetLocalData();
2417
}
2418
}
2418

2419

2419-
currentFileName = basePath + "/" + match[2];
2420+
currentFileName = basePath + "/" + value;
2420-
currentFileOptions[match[1]] = match[2];
2421+
currentFileOptions[key] = value;
2421
}
2422
}
2422
else {
2423
else {
2423
// Add other fileMetadata flag
2424
// Add other fileMetadata flag
2424-
currentFileOptions[match[1]] = match[2];
2425+
currentFileOptions[key] = value;
2425
}
2426
}
2426
}
2427
}
2427
}
2428
}
@@ -2509,7 +2510,7 @@ ${code}
2509
}
2510
}
2510

2511

2511
const marker: Marker = {
2512
const marker: Marker = {
2512-
fileName: fileName,
2513+
fileName,
2513
position: location.position,
2514
position: location.position,
2514
data: markerValue
2515
data: markerValue
2515
};
2516
};
@@ -2526,7 +2527,7 @@ ${code}
2526

2527

2527
function recordMarker(fileName: string, location: LocationInformation, name: string, markerMap: MarkerMap, markers: Marker[]): Marker {
2528
function recordMarker(fileName: string, location: LocationInformation, name: string, markerMap: MarkerMap, markers: Marker[]): Marker {
2528
const marker: Marker = {
2529
const marker: Marker = {
2529-
fileName: fileName,
2530+
fileName,
2530
position: location.position
2531
position: location.position
2531
};
2532
};
2532

2533

Original file line numberOriginal file lineDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
//// [tests/cases/conformance/typings/typingsLookup2.ts] ////
2+
3+
//// [package.json]
4+
{ "typings": null }
5+
6+
//// [a.ts]
7+
8+
9+
//// [a.js]
Original file line numberOriginal file lineDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
=== /a.ts ===
2+
3+
No type information for this code.
Original file line numberOriginal file lineDiff line numberDiff line change
@@ -0,0 +1 @@
1+
[]
Original file line numberOriginal file lineDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
=== /a.ts ===
2+
3+
No type information for this code.
Original file line numberOriginal file lineDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// @traceResolution: true
2+
// @noImplicitReferences: true
3+
// @currentDirectory: /
4+
// This tests that an @types package with `"typings": null` is not automatically included.
5+
// (If it were, this test would break because there are no typings to be found.)
6+
7+
// @filename: /tsconfig.json
8+
{}
9+
10+
// @filename: /node_modules/@types/angular2/package.json
11+
{ "typings": null }
12+
13+
// @filename: /a.ts

0 commit comments

Comments
 (0)