Skip to content

Allow "typings": null #10303

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
1 commit merged into from
Aug 12, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 44 additions & 41 deletions src/compiler/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,49 +119,31 @@ namespace ts {
}

function tryReadTypesSection(packageJsonPath: string, baseDirectory: string, state: ModuleResolutionState): string {
let jsonContent: { typings?: string, types?: string, main?: string };
try {
const jsonText = state.host.readFile(packageJsonPath);
jsonContent = jsonText ? <{ typings?: string, types?: string, main?: string }>JSON.parse(jsonText) : {};
}
catch (e) {
// gracefully handle if readFile fails or returns not JSON
jsonContent = {};
}

let typesFile: string;
let fieldName: string;
// first try to read content of 'typings' section (backward compatibility)
if (jsonContent.typings) {
if (typeof jsonContent.typings === "string") {
fieldName = "typings";
typesFile = jsonContent.typings;
}
else {
if (state.traceEnabled) {
trace(state.host, Diagnostics.Expected_type_of_0_field_in_package_json_to_be_string_got_1, "typings", typeof jsonContent.typings);
const jsonContent = readJson(packageJsonPath, state.host);

function tryReadFromField(fieldName: string) {
if (hasProperty(jsonContent, fieldName)) {
const typesFile = (<any>jsonContent)[fieldName];
if (typeof typesFile === "string") {
const typesFilePath = normalizePath(combinePaths(baseDirectory, typesFile));
if (state.traceEnabled) {
trace(state.host, Diagnostics.package_json_has_0_field_1_that_references_2, fieldName, typesFile, typesFilePath);
}
return typesFilePath;
}
}
}
// then read 'types'
if (!typesFile && jsonContent.types) {
if (typeof jsonContent.types === "string") {
fieldName = "types";
typesFile = jsonContent.types;
}
else {
if (state.traceEnabled) {
trace(state.host, Diagnostics.Expected_type_of_0_field_in_package_json_to_be_string_got_1, "types", typeof jsonContent.types);
else {
if (state.traceEnabled) {
trace(state.host, Diagnostics.Expected_type_of_0_field_in_package_json_to_be_string_got_1, fieldName, typeof typesFile);
}
}
}
}
if (typesFile) {
const typesFilePath = normalizePath(combinePaths(baseDirectory, typesFile));
if (state.traceEnabled) {
trace(state.host, Diagnostics.package_json_has_0_field_1_that_references_2, fieldName, typesFile, typesFilePath);
}

const typesFilePath = tryReadFromField("typings") || tryReadFromField("types");
if (typesFilePath) {
return typesFilePath;
}

// Use the main module for inferring types if no types package specified and the allowJs is set
if (state.compilerOptions.allowJs && jsonContent.main && typeof jsonContent.main === "string") {
if (state.traceEnabled) {
Expand All @@ -173,6 +155,17 @@ namespace ts {
return undefined;
}

function readJson(path: string, host: ModuleResolutionHost): { typings?: string, types?: string, main?: string } {
try {
const jsonText = host.readFile(path);
return jsonText ? JSON.parse(jsonText) : {};
}
catch (e) {
// gracefully handle if readFile fails or returns not JSON
return {};
}
}

const typeReferenceExtensions = [".d.ts"];

function getEffectiveTypeRoots(options: CompilerOptions, host: ModuleResolutionHost) {
Expand Down Expand Up @@ -717,7 +710,7 @@ namespace ts {
}

function loadNodeModuleFromDirectory(extensions: string[], candidate: string, failedLookupLocation: string[], onlyRecordFailures: boolean, state: ModuleResolutionState): string {
const packageJsonPath = combinePaths(candidate, "package.json");
const packageJsonPath = pathToPackageJson(candidate);
const directoryExists = !onlyRecordFailures && directoryProbablyExists(candidate, state.host);
if (directoryExists && state.host.fileExists(packageJsonPath)) {
if (state.traceEnabled) {
Expand Down Expand Up @@ -747,6 +740,10 @@ namespace ts {
return loadModuleFromFile(combinePaths(candidate, "index"), extensions, failedLookupLocation, !directoryExists, state);
}

function pathToPackageJson(directory: string): string {
return combinePaths(directory, "package.json");
}

function loadModuleFromNodeModulesFolder(moduleName: string, directory: string, failedLookupLocations: string[], state: ModuleResolutionState): string {
const nodeModulesFolder = combinePaths(directory, "node_modules");
const nodeModulesFolderExists = directoryProbablyExists(nodeModulesFolder, state.host);
Expand Down Expand Up @@ -1070,15 +1067,21 @@ namespace ts {
}

// Walk the primary type lookup locations
let result: string[] = [];
const result: string[] = [];
if (host.directoryExists && host.getDirectories) {
const typeRoots = getEffectiveTypeRoots(options, host);
if (typeRoots) {
for (const root of typeRoots) {
if (host.directoryExists(root)) {
for (const typeDirectivePath of host.getDirectories(root)) {
// Return just the type directive names
result = result.concat(getBaseFileName(normalizePath(typeDirectivePath)));
const normalized = normalizePath(typeDirectivePath);
const packageJsonPath = pathToPackageJson(combinePaths(root, normalized));
// tslint:disable-next-line:no-null-keyword
const isNotNeededPackage = host.fileExists(packageJsonPath) && readJson(packageJsonPath, host).typings === null;
if (!isNotNeededPackage) {
// Return just the type directive names
result.push(getBaseFileName(normalized));
}
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/harness/compilerRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ class CompilerBaselineRunner extends RunnerBase {
private makeUnitName(name: string, root: string) {
const path = ts.toPath(name, root, (fileName) => Harness.Compiler.getCanonicalFileName(fileName));
const pathStart = ts.toPath(Harness.IO.getCurrentDirectory(), "", (fileName) => Harness.Compiler.getCanonicalFileName(fileName));
return path.replace(pathStart, "/");
return pathStart ? path.replace(pathStart, "/") : path;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes what was breaking web tests: if pathStart was "" then path.replace(pathStart, "/") would put an extra "/" at the beginning (leading to //src/...). Then when the host was determining file existence it would look for strings starting with /src, which there were none of.

};

public checkTestCodeOutput(fileName: string) {
Expand Down
19 changes: 10 additions & 9 deletions src/harness/fourslash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2395,13 +2395,14 @@ ${code}
// Comment line, check for global/file @options and record them
const match = optionRegex.exec(line.substr(2));
if (match) {
const fileMetadataNamesIndex = fileMetadataNames.indexOf(match[1]);
const [key, value] = match.slice(1);
const fileMetadataNamesIndex = fileMetadataNames.indexOf(key);
if (fileMetadataNamesIndex === -1) {
// Check if the match is already existed in the global options
if (globalOptions[match[1]] !== undefined) {
throw new Error("Global Option : '" + match[1] + "' is already existed");
if (globalOptions[key] !== undefined) {
throw new Error(`Global option '${key}' already exists`);
}
globalOptions[match[1]] = match[2];
globalOptions[key] = value;
}
else {
if (fileMetadataNamesIndex === fileMetadataNames.indexOf(metadataOptionNames.fileName)) {
Expand All @@ -2416,12 +2417,12 @@ ${code}
resetLocalData();
}

currentFileName = basePath + "/" + match[2];
currentFileOptions[match[1]] = match[2];
currentFileName = basePath + "/" + value;
currentFileOptions[key] = value;
}
else {
// Add other fileMetadata flag
currentFileOptions[match[1]] = match[2];
currentFileOptions[key] = value;
}
}
}
Expand Down Expand Up @@ -2509,7 +2510,7 @@ ${code}
}

const marker: Marker = {
fileName: fileName,
fileName,
position: location.position,
data: markerValue
};
Expand All @@ -2526,7 +2527,7 @@ ${code}

function recordMarker(fileName: string, location: LocationInformation, name: string, markerMap: MarkerMap, markers: Marker[]): Marker {
const marker: Marker = {
fileName: fileName,
fileName,
position: location.position
};

Expand Down
9 changes: 9 additions & 0 deletions tests/baselines/reference/typingsLookup2.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
//// [tests/cases/conformance/typings/typingsLookup2.ts] ////

//// [package.json]
{ "typings": null }

//// [a.ts]


//// [a.js]
3 changes: 3 additions & 0 deletions tests/baselines/reference/typingsLookup2.symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
=== /a.ts ===

No type information for this code.
1 change: 1 addition & 0 deletions tests/baselines/reference/typingsLookup2.trace.json
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
[]
3 changes: 3 additions & 0 deletions tests/baselines/reference/typingsLookup2.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
=== /a.ts ===

No type information for this code.
13 changes: 13 additions & 0 deletions tests/cases/conformance/typings/typingsLookup2.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// @traceResolution: true
// @noImplicitReferences: true
// @currentDirectory: /
// This tests that an @types package with `"typings": null` is not automatically included.
// (If it were, this test would break because there are no typings to be found.)

// @filename: /tsconfig.json
{}

// @filename: /node_modules/@types/angular2/package.json
{ "typings": null }

// @filename: /a.ts