Skip to content

Commit c968b36

Browse files
vladimavladima
committed
addressed PR feedback
1 parent df508de commit c968b36

6 files changed

Lines changed: 39 additions & 46 deletions

File tree

‎src/compiler/checker.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -899,7 +899,7 @@ namespace ts {
899899
}
900900
}
901901

902-
let fileName = getResolvedModuleFileName(getSourceFile(location), moduleReferenceLiteral);
902+
let fileName = getResolvedModuleFileName(getSourceFile(location), moduleReferenceLiteral.text);
903903
let sourceFile = fileName && host.getSourceFile(fileName);
904904
if (sourceFile) {
905905
if (sourceFile.symbol) {

‎src/compiler/program.ts‎

Lines changed: 20 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -163,10 +163,14 @@ namespace ts {
163163

164164
let filesByName = createFileMap<SourceFile>(fileName => host.getCanonicalFileName(fileName));
165165

166-
// if old program was provided by has different target module kind - assume that it cannot be reused
167-
// different module kind can lead to different way of resolving modules
168-
if (oldProgram && oldProgram.getCompilerOptions().module !== options.module) {
169-
oldProgram = undefined;
166+
if (oldProgram) {
167+
let oldOptions = oldProgram.getCompilerOptions();
168+
if ((oldOptions.module !== options.module) ||
169+
(oldOptions.noResolve !== options.noResolve) ||
170+
(oldOptions.target !== options.target) ||
171+
(oldOptions.noLib !== options.noLib)) {
172+
oldProgram = undefined;
173+
}
170174
}
171175

172176
if (!tryReuseStructureFromOldProgram()) {
@@ -222,10 +226,6 @@ namespace ts {
222226
}
223227

224228
function tryReuseStructureFromOldProgram(): boolean {
225-
if (!host.hasChanges) {
226-
// host does not support method 'hasChanges'
227-
return false;
228-
}
229229
if (!oldProgram) {
230230
return false;
231231
}
@@ -234,28 +234,19 @@ namespace ts {
234234

235235
// there is an old program, check if we can reuse its structure
236236
let oldRootNames = oldProgram.getRootFileNames();
237-
if (rootNames.length !== oldRootNames.length) {
238-
// different amount of root names - structure cannot be reused
237+
if (!arrayIsEqualTo(oldRootNames, rootNames)) {
239238
return false;
240239
}
241240

242-
for (let i = 0; i < rootNames.length; i++) {
243-
if (oldRootNames[i] !== rootNames[i]) {
244-
// different order of root names - structure cannot be reused
245-
return false;
246-
}
247-
}
248-
249241
// check if program source files has changed in the way that can affect structure of the program
250242
let newSourceFiles: SourceFile[] = [];
251243
for (let oldSourceFile of oldProgram.getSourceFiles()) {
252-
let newSourceFile: SourceFile;
253-
if (host.hasChanges(oldSourceFile)) {
254-
newSourceFile = host.getSourceFile(oldSourceFile.fileName, options.target);
255-
if (!newSourceFile) {
256-
return false;
257-
}
258-
244+
let newSourceFile = host.getSourceFile(oldSourceFile.fileName, options.target);
245+
if (!newSourceFile) {
246+
return false;
247+
}
248+
249+
if (oldSourceFile !== newSourceFile) {
259250
// check tripleslash references
260251
if (!arrayIsEqualTo(oldSourceFile.referencedFiles, newSourceFile.referencedFiles, fileReferenceIsEqualTo)) {
261252
// tripleslash references has changed
@@ -420,7 +411,7 @@ namespace ts {
420411
}
421412

422413
function moduleNameIsEqualTo(a: LiteralExpression, b: LiteralExpression): boolean {
423-
return a.text ===b.text;
414+
return a.text === b.text;
424415
}
425416

426417
function collectExternalModuleReferences(file: SourceFile): void {
@@ -603,7 +594,7 @@ namespace ts {
603594
checkImports: {
604595
if (file.resolvedModules) {
605596
for (let moduleName of file.imports) {
606-
if (!hasResolvedModuleName(file, moduleName)) {
597+
if (!hasResolvedModuleName(file, moduleName.text)) {
607598
break checkImports;
608599
}
609600
}
@@ -640,7 +631,7 @@ namespace ts {
640631
if (existingResolutions && hasProperty(existingResolutions, moduleNameExpr.text)) {
641632
let fileName = existingResolutions[moduleNameExpr.text];
642633
// use existing resolution
643-
setResolvedModuleName(file, moduleNameExpr, fileName);
634+
setResolvedModuleName(file, moduleNameExpr.text, fileName);
644635
if (fileName) {
645636
findModuleSourceFile(fileName, moduleNameExpr);
646637
}
@@ -651,7 +642,7 @@ namespace ts {
651642
searchName = normalizePath(combinePaths(searchPath, moduleNameExpr.text));
652643
let referencedSourceFile = forEach(supportedExtensions, extension => findModuleSourceFile(searchName + extension, moduleNameExpr));
653644
if (referencedSourceFile) {
654-
setResolvedModuleName(file, moduleNameExpr, referencedSourceFile.fileName);
645+
setResolvedModuleName(file, moduleNameExpr.text, referencedSourceFile.fileName);
655646
return;
656647
}
657648

@@ -662,7 +653,7 @@ namespace ts {
662653
searchPath = parentPath;
663654
}
664655
// mark reference as non-resolved
665-
setResolvedModuleName(file, moduleNameExpr, undefined);
656+
setResolvedModuleName(file, moduleNameExpr.text, undefined);
666657
}
667658
}
668659

‎src/compiler/types.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2075,7 +2075,6 @@ namespace ts {
20752075
getCanonicalFileName(fileName: string): string;
20762076
useCaseSensitiveFileNames(): boolean;
20772077
getNewLine(): string;
2078-
hasChanges?(oldFile: SourceFile): boolean;
20792078
}
20802079

20812080
export interface TextSpan {

‎src/compiler/utilities.ts‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ namespace ts {
7878
return node.end - node.pos;
7979
}
8080

81-
export function arrayIsEqualTo<T>(arr1: T[], arr2: T[], comparer: (a: T, b: T) => boolean): boolean {
81+
export function arrayIsEqualTo<T>(arr1: T[], arr2: T[], comparer?: (a: T, b: T) => boolean): boolean {
8282
if (!arr1 || !arr2) {
8383
return arr1 === arr2;
8484
}
@@ -88,28 +88,29 @@ namespace ts {
8888
}
8989

9090
for (let i = 0; i < arr1.length; ++i) {
91-
if (!comparer(arr1[i], arr2[i])) {
91+
let equals = comparer ? comparer(arr1[i], arr2[i]) : arr1[i] === arr2[i];
92+
if (!equals) {
9293
return false;
9394
}
9495
}
9596

9697
return true;
9798
}
9899

99-
export function hasResolvedModuleName(sourceFile: SourceFile, moduleName: LiteralExpression): boolean {
100-
return sourceFile.resolvedModules && hasProperty(sourceFile.resolvedModules, moduleName.text);
100+
export function hasResolvedModuleName(sourceFile: SourceFile, moduleNameText: string): boolean {
101+
return sourceFile.resolvedModules && hasProperty(sourceFile.resolvedModules, moduleNameText);
101102
}
102103

103-
export function getResolvedModuleFileName(sourceFile: SourceFile, moduleName: LiteralExpression): string {
104-
return sourceFile.resolvedModules && sourceFile.resolvedModules[moduleName.text];
104+
export function getResolvedModuleFileName(sourceFile: SourceFile, moduleNameText: string): string {
105+
return hasResolvedModuleName(sourceFile, moduleNameText) ? sourceFile.resolvedModules[moduleNameText]: undefined;
105106
}
106107

107-
export function setResolvedModuleName(sourceFile: SourceFile, moduleName: LiteralExpression, resolvedFileName: string): void {
108+
export function setResolvedModuleName(sourceFile: SourceFile, moduleNameText: string, resolvedFileName: string): void {
108109
if (!sourceFile.resolvedModules) {
109110
sourceFile.resolvedModules = {};
110111
}
111112

112-
sourceFile.resolvedModules[moduleName.text] = resolvedFileName;
113+
sourceFile.resolvedModules[moduleNameText] = resolvedFileName;
113114
}
114115

115116
// Returns true if this node contains a parse error anywhere underneath it.

‎src/services/services.ts‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1898,7 +1898,7 @@ namespace ts {
18981898
let getCanonicalFileName = createGetCanonicalFileName(!!useCaseSensitiveFileNames);
18991899

19001900
function getKeyFromCompilationSettings(settings: CompilerOptions): string {
1901-
return "_" + settings.target; // + "|" + settings.propagateEnumConstantoString()
1901+
return "_" + settings.target + "|" + settings.module + "|" + settings.noResolve;
19021902
}
19031903

19041904
function getBucketForCompilationSettings(settings: CompilerOptions, createIfMissing: boolean): FileMap<DocumentRegistryEntry> {
@@ -2472,8 +2472,6 @@ namespace ts {
24722472
let newSettings = hostCache.compilationSettings();
24732473
let changesInCompilationSettingsAffectSyntax = oldSettings && oldSettings.target !== newSettings.target;
24742474

2475-
let reusableOldProgram = changesInCompilationSettingsAffectSyntax ? undefined : program;
2476-
24772475
// Now create a new compiler
24782476
let newProgram = createProgram(hostCache.getRootFileNames(), newSettings, {
24792477
getSourceFile: getOrCreateSourceFile,
@@ -2484,8 +2482,7 @@ namespace ts {
24842482
getDefaultLibFileName: (options) => host.getDefaultLibFileName(options),
24852483
writeFile: (fileName, data, writeByteOrderMark) => { },
24862484
getCurrentDirectory: () => host.getCurrentDirectory(),
2487-
hasChanges: oldFile => oldFile.version !== hostCache.getVersion(oldFile.fileName)
2488-
}, reusableOldProgram);
2485+
}, program);
24892486

24902487
// Release any files we have acquired in the old program but are
24912488
// not part of the new program.

‎tests/cases/unittests/services/documentRegistry.ts‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,15 @@ describe("DocumentRegistry", () => {
3030

3131
assert(f1 !== f3, "Changed target: Expected to have different instances of document");
3232

33-
compilerOptions.module = ts.ModuleKind.CommonJS;
33+
compilerOptions.preserveConstEnums = true;
3434
var f4 = documentRegistry.acquireDocument("file1.ts", compilerOptions, ts.ScriptSnapshot.fromString("var x = 1;"), /* version */ "1");
3535

36-
assert(f3 === f4, "Changed module: Expected to have the same instance of the document");
36+
assert(f3 === f4, "Changed preserveConstEnums: Expected to have the same instance of the document");
37+
38+
compilerOptions.module = ts.ModuleKind.System;
39+
var f5 = documentRegistry.acquireDocument("file1.ts", compilerOptions, ts.ScriptSnapshot.fromString("var x = 1;"), /* version */ "1");
40+
41+
assert(f4 !== f5, "Changed module: Expected to have different instances of the document");
3742
});
3843

3944
it("Acquiring document gets correct version 1", () => {

0 commit comments

Comments
 (0)