-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
Improvements to tsc --watch #17669
Improvements to tsc --watch #17669
Changes from 23 commits
802e283
499fabc
273569f
94a589b
ef5935b
e068475
6237b22
9b18f7b
85b9254
69e5abd
c814d8e
2dd6aed
89c61e7
bb91b32
46e3d1c
031a637
0d5e6c9
2762232
65a6ee0
d55150c
8dc6248
7474ba7
6385f7e
d0a23bb
59d07dc
9895082
f1b1b12
136b091
6bf9133
a99c04e
b66b752
e639ceb
8deef58
60e2e68
3908325
7173da2
e500be2
6227a36
55931c4
e65df12
e711238
3b85f3f
ea95f3b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -473,12 +473,14 @@ namespace ts { | |
return result; | ||
} | ||
|
||
export function flatMapIter<T, U>(iter: Iterator<T>, mapfn: (x: T) => U | U[] | undefined): U[] { | ||
const result: U[] = []; | ||
export function flatMapIter<T>(iter: Iterator<T>): T[]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No need to modify this function, |
||
export function flatMapIter<T, U>(iter: Iterator<T>, mapfn: (x: T) => U | U[] | undefined): U[]; | ||
export function flatMapIter<T>(iter: Iterator<T>, mapfn?: (x: any) => any): any[] { | ||
const result = []; | ||
while (true) { | ||
const { value, done } = iter.next(); | ||
if (done) break; | ||
const res = mapfn(value); | ||
const res = mapfn ? mapfn(value) : value; | ||
if (res) { | ||
if (isArray(res)) { | ||
result.push(...res); | ||
|
@@ -1201,6 +1203,13 @@ namespace ts { | |
return Array.isArray ? Array.isArray(value) : value instanceof Array; | ||
} | ||
|
||
/** | ||
* Tests whether a value is string | ||
*/ | ||
export function isString(text: any): text is string { | ||
return typeof text === "string"; | ||
} | ||
|
||
export function tryCast<TOut extends TIn, TIn = any>(value: TIn | undefined, test: (value: TIn) => value is TOut): TOut | undefined { | ||
return value !== undefined && test(value) ? value : undefined; | ||
} | ||
|
@@ -1211,7 +1220,7 @@ namespace ts { | |
} | ||
|
||
/** Does nothing. */ | ||
export function noop(): void {} | ||
export function noop(): any {} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid using |
||
|
||
/** Throws an error because a function is not implemented. */ | ||
export function notImplemented(): never { | ||
|
@@ -1454,16 +1463,16 @@ namespace ts { | |
function compareMessageText(text1: string | DiagnosticMessageChain, text2: string | DiagnosticMessageChain): Comparison { | ||
while (text1 && text2) { | ||
// We still have both chains. | ||
const string1 = typeof text1 === "string" ? text1 : text1.messageText; | ||
const string2 = typeof text2 === "string" ? text2 : text2.messageText; | ||
const string1 = isString(text1) ? text1 : text1.messageText; | ||
const string2 = isString(text2) ? text2 : text2.messageText; | ||
|
||
const res = compareValues(string1, string2); | ||
if (res) { | ||
return res; | ||
} | ||
|
||
text1 = typeof text1 === "string" ? undefined : text1.next; | ||
text2 = typeof text2 === "string" ? undefined : text2.next; | ||
text1 = isString(text1) ? undefined : text1.next; | ||
text2 = isString(text2) ? undefined : text2.next; | ||
} | ||
|
||
if (!text1 && !text2) { | ||
|
@@ -2552,4 +2561,168 @@ namespace ts { | |
export function isCheckJsEnabledForFile(sourceFile: SourceFile, compilerOptions: CompilerOptions) { | ||
return sourceFile.checkJsDirective ? sourceFile.checkJsDirective.enabled : compilerOptions.checkJs; | ||
} | ||
|
||
export interface HostForCaching { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could this inherit from something? These signatures look familar. |
||
useCaseSensitiveFileNames: boolean; | ||
writeFile(path: string, data: string, writeByteOrderMark?: boolean): void; | ||
fileExists(path: string): boolean; | ||
directoryExists(path: string): boolean; | ||
createDirectory(path: string): void; | ||
getCurrentDirectory(): string; | ||
getDirectories(path: string): string[]; | ||
readDirectory(path: string, extensions?: ReadonlyArray<string>, exclude?: ReadonlyArray<string>, include?: ReadonlyArray<string>, depth?: number): string[]; | ||
} | ||
|
||
export interface CachedHost { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same comment as above. |
||
writeFile(path: string, data: string, writeByteOrderMark?: boolean): void; | ||
fileExists(path: string): boolean; | ||
directoryExists(path: string): boolean; | ||
createDirectory(path: string): void; | ||
getCurrentDirectory(): string; | ||
getDirectories(path: string): string[]; | ||
readDirectory(path: string, extensions?: ReadonlyArray<string>, exclude?: ReadonlyArray<string>, include?: ReadonlyArray<string>, depth?: number): string[]; | ||
addOrDeleteFileOrFolder(fileOrFolder: string): void; | ||
clearCache(): void; | ||
} | ||
|
||
export function createCachedHost(host: HostForCaching): CachedHost { | ||
const cachedReadDirectoryResult = createMap<FileSystemEntries>(); | ||
const getCurrentDirectory = memoize(() => host.getCurrentDirectory()); | ||
const getCanonicalFileName = createGetCanonicalFileName(host.useCaseSensitiveFileNames); | ||
return { | ||
writeFile, | ||
fileExists, | ||
directoryExists, | ||
createDirectory, | ||
getCurrentDirectory, | ||
getDirectories, | ||
readDirectory, | ||
addOrDeleteFileOrFolder, | ||
clearCache | ||
}; | ||
|
||
function toPath(fileName: string) { | ||
return ts.toPath(fileName, getCurrentDirectory(), getCanonicalFileName); | ||
} | ||
|
||
function getFileSystemEntries(rootDir: string) { | ||
const path = toPath(rootDir); | ||
const cachedResult = cachedReadDirectoryResult.get(path); | ||
if (cachedResult) { | ||
return cachedResult; | ||
} | ||
|
||
const resultFromHost: FileSystemEntries = { | ||
files: host.readDirectory(rootDir, /*extensions*/ undefined, /*exclude*/ undefined, /*include*/["*.*"]) || [], | ||
directories: host.getDirectories(rootDir) || [] | ||
}; | ||
|
||
cachedReadDirectoryResult.set(path, resultFromHost); | ||
return resultFromHost; | ||
} | ||
|
||
function canWorkWithCacheForDir(rootDir: string) { | ||
// Some of the hosts might not be able to handle read directory or getDirectories | ||
const path = toPath(rootDir); | ||
if (cachedReadDirectoryResult.get(path)) { | ||
return true; | ||
} | ||
try { | ||
return getFileSystemEntries(rootDir); | ||
} | ||
catch (_e) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Include an assertion about the kind of exception you expected to catch. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It could be any exception, i have updated the comment There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wait, why do we want to catch any exception and ignore it? |
||
return false; | ||
} | ||
} | ||
|
||
function fileNameEqual(name1: string, name2: string) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it would be better to use canonical file names from the start, rather than to re-canonicalize them every time they are compared. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I dont agree with that. This allows us to retain the casing (instead of always providing using lower case) of file names even if the host is case-insensitive |
||
return getCanonicalFileName(name1) === getCanonicalFileName(name2); | ||
} | ||
|
||
function hasEntry(entries: ReadonlyArray<string>, name: string) { | ||
return some(entries, file => fileNameEqual(file, name)); | ||
} | ||
|
||
function updateFileSystemEntry(entries: ReadonlyArray<string>, baseName: string, isValid: boolean) { | ||
if (hasEntry(entries, baseName)) { | ||
if (!isValid) { | ||
return filter(entries, entry => !fileNameEqual(entry, baseName)); | ||
} | ||
} | ||
else if (isValid) { | ||
return entries.concat(baseName); | ||
} | ||
return entries; | ||
} | ||
|
||
function writeFile(fileName: string, data: string, writeByteOrderMark?: boolean): void { | ||
const path = toPath(fileName); | ||
const result = cachedReadDirectoryResult.get(getDirectoryPath(path)); | ||
const baseFileName = getBaseFileName(normalizePath(fileName)); | ||
if (result) { | ||
result.files = updateFileSystemEntry(result.files, baseFileName, /*isValid*/ true); | ||
} | ||
return host.writeFile(fileName, data, writeByteOrderMark); | ||
} | ||
|
||
function fileExists(fileName: string): boolean { | ||
const path = toPath(fileName); | ||
const result = cachedReadDirectoryResult.get(getDirectoryPath(path)); | ||
const baseName = getBaseFileName(normalizePath(fileName)); | ||
return (result && hasEntry(result.files, baseName)) || host.fileExists(fileName); | ||
} | ||
|
||
function directoryExists(dirPath: string): boolean { | ||
const path = toPath(dirPath); | ||
return cachedReadDirectoryResult.has(path) || host.directoryExists(dirPath); | ||
} | ||
|
||
function createDirectory(dirPath: string) { | ||
const path = toPath(dirPath); | ||
const result = cachedReadDirectoryResult.get(getDirectoryPath(path)); | ||
const baseFileName = getBaseFileName(path); | ||
if (result) { | ||
result.directories = updateFileSystemEntry(result.directories, baseFileName, /*isValid*/ true); | ||
} | ||
host.createDirectory(dirPath); | ||
} | ||
|
||
function getDirectories(rootDir: string): string[] { | ||
if (canWorkWithCacheForDir(rootDir)) { | ||
return getFileSystemEntries(rootDir).directories.slice(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
} | ||
return host.getDirectories(rootDir); | ||
} | ||
function readDirectory(rootDir: string, extensions?: ReadonlyArray<string>, excludes?: ReadonlyArray<string>, includes?: ReadonlyArray<string>, depth?: number): string[] { | ||
if (canWorkWithCacheForDir(rootDir)) { | ||
return matchFiles(rootDir, extensions, excludes, includes, host.useCaseSensitiveFileNames, getCurrentDirectory(), depth, path => getFileSystemEntries(path)); | ||
} | ||
return host.readDirectory(rootDir, extensions, excludes, includes, depth); | ||
} | ||
|
||
function addOrDeleteFileOrFolder(fileOrFolder: string) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's kind of strange to have a function that has to infer what kind of action it should take. Can't the caller be responsible for calling an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No because caller to this function is normally from the watch and directory watches dont specify what the action was. For now this is best one can do to infer There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added addOrDeleteFile that can be called from file watcher because there we know the status of the file |
||
const path = toPath(fileOrFolder); | ||
const existingResult = cachedReadDirectoryResult.get(path); | ||
if (existingResult) { | ||
if (!host.directoryExists(fileOrFolder)) { | ||
cachedReadDirectoryResult.delete(path); | ||
} | ||
} | ||
else { | ||
// Was this earlier file | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you clarify this comment? |
||
const parentResult = cachedReadDirectoryResult.get(getDirectoryPath(path)); | ||
if (parentResult) { | ||
const baseName = getBaseFileName(fileOrFolder); | ||
if (parentResult) { | ||
parentResult.files = updateFileSystemEntry(parentResult.files, baseName, host.fileExists(path)); | ||
parentResult.directories = updateFileSystemEntry(parentResult.directories, baseName, host.directoryExists(path)); | ||
} | ||
} | ||
} | ||
} | ||
|
||
function clearCache() { | ||
cachedReadDirectoryResult.clear(); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like this last argument is always provided?