Skip to content
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

Load JavaScript modules from Node packages #7075

Merged
merged 20 commits into from
Jun 28, 2016
Merged
Show file tree
Hide file tree
Changes from 18 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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,4 @@ internal/
**/.vscode
!**/.vscode/tasks.json
!tests/cases/projects/projectOption/**/node_modules
!tests/cases/projects/NodeModulesSearch/**/*
5 changes: 5 additions & 0 deletions src/compiler/commandLineParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,11 @@ namespace ts {
type: "boolean",
description: Diagnostics.Do_not_emit_use_strict_directives_in_module_output
},
{
name: "maxNodeModuleJsDepth",
type: "number",
description: Diagnostics.The_maximum_dependency_depth_to_search_under_node_modules_and_load_JavaScript_files
},
{
name: "listEmittedFiles",
type: "boolean"
Expand Down
9 changes: 8 additions & 1 deletion src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -2804,7 +2804,14 @@
"category": "Message",
"code": 6135
},

"The maximum dependency depth to search under node_modules and load JavaScript files": {
"category": "Message",
"code": 6136
},
"No types specified in 'package.json' but 'allowJs' is set, so returning 'main' value of '{0}'": {
"category": "Message",
"code": 6137
},
"Variable '{0}' implicitly has an '{1}' type.": {
"category": "Error",
"code": 7005
Expand Down
84 changes: 72 additions & 12 deletions src/compiler/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,10 +130,10 @@ namespace ts {
}

function tryReadTypesSection(packageJsonPath: string, baseDirectory: string, state: ModuleResolutionState): string {
let jsonContent: { typings?: string, types?: string };
let jsonContent: { typings?: string, types?: string, main?: string };
try {
const jsonText = state.host.readFile(packageJsonPath);
jsonContent = jsonText ? <{ typings?: string, types?: string }>JSON.parse(jsonText) : {};
jsonContent = jsonText ? <{ typings?: string, types?: string, main?: string }>JSON.parse(jsonText) : {};
}
catch (e) {
// gracefully handle if readFile fails or returns not JSON
Expand Down Expand Up @@ -173,6 +173,14 @@ namespace ts {
}
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) {
trace(state.host, Diagnostics.No_types_specified_in_package_json_but_allowJs_is_set_so_returning_main_value_of_0, jsonContent.main);
}
const mainFilePath = normalizePath(combinePaths(baseDirectory, jsonContent.main));
return mainFilePath;
}
return undefined;
}

Expand Down Expand Up @@ -610,7 +618,7 @@ namespace ts {
failedLookupLocations, supportedExtensions, state);

let isExternalLibraryImport = false;
if (!resolvedFileName) {
if (!resolvedFileName) {
if (moduleHasNonRelativeName(moduleName)) {
if (traceEnabled) {
trace(host, Diagnostics.Loading_module_0_from_node_modules_folder, moduleName);
Expand Down Expand Up @@ -740,12 +748,13 @@ namespace ts {
const nodeModulesFolder = combinePaths(directory, "node_modules");
const nodeModulesFolderExists = directoryProbablyExists(nodeModulesFolder, state.host);
const candidate = normalizePath(combinePaths(nodeModulesFolder, moduleName));
// Load only typescript files irrespective of allowJs option if loading from node modules
let result = loadModuleFromFile(candidate, supportedTypeScriptExtensions, failedLookupLocations, !nodeModulesFolderExists, state);
const supportedExtensions = getSupportedExtensions(state.compilerOptions);

let result = loadModuleFromFile(candidate, supportedExtensions, failedLookupLocations, !nodeModulesFolderExists, state);
if (result) {
return result;
}
result = loadNodeModuleFromDirectory(supportedTypeScriptExtensions, candidate, failedLookupLocations, !nodeModulesFolderExists, state);
result = loadNodeModuleFromDirectory(supportedExtensions, candidate, failedLookupLocations, !nodeModulesFolderExists, state);
if (result) {
return result;
}
Expand Down Expand Up @@ -1057,6 +1066,23 @@ namespace ts {
let resolvedTypeReferenceDirectives: Map<ResolvedTypeReferenceDirective> = {};
let fileProcessingDiagnostics = createDiagnosticCollection();

// The below settings are to track if a .js file should be add to the program if loaded via searching under node_modules.
// This works as imported modules are discovered recursively in a depth first manner, specifically:
// - For each root file, findSourceFile is called.
// - This calls processImportedModules for each module imported in the source file.
// - This calls resolveModuleNames, and then calls findSourceFile for each resolved module.
// As all these operations happen - and are nested - within the createProgram call, they close over the below variables.
// The current resolution depth is tracked by incrementing/decrementing as the depth first search progresses.
const maxNodeModulesJsDepth = typeof options.maxNodeModuleJsDepth === "number" ? options.maxNodeModuleJsDepth : 2;
let currentNodeModulesJsDepth = 0;

// If a module has some of its imports skipped due to being at the depth limit under node_modules, then track
// this, as it may be imported at a shallower depth later, and then it will need its skipped imports processed.
const modulesWithElidedImports: Map<boolean> = {};

// Track source files that are JavaScript files found by searching under node_modules, as these shouldn't be compiled.
const jsFilesFoundSearchingNodeModules: Map<boolean> = {};

const start = new Date().getTime();

host = host || createCompilerHost(options);
Expand Down Expand Up @@ -1211,6 +1237,7 @@ namespace ts {
(oldOptions.rootDir !== options.rootDir) ||
(oldOptions.configFilePath !== options.configFilePath) ||
(oldOptions.baseUrl !== options.baseUrl) ||
(oldOptions.maxNodeModuleJsDepth !== options.maxNodeModuleJsDepth) ||
!arrayIsEqualTo(oldOptions.typeRoots, oldOptions.typeRoots) ||
!arrayIsEqualTo(oldOptions.rootDirs, options.rootDirs) ||
!mapIsEqualTo(oldOptions.paths, options.paths)) {
Expand Down Expand Up @@ -1334,7 +1361,10 @@ namespace ts {
getNewLine: () => host.getNewLine(),
getSourceFile: program.getSourceFile,
getSourceFileByPath: program.getSourceFileByPath,
getSourceFiles: program.getSourceFiles,
getSourceFiles: () => filter(program.getSourceFiles(),
// Remove JavaScript files found by searching node_modules from the source files to emit
sourceFile => !lookUp(jsFilesFoundSearchingNodeModules, sourceFile.path)
Copy link
Contributor

Choose a reason for hiding this comment

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

do not think we should do this. the file is part of the program. getSourceFle, and getSourceFile, should give an accurate representation of the files in the program. you could get an error in this file, or get a definition in this file, and using the file name you should be able to get to the SourceFile object using getSourceFile and you should also be able to find it if you enumerate the getSourceFiles.

I would say create a new method, Program.jsFilesFoundSearchingNodeModules(path) that the emitter checks and skips emitting if true.

),
writeFile: writeFileCallback || (
(fileName, data, writeByteOrderMark, onError, sourceFiles) => host.writeFile(fileName, data, writeByteOrderMark, onError, sourceFiles)),
isEmitBlocked,
Expand Down Expand Up @@ -1869,6 +1899,14 @@ namespace ts {
reportFileNamesDifferOnlyInCasingError(fileName, file.fileName, refFile, refPos, refEnd);
}

// See if we need to reprocess the imports due to prior skipped imports
if (file && lookUp(modulesWithElidedImports, file.path)) {
if (currentNodeModulesJsDepth < maxNodeModulesJsDepth) {
modulesWithElidedImports[file.path] = false;
processImportedModules(file, getDirectoryPath(fileName));
}
}

return file;
}

Expand Down Expand Up @@ -2007,16 +2045,38 @@ namespace ts {
for (let i = 0; i < moduleNames.length; i++) {
const resolution = resolutions[i];
setResolvedModule(file, moduleNames[i], resolution);
const resolvedPath = resolution ? toPath(resolution.resolvedFileName, currentDirectory, getCanonicalFileName) : undefined;

// add file to program only if:
// - resolution was successful
// - noResolve is falsy
// - module name comes from the list of imports
const shouldAddFile = resolution &&
!options.noResolve &&
i < file.imports.length;
// - it's not a top level JavaScript module that exceeded the search max
const isJsFileUnderNodeModules = resolution && resolution.isExternalLibraryImport &&
hasJavaScriptFileExtension(resolution.resolvedFileName);

if (isJsFileUnderNodeModules) {
jsFilesFoundSearchingNodeModules[resolvedPath] = true;
currentNodeModulesJsDepth++;
}

const elideImport = isJsFileUnderNodeModules && currentNodeModulesJsDepth > maxNodeModulesJsDepth;
const shouldAddFile = resolution && !options.noResolve && i < file.imports.length && !elideImport;

if (elideImport) {
modulesWithElidedImports[file.path] = true;
}
else if (shouldAddFile) {
findSourceFile(resolution.resolvedFileName,
resolvedPath,
/*isDefaultLib*/ false, /*isReference*/ false,
file,
skipTrivia(file.text, file.imports[i].pos),
file.imports[i].end);
}

if (shouldAddFile) {
findSourceFile(resolution.resolvedFileName, toPath(resolution.resolvedFileName, currentDirectory, getCanonicalFileName), /*isDefaultLib*/ false, /*isReference*/ false, file, skipTrivia(file.text, file.imports[i].pos), file.imports[i].end);
if (isJsFileUnderNodeModules) {
currentNodeModulesJsDepth--;
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2533,6 +2533,7 @@ namespace ts {
declaration?: boolean;
declarationDir?: string;
/* @internal */ diagnostics?: boolean;
disableSizeLimit?: boolean;
emitBOM?: boolean;
emitDecoratorMetadata?: boolean;
experimentalDecorators?: boolean;
Expand All @@ -2548,6 +2549,7 @@ namespace ts {
/*@internal*/listFiles?: boolean;
locale?: string;
mapRoot?: string;
maxNodeModuleJsDepth?: number;
module?: ModuleKind;
moduleResolution?: ModuleResolutionKind;
newLine?: NewLineKind;
Expand Down Expand Up @@ -2586,7 +2588,6 @@ namespace ts {
/* @internal */ suppressOutputPathCheck?: boolean;
target?: ScriptTarget;
traceResolution?: boolean;
disableSizeLimit?: boolean;
types?: string[];
/** Paths used to used to compute primary types search locations */
typeRoots?: string[];
Expand Down
9 changes: 5 additions & 4 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2275,6 +2275,7 @@ namespace ts {
else {
const sourceFiles = targetSourceFile === undefined ? host.getSourceFiles() : [targetSourceFile];
for (const sourceFile of sourceFiles) {
// Don't emit if source file is a declaration file
if (!isDeclarationFile(sourceFile)) {
onSingleFileEmit(host, sourceFile);
}
Expand Down Expand Up @@ -2308,10 +2309,10 @@ namespace ts {

function onBundledEmit(host: EmitHost) {
// Can emit only sources that are not declaration file and are either non module code or module with --module or --target es6 specified
const bundledSources = filter(host.getSourceFiles(), sourceFile =>
!isDeclarationFile(sourceFile) // Not a declaration file
&& (!isExternalModule(sourceFile) || !!getEmitModuleKind(options))); // and not a module, unless module emit enabled

const bundledSources = filter(host.getSourceFiles(),
sourceFile => !isDeclarationFile(sourceFile) && // Not a declaration file
(!isExternalModule(sourceFile) || // non module file
!!getEmitModuleKind(options))); // module that can emit - note falsy value from getEmitModuleKind means the module kind that shouldn't be emitted
if (bundledSources.length) {
const jsFilePath = options.outFile || options.out;
const emitFileNames: EmitFileNames = {
Expand Down
12 changes: 3 additions & 9 deletions src/harness/runnerbase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,18 +31,12 @@ abstract class RunnerBase {

/** Replaces instances of full paths with fileNames only */
static removeFullPaths(path: string) {
let fixedPath = path;

// full paths either start with a drive letter or / for *nix, shouldn't have \ in the path at this point
const fullPath = /(\w+:|\/)?([\w+\-\.]|\/)*\.tsx?/g;
const fullPathList = fixedPath.match(fullPath);
if (fullPathList) {
fullPathList.forEach((match: string) => fixedPath = fixedPath.replace(match, Harness.Path.getFileName(match)));
}
// If its a full path (starts with "C:" or "/") replace with just the filename
let fixedPath = /^(\w:|\/)/.test(path) ? Harness.Path.getFileName(path) : path;

// when running in the browser the 'full path' is the host name, shows up in error baselines
const localHost = /http:\/localhost:\d+/g;
fixedPath = fixedPath.replace(localHost, "");
return fixedPath;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@ tests/cases/compiler/declarationEmit_invalidReference2.ts(1,1): error TS6053: Fi
==== tests/cases/compiler/declarationEmit_invalidReference2.ts (1 errors) ====
/// <reference path="invalid.ts" />
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS6053: File 'invalid.ts' not found.
!!! error TS6053: File 'tests/cases/compiler/invalid.ts' not found.
var x = 0;
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
error TS5055: Cannot write file 'tests/cases/compiler/a.d.ts' because it would overwrite input file.


!!! error TS5055: Cannot write file 'a.d.ts' because it would overwrite input file.
!!! error TS5055: Cannot write file 'tests/cases/compiler/a.d.ts' because it would overwrite input file.
==== tests/cases/compiler/a.d.ts (0 errors) ====

declare class c {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
error TS5055: Cannot write file 'tests/cases/compiler/out.d.ts' because it would overwrite input file.


!!! error TS5055: Cannot write file 'out.d.ts' because it would overwrite input file.
!!! error TS5055: Cannot write file 'tests/cases/compiler/out.d.ts' because it would overwrite input file.
==== tests/cases/compiler/out.d.ts (0 errors) ====

declare class c {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ tests/cases/compiler/exportStarFromEmptyModule_module4.ts(4,5): error TS2339: Pr
==== tests/cases/compiler/exportStarFromEmptyModule_module3.ts (1 errors) ====
export * from "./exportStarFromEmptyModule_module2";
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS2306: File 'exportStarFromEmptyModule_module2.ts' is not a module.
!!! error TS2306: File 'tests/cases/compiler/exportStarFromEmptyModule_module2.ts' is not a module.
export * from "./exportStarFromEmptyModule_module1";

export class A {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ tests/cases/conformance/externalModules/foo_1.ts(1,22): error TS2306: File 'test
==== tests/cases/conformance/externalModules/foo_1.ts (1 errors) ====
import foo = require("./foo_0");
~~~~~~~~~
!!! error TS2306: File 'foo_0.ts' is not a module.
!!! error TS2306: File 'tests/cases/conformance/externalModules/foo_0.ts' is not a module.
// Import should fail. foo_0 not an external module
if(foo.answer === 42){

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ tests/cases/compiler/invalidTripleSlashReference.ts(2,1): error TS6053: File 'te
==== tests/cases/compiler/invalidTripleSlashReference.ts (2 errors) ====
/// <reference path='filedoesnotexist.ts'/>
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS6053: File 'filedoesnotexist.ts' not found.
!!! error TS6053: File 'tests/cases/compiler/filedoesnotexist.ts' not found.
/// <reference path='otherdoesnotexist.d.ts'/>
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS6053: File 'otherdoesnotexist.d.ts' not found.
!!! error TS6053: File 'tests/cases/compiler/otherdoesnotexist.d.ts' not found.

// this test doesn't actually give the errors you want due to the way the compiler reports errors
var x = 1;
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
error TS5055: Cannot write file 'tests/cases/compiler/a.d.ts' because it would overwrite input file.


!!! error TS5055: Cannot write file 'a.d.ts' because it would overwrite input file.
!!! error TS5055: Cannot write file 'tests/cases/compiler/a.d.ts' because it would overwrite input file.
==== tests/cases/compiler/a.ts (0 errors) ====
class c {
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
error TS5055: Cannot write file 'tests/cases/compiler/b.d.ts' because it would overwrite input file.


!!! error TS5055: Cannot write file 'b.d.ts' because it would overwrite input file.
!!! error TS5055: Cannot write file 'tests/cases/compiler/b.d.ts' because it would overwrite input file.
==== tests/cases/compiler/a.ts (0 errors) ====
class c {
}
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/library-reference-5.errors.txt
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
==== /node_modules/bar/index.d.ts (1 errors) ====
/// <reference types="alpha" />
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! message TS4090: Conflicting library definitions for 'alpha' found at 'index.d.ts' and 'index.d.ts'. Copy the correct file to the 'typings' folder to resolve this conflict.
!!! message TS4090: Conflicting library definitions for 'alpha' found at '/node_modules/bar/node_modules/alpha/index.d.ts' and '/node_modules/foo/node_modules/alpha/index.d.ts'. Copy the correct file to the 'typings' folder to resolve this conflict.
Copy link
Member Author

Choose a reason for hiding this comment

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

Fixing the regular expression makes messages such as this useful.

declare var bar: any;

==== /node_modules/bar/node_modules/alpha/index.d.ts (0 errors) ====
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/parserRealSource1.errors.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ tests/cases/conformance/parser/ecmascript5/parserRealSource1.ts(4,1): error TS60

///<reference path='typescript.ts' />
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS6053: File 'typescript.ts' not found.
!!! error TS6053: File 'tests/cases/conformance/parser/ecmascript5/typescript.ts' not found.

module TypeScript {
export module CompilerDiagnostics {
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/parserRealSource10.errors.txt
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ tests/cases/conformance/parser/ecmascript5/parserRealSource10.ts(449,40): error

///<reference path='typescript.ts' />
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS6053: File 'typescript.ts' not found.
!!! error TS6053: File 'tests/cases/conformance/parser/ecmascript5/typescript.ts' not found.

module TypeScript {
export enum TokenID {
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/parserRealSource11.errors.txt
Original file line number Diff line number Diff line change
Expand Up @@ -523,7 +523,7 @@ tests/cases/conformance/parser/ecmascript5/parserRealSource11.ts(2356,48): error

///<reference path='typescript.ts' />
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS6053: File 'typescript.ts' not found.
!!! error TS6053: File 'tests/cases/conformance/parser/ecmascript5/typescript.ts' not found.

module TypeScript {
export class ASTSpan {
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/parserRealSource12.errors.txt
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ tests/cases/conformance/parser/ecmascript5/parserRealSource12.ts(524,30): error

///<reference path='typescript.ts' />
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS6053: File 'typescript.ts' not found.
!!! error TS6053: File 'tests/cases/conformance/parser/ecmascript5/typescript.ts' not found.

module TypeScript {
export interface IAstWalker {
Expand Down
Loading