From 3e37b3158b538a370680bc551579e90fd54bca82 Mon Sep 17 00:00:00 2001 From: zhengbli Date: Wed, 14 Oct 2015 21:36:35 -0700 Subject: [PATCH 1/4] Address code review at 5127 --- src/compiler/core.ts | 6 +++--- src/compiler/sys.ts | 2 +- src/compiler/tsc.ts | 2 +- src/compiler/utilities.ts | 19 ++++++------------- src/server/editorServices.ts | 2 +- 5 files changed, 12 insertions(+), 19 deletions(-) diff --git a/src/compiler/core.ts b/src/compiler/core.ts index 9a4f5452eed..55d2447dd2d 100644 --- a/src/compiler/core.ts +++ b/src/compiler/core.ts @@ -844,9 +844,9 @@ namespace ts { export function copyListRemovingItem(item: T, list: T[]) { let copiedList: T[] = []; - for (var i = 0, len = list.length; i < len; i++) { - if (list[i] !== item) { - copiedList.push(list[i]); + for (let e of list) { + if (e !== item) { + copiedList.push(e); } } return copiedList; diff --git a/src/compiler/sys.ts b/src/compiler/sys.ts index 0872a2e5ba5..3eeb126c065 100644 --- a/src/compiler/sys.ts +++ b/src/compiler/sys.ts @@ -407,7 +407,7 @@ namespace ts { // (ref: https://github.com/nodejs/node/pull/2649 and https://github.com/Microsoft/TypeScript/issues/4643) return _fs.watch( path, - { persisten: true, recursive: !!recursive }, + { persistent: true, recursive: !!recursive }, (eventName: string, relativeFileName: string) => { // In watchDirectory we only care about adding and removing files (when event name is // "rename"); changes made within files are handled by corresponding fileWatchers (when diff --git a/src/compiler/tsc.ts b/src/compiler/tsc.ts index 9d79d435a04..5129f5e9efe 100644 --- a/src/compiler/tsc.ts +++ b/src/compiler/tsc.ts @@ -360,7 +360,7 @@ namespace ts { let newFileNames = ts.map(parsedCommandLine.fileNames, compilerHost.getCanonicalFileName); let canonicalRootFileNames = ts.map(rootFileNames, compilerHost.getCanonicalFileName); - if (!arrayStructurallyIsEqualTo(newFileNames, canonicalRootFileNames)) { + if (!arrayIsEqualTo(newFileNames, canonicalRootFileNames, /*comparer*/ undefined, /*sortBeforeComparison*/ true)) { setCachedProgram(undefined); startTimerForRecompilation(); } diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index ccfab49375d..37109d1aa73 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -82,7 +82,7 @@ namespace ts { return node.end - node.pos; } - export function arrayIsEqualTo(arr1: T[], arr2: T[], comparer?: (a: T, b: T) => boolean): boolean { + export function arrayIsEqualTo(arr1: T[], arr2: T[], comparer?: (a: T, b: T) => boolean, sortBeforeComparison = false): boolean { if (!arr1 || !arr2) { return arr1 === arr2; } @@ -91,6 +91,11 @@ namespace ts { return false; } + if (sortBeforeComparison) { + arr1.sort(); + arr2.sort(); + } + for (let i = 0; i < arr1.length; ++i) { let equals = comparer ? comparer(arr1[i], arr2[i]) : arr1[i] === arr2[i]; if (!equals) { @@ -2414,16 +2419,4 @@ namespace ts { } } } - - export function arrayStructurallyIsEqualTo(array1: Array, array2: Array): boolean { - if (!array1 || !array2) { - return false; - } - - if (array1.length !== array2.length) { - return false; - } - - return arrayIsEqualTo(array1.sort(), array2.sort()); - } } diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index 09f8a372463..bd334279798 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -576,7 +576,7 @@ namespace ts.server { let newRootFiles = projectOptions.files.map((f => this.getCanonicalFileName(f))); let currentRootFiles = project.getRootFiles().map((f => this.getCanonicalFileName(f))); - if (!arrayStructurallyIsEqualTo(currentRootFiles, newRootFiles)) { + if (!arrayIsEqualTo(currentRootFiles, newRootFiles, /*comparer*/ undefined, /*sortBeforeComparison*/ true)) { // For configured projects, the change is made outside the tsconfig file, and // it is not likely to affect the project for other files opened by the client. We can // just update the current project. From ea9bf7313ae7334cdbed9a32306bccb4a22e3f5a Mon Sep 17 00:00:00 2001 From: zhengbli Date: Thu, 15 Oct 2015 13:53:37 -0700 Subject: [PATCH 2/4] CR feedback --- src/compiler/tsc.ts | 2 +- src/compiler/utilities.ts | 7 +------ src/server/editorServices.ts | 2 +- 3 files changed, 3 insertions(+), 8 deletions(-) diff --git a/src/compiler/tsc.ts b/src/compiler/tsc.ts index 5129f5e9efe..5420caabe7c 100644 --- a/src/compiler/tsc.ts +++ b/src/compiler/tsc.ts @@ -360,7 +360,7 @@ namespace ts { let newFileNames = ts.map(parsedCommandLine.fileNames, compilerHost.getCanonicalFileName); let canonicalRootFileNames = ts.map(rootFileNames, compilerHost.getCanonicalFileName); - if (!arrayIsEqualTo(newFileNames, canonicalRootFileNames, /*comparer*/ undefined, /*sortBeforeComparison*/ true)) { + if (!arrayIsEqualTo(newFileNames.sort(), canonicalRootFileNames.sort())) { setCachedProgram(undefined); startTimerForRecompilation(); } diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 37109d1aa73..2a16c2c71fb 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -82,7 +82,7 @@ namespace ts { return node.end - node.pos; } - export function arrayIsEqualTo(arr1: T[], arr2: T[], comparer?: (a: T, b: T) => boolean, sortBeforeComparison = false): boolean { + export function arrayIsEqualTo(arr1: T[], arr2: T[], comparer?: (a: T, b: T) => boolean): boolean { if (!arr1 || !arr2) { return arr1 === arr2; } @@ -91,11 +91,6 @@ namespace ts { return false; } - if (sortBeforeComparison) { - arr1.sort(); - arr2.sort(); - } - for (let i = 0; i < arr1.length; ++i) { let equals = comparer ? comparer(arr1[i], arr2[i]) : arr1[i] === arr2[i]; if (!equals) { diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index bd334279798..b98129f99e9 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -576,7 +576,7 @@ namespace ts.server { let newRootFiles = projectOptions.files.map((f => this.getCanonicalFileName(f))); let currentRootFiles = project.getRootFiles().map((f => this.getCanonicalFileName(f))); - if (!arrayIsEqualTo(currentRootFiles, newRootFiles, /*comparer*/ undefined, /*sortBeforeComparison*/ true)) { + if (!arrayIsEqualTo(currentRootFiles.sort(), newRootFiles.sort())) { // For configured projects, the change is made outside the tsconfig file, and // it is not likely to affect the project for other files opened by the client. We can // just update the current project. From e7e1fa72ec198f63d794abb296c4befb8b823dbb Mon Sep 17 00:00:00 2001 From: zhengbli Date: Fri, 16 Oct 2015 12:00:31 -0700 Subject: [PATCH 3/4] Add sortBeforeComparison option back to arrayIsEqualTo --- src/compiler/tsc.ts | 3 ++- src/compiler/utilities.ts | 16 ++++++++++------ src/server/editorServices.ts | 3 ++- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/compiler/tsc.ts b/src/compiler/tsc.ts index 5420caabe7c..b4774b9a842 100644 --- a/src/compiler/tsc.ts +++ b/src/compiler/tsc.ts @@ -360,7 +360,8 @@ namespace ts { let newFileNames = ts.map(parsedCommandLine.fileNames, compilerHost.getCanonicalFileName); let canonicalRootFileNames = ts.map(rootFileNames, compilerHost.getCanonicalFileName); - if (!arrayIsEqualTo(newFileNames.sort(), canonicalRootFileNames.sort())) { + // We check if the project file list has changed. If so, we just throw away the old program and start fresh. + if (!arrayIsEqualTo(newFileNames, canonicalRootFileNames, /*equaler*/ undefined, /*sortBeforeComparison*/ true)) { setCachedProgram(undefined); startTimerForRecompilation(); } diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 2a16c2c71fb..b56c63013aa 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -82,17 +82,21 @@ namespace ts { return node.end - node.pos; } - export function arrayIsEqualTo(arr1: T[], arr2: T[], comparer?: (a: T, b: T) => boolean): boolean { - if (!arr1 || !arr2) { - return arr1 === arr2; + export function arrayIsEqualTo(array1: T[], array2: T[], equaler?: (a: T, b: T) => boolean, + sortBeforeComparison?: boolean, comparer?: (a: T, b: T) => number): boolean { + if (!array1 || !array2) { + return array1 === array2; } - if (arr1.length !== arr2.length) { + if (array1.length !== array2.length) { return false; } - for (let i = 0; i < arr1.length; ++i) { - let equals = comparer ? comparer(arr1[i], arr2[i]) : arr1[i] === arr2[i]; + let newArray1 = sortBeforeComparison ? array1.slice().sort(comparer) : array1; + let newArray2 = sortBeforeComparison ? array2.slice().sort(comparer) : array2; + + for (let i = 0; i < array1.length; ++i) { + let equals = equaler ? equaler(newArray1[i], newArray2[i]) : newArray1[i] === newArray2[i]; if (!equals) { return false; } diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index b98129f99e9..7cb047bb74d 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -576,7 +576,8 @@ namespace ts.server { let newRootFiles = projectOptions.files.map((f => this.getCanonicalFileName(f))); let currentRootFiles = project.getRootFiles().map((f => this.getCanonicalFileName(f))); - if (!arrayIsEqualTo(currentRootFiles.sort(), newRootFiles.sort())) { + // We check if the project file list has changed. If so, we update the project. + if (!arrayIsEqualTo(currentRootFiles, newRootFiles, /*equaler*/ undefined, /*sortBeforeComparison*/ true)) { // For configured projects, the change is made outside the tsconfig file, and // it is not likely to affect the project for other files opened by the client. We can // just update the current project. From 39254b54ae3b5b51517aa7a2217c5c852e8ceab2 Mon Sep 17 00:00:00 2001 From: zhengbli Date: Mon, 19 Oct 2015 21:48:40 -0700 Subject: [PATCH 4/4] CR feedback --- src/compiler/tsc.ts | 2 +- src/compiler/utilities.ts | 8 ++------ src/server/editorServices.ts | 2 +- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/compiler/tsc.ts b/src/compiler/tsc.ts index b4774b9a842..e51fda074c5 100644 --- a/src/compiler/tsc.ts +++ b/src/compiler/tsc.ts @@ -361,7 +361,7 @@ namespace ts { let canonicalRootFileNames = ts.map(rootFileNames, compilerHost.getCanonicalFileName); // We check if the project file list has changed. If so, we just throw away the old program and start fresh. - if (!arrayIsEqualTo(newFileNames, canonicalRootFileNames, /*equaler*/ undefined, /*sortBeforeComparison*/ true)) { + if (!arrayIsEqualTo(newFileNames && newFileNames.sort(), canonicalRootFileNames && canonicalRootFileNames.sort())) { setCachedProgram(undefined); startTimerForRecompilation(); } diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index b56c63013aa..7c1b606dd00 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -82,8 +82,7 @@ namespace ts { return node.end - node.pos; } - export function arrayIsEqualTo(array1: T[], array2: T[], equaler?: (a: T, b: T) => boolean, - sortBeforeComparison?: boolean, comparer?: (a: T, b: T) => number): boolean { + export function arrayIsEqualTo(array1: T[], array2: T[], equaler?: (a: T, b: T) => boolean): boolean { if (!array1 || !array2) { return array1 === array2; } @@ -92,11 +91,8 @@ namespace ts { return false; } - let newArray1 = sortBeforeComparison ? array1.slice().sort(comparer) : array1; - let newArray2 = sortBeforeComparison ? array2.slice().sort(comparer) : array2; - for (let i = 0; i < array1.length; ++i) { - let equals = equaler ? equaler(newArray1[i], newArray2[i]) : newArray1[i] === newArray2[i]; + let equals = equaler ? equaler(array1[i], array2[i]) : array1[i] === array2[i]; if (!equals) { return false; } diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index 7cb047bb74d..efcca0e37c5 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -577,7 +577,7 @@ namespace ts.server { let currentRootFiles = project.getRootFiles().map((f => this.getCanonicalFileName(f))); // We check if the project file list has changed. If so, we update the project. - if (!arrayIsEqualTo(currentRootFiles, newRootFiles, /*equaler*/ undefined, /*sortBeforeComparison*/ true)) { + if (!arrayIsEqualTo(currentRootFiles && currentRootFiles.sort(), newRootFiles && newRootFiles.sort())) { // For configured projects, the change is made outside the tsconfig file, and // it is not likely to affect the project for other files opened by the client. We can // just update the current project.