diff --git a/src/compiler/program.ts b/src/compiler/program.ts index da9fe1b2ba9a0..385a0543298cc 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -1107,7 +1107,7 @@ namespace ts { } function resolveModuleNamesReusingOldState(moduleNames: string[], file: SourceFile): readonly ResolvedModuleFull[] { - if (structuralIsReused === StructureIsReused.Not && !file.ambientModuleNames.length) { + if (!doesStructuralPermitResolutionsReuse() && !file.ambientModuleNames.length) { // If the old program state does not permit reusing resolutions and `file` does not contain locally defined ambient modules, // the best we can do is fallback to the default logic. return resolveModuleNamesWorker(moduleNames, file, /*reusedNames*/ undefined); @@ -1218,6 +1218,31 @@ namespace ts { return result; + function doesStructuralPermitResolutionsReuse(): boolean { + switch (structuralIsReused) { + case StructureIsReused.Not: return false; + case StructureIsReused.SafeProjectReferenceModules: + return file.resolvedModules !== undefined && + isSourceOfProjectReferenceRedirect(file.originalFileName); + case StructureIsReused.SafeModules: return true; + case StructureIsReused.Completely: return true; + + // It's possible for resolveModuleNamesReusingOldState to be called by + // tryReuseStructureFromOldProgram before structuralIsReused has been defined. + // In this case, the caller (tryReuseStructureFromOldProgram) expects this + // function to reuse resolutions for its checks. Returning true to accommodate + // for that specific scenario. + // + // TODO: Clean this because the above scenario is a bit unexpected. This case + // should ideally return false or throw. + case undefined: return true; + + // The above cases should exhaustively cover all branches. Defaulting to false + // since that's safer in unexpected situations. + default: return false; + } + } + // If we change our policy of rechecking failed lookups on each program create, // we should adjust the value returned here. function moduleNameResolvesToAmbientModuleInNonModifiedFile(moduleName: string): boolean { @@ -1271,7 +1296,10 @@ namespace ts { function tryReuseStructureFromOldProgram(): StructureIsReused { if (!oldProgram) { - return StructureIsReused.Not; + // During initial program creation, root files may import files from project + // references that were previously loaded. Those resolutions are safe to reuse + // since another program instance kept them up to date. + return StructureIsReused.SafeProjectReferenceModules; } // check properties that can affect structure of the program or module resolution strategy diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 2c37d20298dca..35374d2af230b 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -3822,10 +3822,12 @@ namespace ts { } /* @internal */ + /** "Structure" refers to the SourceFile graph of a Program linked by module resolutions. */ export const enum StructureIsReused { - Not = 0, - SafeModules = 1 << 0, - Completely = 1 << 1, + Not = 0, // The entire Program must be (re)created. + SafeProjectReferenceModules = 1 << 0, // SourceFile objects need to be rediscovered, but module resolutions within project reference sources may be reused. + SafeModules = 1 << 1, // SourceFile objects need to be rediscovered, but module resolutions can be reused. + Completely = 1 << 2, // SourceFile objects and module resolutions can be reused from an old Program. } export type CustomTransformerFactory = (context: TransformationContext) => CustomTransformer; diff --git a/src/testRunner/unittests/reuseProgramStructure.ts b/src/testRunner/unittests/reuseProgramStructure.ts index 8c3f9d6a94fb7..d007f125c61c0 100644 --- a/src/testRunner/unittests/reuseProgramStructure.ts +++ b/src/testRunner/unittests/reuseProgramStructure.ts @@ -793,6 +793,92 @@ namespace ts { } }); + it("can reuse module resolutions within project references", () => { + const commonOptions = { + composite: true, + declaration: true, + target: ScriptTarget.ES2015, + traceResolution: true, + moduleResolution: ModuleResolutionKind.Classic, + }; + const tsconfigA = { + compilerOptions: commonOptions, + include: ["src"] + }; + const tsconfigB = { + compilerOptions: { + ...commonOptions, + paths: { + a: ["/a/src/index.ts"] + } + }, + projectReferences: [ + { path: "/a" } + ] + }; + + const files = [ + { name: "/a/src/x.ts", text: SourceText.New("", "export const x = 1;", "") }, + { name: "/a/src/index.ts", text: SourceText.New("", "export { x } from './x';", "") }, + { name: "/a/tsconfig.json", text: SourceText.New("", "", JSON.stringify(tsconfigA)) }, + { name: "/b/src/b.ts", text: SourceText.New("", "import { x } from 'a';", "") }, + ]; + const rootNamesA = ["/a/src/index.ts", "/a/src/x.ts"]; + const rootNamesB = ["/b/src/b.ts"]; + + const host = createTestCompilerHost(files, commonOptions.target); + + // Instead of hard-coding file system entries for this test, we could also write a function that more + // generally transforms a list of files into a tree of FileSystemEntries. + function getFileSystemEntries(path: string) { + const mapPathToFileSystemEntries: { [path: string]: FileSystemEntries | undefined } = { + "/a": { files: [], directories: ["src"] }, + "/a/src": { files: ["index.ts", "x.ts"], directories: [] } + }; + + const entries = mapPathToFileSystemEntries[path]; + if (!entries) { + throw new Error(`Unexpected path "${path}" requested from readDirectory. Test is broken.`); + } + return entries; + } + + host.readDirectory = (rootDir, extensions, excludes, includes, depth) => + matchFiles( + rootDir, + extensions, + excludes, + includes, + /*useCaseSensitiveFileNames*/ true, + /*currentDirectory*/ "/", + depth, + getFileSystemEntries, + /*realpath*/ path => path); + + createProgram(rootNamesA, tsconfigA.compilerOptions, host); + createProgram({ + rootNames: rootNamesB, + options: tsconfigB.compilerOptions, + host, + projectReferences: tsconfigB.projectReferences + }); + + // Resolution should not be performed for "./x" from "/a/src/index.ts" multiple times. + assert.deepEqual(host.getTrace(), [ + "======== Resolving module './x' from '/a/src/index.ts'. ========", + "Explicitly specified module resolution kind: 'Classic'.", + "File '/a/src/x.ts' exist - use it as a name resolution result.", + "======== Module name './x' was successfully resolved to '/a/src/x.ts'. ========", + "======== Resolving module 'a' from '/b/src/b.ts'. ========", + "Explicitly specified module resolution kind: 'Classic'.", + "'paths' option is specified, looking for a pattern to match module name 'a'.", + "Module name 'a', matched pattern 'a'.", + "Trying substitution '/a/src/index.ts', candidate module location: '/a/src/index.ts'.", + "File '/a/src/index.ts' exist - use it as a name resolution result.", + "======== Module name 'a' was successfully resolved to '/a/src/index.ts'. ========", + ], "should reuse resolution to /a/src/x.ts"); + }); + describe("redirects", () => { const axIndex = "/node_modules/a/node_modules/x/index.d.ts"; const axPackage = "/node_modules/a/node_modules/x/package.json";