Skip to content

Commit

Permalink
For duplicate source files of the same package, make one redirect to …
Browse files Browse the repository at this point in the history 
…the other (#16274)

* For duplicate source files of the same package, make one redirect to the other

* Add reuseProgramStructure tests

* Copy `sourceFileToPackageId` and `isSourceFileTargetOfRedirect` only if we completely reuse old structure

* Use fallthrough instead of early exit from loop

* Use a set to efficiently detect duplicate package names

* Move map setting outside of createRedirectSourceFile

* Correctly handle seenPackageNames set

* sourceFileToPackageId -> sourceFileToPackageName

* Renames

* Respond to PR comments

* Fix bug where `oldSourceFile !== newSourceFile` because oldSourceFile was a redirect

* Clean up redirectInfo

* Respond to PR comments
  • Loading branch information
Andy authored Aug 9, 2017
1 parent 17a6f7b commit 37b20ee
Show file tree
Hide file tree
Showing 16 changed files with 696 additions and 106 deletions.
15 changes: 7 additions & 8 deletions src/compiler/core.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2210,20 +2210,14 @@ namespace ts {
/** Must have ".d.ts" first because if ".ts" goes first, that will be detected as the extension instead of ".d.ts". */
export const supportedTypescriptExtensionsForExtractExtension: ReadonlyArray<Extension> = [Extension.Dts, Extension.Ts, Extension.Tsx];
export const supportedJavascriptExtensions: ReadonlyArray<Extension> = [Extension.Js, Extension.Jsx];
const allSupportedExtensions = [...supportedTypeScriptExtensions, ...supportedJavascriptExtensions];
const allSupportedExtensions: ReadonlyArray<Extension> = [...supportedTypeScriptExtensions, ...supportedJavascriptExtensions];

export function getSupportedExtensions(options?: CompilerOptions, extraFileExtensions?: ReadonlyArray<JsFileExtensionInfo>): ReadonlyArray<string> {
const needAllExtensions = options && options.allowJs;
if (!extraFileExtensions || extraFileExtensions.length === 0 || !needAllExtensions) {
return needAllExtensions ? allSupportedExtensions : supportedTypeScriptExtensions;
}
const extensions: string[] = allSupportedExtensions.slice(0);
for (const extInfo of extraFileExtensions) {
if (extensions.indexOf(extInfo.extension) === -1) {
extensions.push(extInfo.extension);
}
}
return extensions;
return deduplicate([...allSupportedExtensions, ...extraFileExtensions.map(e => e.extension)]);
}

export function hasJavaScriptFileExtension(fileName: string) {
Expand Down Expand Up @@ -2590,6 +2584,11 @@ namespace ts {
}
Debug.fail(`File ${path} has unknown extension.`);
}

export function isAnySupportedFileExtension(path: string): boolean {
return tryGetExtensionFromPath(path) !== undefined;
}

export function tryGetExtensionFromPath(path: string): Extension | undefined {
return find<Extension>(supportedTypescriptExtensionsForExtractExtension, e => fileExtensionIs(path, e)) || find(supportedJavascriptExtensions, e => fileExtensionIs(path, e));
}
Expand Down
110 changes: 76 additions & 34 deletions src/compiler/moduleNameResolver.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,32 @@ namespace ts {
return compilerOptions.traceResolution && host.trace !== undefined;
}

/**
* Result of trying to resolve a module.
* At least one of `ts` and `js` should be defined, or the whole thing should be `undefined`.
*/
/** Array that is only intended to be pushed to, never read. */
/* @internal */
export interface Push<T> {
push(value: T): void;
}

function withPackageId(packageId: PackageId | undefined, r: PathAndExtension | undefined): Resolved {
return r && { path: r.path, extension: r.ext, packageId };
}

function noPackageId(r: PathAndExtension | undefined): Resolved {
return withPackageId(/*packageId*/ undefined, r);
}

/** Result of trying to resolve a module. */
interface Resolved {
path: string;
extension: Extension;
packageId: PackageId | undefined;
}

/** Result of trying to resolve a module at a file. Needs to have 'packageId' added later. */
interface PathAndExtension {
path: string;
// (Use a different name than `extension` to make sure Resolved isn't assignable to PathAndExtension.)
ext: Extension;
}

/**
Expand All @@ -43,7 +62,7 @@ namespace ts {

function createResolvedModuleWithFailedLookupLocations(resolved: Resolved | undefined, isExternalLibraryImport: boolean, failedLookupLocations: string[]): ResolvedModuleWithFailedLookupLocations {
return {
resolvedModule: resolved && { resolvedFileName: resolved.path, extension: resolved.extension, isExternalLibraryImport },
resolvedModule: resolved && { resolvedFileName: resolved.path, extension: resolved.extension, isExternalLibraryImport, packageId: resolved.packageId },
failedLookupLocations
};
}
Expand All @@ -54,9 +73,16 @@ namespace ts {
traceEnabled: boolean;
}

interface PackageJson {
name?: string;
version?: string;
typings?: string;
types?: string;
main?: string;
}

/** Reads from "main" or "types"/"typings" depending on `extensions`. */
function tryReadPackageJsonFields(readTypes: boolean, packageJsonPath: string, baseDirectory: string, state: ModuleResolutionState): string | undefined {
const jsonContent = readJson(packageJsonPath, state.host);
function tryReadPackageJsonFields(readTypes: boolean, jsonContent: PackageJson, baseDirectory: string, state: ModuleResolutionState): string | undefined {
return readTypes ? tryReadFromField("typings") || tryReadFromField("types") : tryReadFromField("main");

function tryReadFromField(fieldName: "typings" | "types" | "main"): string | undefined {
Expand All @@ -83,7 +109,7 @@ namespace ts {
}
}

function readJson(path: string, host: ModuleResolutionHost): { typings?: string, types?: string, main?: string } {
function readJson(path: string, host: ModuleResolutionHost): PackageJson {
try {
const jsonText = host.readFile(path);
return jsonText ? JSON.parse(jsonText) : {};
Expand Down Expand Up @@ -646,7 +672,7 @@ namespace ts {
if (extension !== undefined) {
const path = tryFile(candidate, failedLookupLocations, /*onlyRecordFailures*/ false, state);
if (path !== undefined) {
return { path, extension };
return { path, extension, packageId: undefined };
}
}

Expand Down Expand Up @@ -709,7 +735,7 @@ namespace ts {
}
const resolved = loadModuleFromNodeModules(extensions, moduleName, containingDirectory, failedLookupLocations, state, cache);
// For node_modules lookups, get the real path so that multiple accesses to an `npm link`-ed module do not create duplicate files.
return resolved && { value: resolved.value && { resolved: { path: realpath(resolved.value.path, host, traceEnabled), extension: resolved.value.extension }, isExternalLibraryImport: true } };
return resolved && { value: resolved.value && { resolved: { ...resolved.value, path: realpath(resolved.value.path, host, traceEnabled) }, isExternalLibraryImport: true } };
}
else {
const candidate = normalizePath(combinePaths(containingDirectory, moduleName));
Expand Down Expand Up @@ -747,7 +773,7 @@ namespace ts {
}
const resolvedFromFile = loadModuleFromFile(extensions, candidate, failedLookupLocations, onlyRecordFailures, state);
if (resolvedFromFile) {
return resolvedFromFile;
return noPackageId(resolvedFromFile);
}
}
if (!onlyRecordFailures) {
Expand All @@ -768,11 +794,15 @@ namespace ts {
return !host.directoryExists || host.directoryExists(directoryName);
}

function loadModuleFromFileNoPackageId(extensions: Extensions, candidate: string, failedLookupLocations: Push<string>, onlyRecordFailures: boolean, state: ModuleResolutionState): Resolved {
return noPackageId(loadModuleFromFile(extensions, candidate, failedLookupLocations, onlyRecordFailures, state));
}

/**
* @param {boolean} onlyRecordFailures - if true then function won't try to actually load files but instead record all attempts as failures. This flag is necessary
* in cases when we know upfront that all load attempts will fail (because containing folder does not exists) however we still need to record all failed lookup locations.
*/
function loadModuleFromFile(extensions: Extensions, candidate: string, failedLookupLocations: Push<string>, onlyRecordFailures: boolean, state: ModuleResolutionState): Resolved | undefined {
function loadModuleFromFile(extensions: Extensions, candidate: string, failedLookupLocations: Push<string>, onlyRecordFailures: boolean, state: ModuleResolutionState): PathAndExtension | undefined {
// First, try adding an extension. An import of "foo" could be matched by a file "foo.ts", or "foo.js" by "foo.js.ts"
const resolvedByAddingExtension = tryAddingExtensions(candidate, extensions, failedLookupLocations, onlyRecordFailures, state);
if (resolvedByAddingExtension) {
Expand All @@ -792,7 +822,7 @@ namespace ts {
}

/** Try to return an existing file that adds one of the `extensions` to `candidate`. */
function tryAddingExtensions(candidate: string, extensions: Extensions, failedLookupLocations: Push<string>, onlyRecordFailures: boolean, state: ModuleResolutionState): Resolved | undefined {
function tryAddingExtensions(candidate: string, extensions: Extensions, failedLookupLocations: Push<string>, onlyRecordFailures: boolean, state: ModuleResolutionState): PathAndExtension | undefined {
if (!onlyRecordFailures) {
// check if containing folder exists - if it doesn't then just record failures for all supported extensions without disk probing
const directory = getDirectoryPath(candidate);
Expand All @@ -810,9 +840,9 @@ namespace ts {
return tryExtension(Extension.Js) || tryExtension(Extension.Jsx);
}

function tryExtension(extension: Extension): Resolved | undefined {
const path = tryFile(candidate + extension, failedLookupLocations, onlyRecordFailures, state);
return path && { path, extension };
function tryExtension(ext: Extension): PathAndExtension | undefined {
const path = tryFile(candidate + ext, failedLookupLocations, onlyRecordFailures, state);
return path && { path, ext };
}
}

Expand All @@ -838,12 +868,23 @@ namespace ts {
function loadNodeModuleFromDirectory(extensions: Extensions, candidate: string, failedLookupLocations: Push<string>, onlyRecordFailures: boolean, state: ModuleResolutionState, considerPackageJson = true): Resolved | undefined {
const directoryExists = !onlyRecordFailures && directoryProbablyExists(candidate, state.host);

let packageId: PackageId | undefined;

if (considerPackageJson) {
const packageJsonPath = pathToPackageJson(candidate);
if (directoryExists && state.host.fileExists(packageJsonPath)) {
const fromPackageJson = loadModuleFromPackageJson(packageJsonPath, extensions, candidate, failedLookupLocations, state);
if (state.traceEnabled) {
trace(state.host, Diagnostics.Found_package_json_at_0, packageJsonPath);
}
const jsonContent = readJson(packageJsonPath, state.host);

if (typeof jsonContent.name === "string" && typeof jsonContent.version === "string") {
packageId = { name: jsonContent.name, version: jsonContent.version };
}

const fromPackageJson = loadModuleFromPackageJson(jsonContent, extensions, candidate, failedLookupLocations, state);
if (fromPackageJson) {
return fromPackageJson;
return withPackageId(packageId, fromPackageJson);
}
}
else {
Expand All @@ -855,15 +896,11 @@ namespace ts {
}
}

return loadModuleFromFile(extensions, combinePaths(candidate, "index"), failedLookupLocations, !directoryExists, state);
return withPackageId(packageId, loadModuleFromFile(extensions, combinePaths(candidate, "index"), failedLookupLocations, !directoryExists, state));
}

function loadModuleFromPackageJson(packageJsonPath: string, extensions: Extensions, candidate: string, failedLookupLocations: Push<string>, state: ModuleResolutionState): Resolved | undefined {
if (state.traceEnabled) {
trace(state.host, Diagnostics.Found_package_json_at_0, packageJsonPath);
}

const file = tryReadPackageJsonFields(extensions !== Extensions.JavaScript, packageJsonPath, candidate, state);
function loadModuleFromPackageJson(jsonContent: PackageJson, extensions: Extensions, candidate: string, failedLookupLocations: Push<string>, state: ModuleResolutionState): PathAndExtension | undefined {
const file = tryReadPackageJsonFields(extensions !== Extensions.JavaScript, jsonContent, candidate, state);
if (!file) {
return undefined;
}
Expand All @@ -883,13 +920,18 @@ namespace ts {
// Even if extensions is DtsOnly, we can still look up a .ts file as a result of package.json "types"
const nextExtensions = extensions === Extensions.DtsOnly ? Extensions.TypeScript : extensions;
// Don't do package.json lookup recursively, because Node.js' package lookup doesn't.
return nodeLoadModuleByRelativeName(nextExtensions, file, failedLookupLocations, onlyRecordFailures, state, /*considerPackageJson*/ false);
const result = nodeLoadModuleByRelativeName(nextExtensions, file, failedLookupLocations, onlyRecordFailures, state, /*considerPackageJson*/ false);
if (result) {
// It won't have a `packageId` set, because we disabled `considerPackageJson`.
Debug.assert(result.packageId === undefined);
return { path: result.path, ext: result.extension };
}
}

/** Resolve from an arbitrarily specified file. Return `undefined` if it has an unsupported extension. */
function resolvedIfExtensionMatches(extensions: Extensions, path: string): Resolved | undefined {
const extension = tryGetExtensionFromPath(path);
return extension !== undefined && extensionIsOk(extensions, extension) ? { path, extension } : undefined;
function resolvedIfExtensionMatches(extensions: Extensions, path: string): PathAndExtension | undefined {
const ext = tryGetExtensionFromPath(path);
return ext !== undefined && extensionIsOk(extensions, ext) ? { path, ext } : undefined;
}

/** True if `extension` is one of the supported `extensions`. */
Expand All @@ -911,7 +953,7 @@ namespace ts {
function loadModuleFromNodeModulesFolder(extensions: Extensions, moduleName: string, nodeModulesFolder: string, nodeModulesFolderExists: boolean, failedLookupLocations: Push<string>, state: ModuleResolutionState): Resolved | undefined {
const candidate = normalizePath(combinePaths(nodeModulesFolder, moduleName));

return loadModuleFromFile(extensions, candidate, failedLookupLocations, !nodeModulesFolderExists, state) ||
return loadModuleFromFileNoPackageId(extensions, candidate, failedLookupLocations, !nodeModulesFolderExists, state) ||
loadNodeModuleFromDirectory(extensions, candidate, failedLookupLocations, !nodeModulesFolderExists, state);
}

Expand Down Expand Up @@ -996,7 +1038,7 @@ namespace ts {
if (traceEnabled) {
trace(host, Diagnostics.Resolution_for_module_0_was_found_in_cache, moduleName);
}
return { value: result.resolvedModule && { path: result.resolvedModule.resolvedFileName, extension: result.resolvedModule.extension } };
return { value: result.resolvedModule && { path: result.resolvedModule.resolvedFileName, extension: result.resolvedModule.extension, packageId: result.resolvedModule.packageId } };
}
}

Expand All @@ -1010,7 +1052,7 @@ namespace ts {
return createResolvedModuleWithFailedLookupLocations(resolved && resolved.value, /*isExternalLibraryImport*/ false, failedLookupLocations);

function tryResolve(extensions: Extensions): SearchResult<Resolved> {
const resolvedUsingSettings = tryLoadModuleUsingOptionalResolutionSettings(extensions, moduleName, containingDirectory, loadModuleFromFile, failedLookupLocations, state);
const resolvedUsingSettings = tryLoadModuleUsingOptionalResolutionSettings(extensions, moduleName, containingDirectory, loadModuleFromFileNoPackageId, failedLookupLocations, state);
if (resolvedUsingSettings) {
return { value: resolvedUsingSettings };
}
Expand All @@ -1024,7 +1066,7 @@ namespace ts {
return resolutionFromCache;
}
const searchName = normalizePath(combinePaths(directory, moduleName));
return toSearchResult(loadModuleFromFile(extensions, searchName, failedLookupLocations, /*onlyRecordFailures*/ false, state));
return toSearchResult(loadModuleFromFileNoPackageId(extensions, searchName, failedLookupLocations, /*onlyRecordFailures*/ false, state));
});
if (resolved) {
return resolved;
Expand All @@ -1036,7 +1078,7 @@ namespace ts {
}
else {
const candidate = normalizePath(combinePaths(containingDirectory, moduleName));
return toSearchResult(loadModuleFromFile(extensions, candidate, failedLookupLocations, /*onlyRecordFailures*/ false, state));
return toSearchResult(loadModuleFromFileNoPackageId(extensions, candidate, failedLookupLocations, /*onlyRecordFailures*/ false, state));
}
}
}
Expand Down
Loading

1 comment on commit 37b20ee

@mightyiam
Copy link

Choose a reason for hiding this comment

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

Thanks a lot for this. @yarom82, you may like to see this.

Please sign in to comment.