Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Make hasInvalidatedResolution non internal for program and add it watchApi #50776

Merged
merged 4 commits into from Sep 22, 2022
Merged

Make hasInvalidatedResolution non internal for program and add it watchApi #50776

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8a4b96d
Make stub for hasInvalidatedResolution
sheetalkamat Sep 14, 2022
1cf5e81
Wire through hasInvalidatedResolutions
sheetalkamat Sep 14, 2022
c710f58
Update comment
sheetalkamat Sep 22, 2022
a60ba88
Feedback
sheetalkamat Sep 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 4 additions & 8 deletions src/compiler/resolutionCache.ts
View file
Open in desktop
Expand Up @@ -14,7 +14,7 @@ namespace ts {
removeResolutionsOfFile(filePath: Path): void;
removeResolutionsFromProjectReferenceRedirects(filePath: Path): void;
setFilesWithInvalidatedNonRelativeUnresolvedImports(filesWithUnresolvedImports: ESMap<Path, readonly string[]>): void;
createHasInvalidatedResolution(forceAllFilesAsInvalidated?: boolean): HasInvalidatedResolution;
createHasInvalidatedResolution(customHasInvalidatedResolution: HasInvalidatedResolution): HasInvalidatedResolution;
hasChangedAutomaticTypeDirectiveNames(): boolean;
isFileWithInvalidatedNonRelativeUnresolvedImports(path: Path): boolean;

Expand Down Expand Up @@ -300,17 +300,13 @@ namespace ts {
return !!value && !!value.length;
}

function createHasInvalidatedResolution(forceAllFilesAsInvalidated?: boolean): HasInvalidatedResolution {
function createHasInvalidatedResolution(customHasInvalidatedResolution: HasInvalidatedResolution): HasInvalidatedResolution {
// Ensure pending resolutions are applied
invalidateResolutionsOfFailedLookupLocations();
if (forceAllFilesAsInvalidated) {
// Any file asked would have invalidated resolution
filesWithInvalidatedResolutions = undefined;
return returnTrue;
}
const collected = filesWithInvalidatedResolutions;
filesWithInvalidatedResolutions = undefined;
return path => (!!collected && collected.has(path)) ||
return path => customHasInvalidatedResolution(path) ||
!!collected?.has(path) ||
isFileWithInvalidatedNonRelativeUnresolvedImports(path);
}

Expand Down
3 changes: 2 additions & 1 deletion src/compiler/types.ts
View file
Open in desktop
Expand Up @@ -7214,7 +7214,8 @@ namespace ts {
getEnvironmentVariable?(name: string): string | undefined;
/* @internal */ onReleaseOldSourceFile?(oldSourceFile: SourceFile, oldOptions: CompilerOptions, hasSourceFileByPath: boolean): void;
/* @internal */ onReleaseParsedCommandLine?(configFileName: string, oldResolvedRef: ResolvedProjectReference | undefined, optionOptions: CompilerOptions): void;
/* @internal */ hasInvalidatedResolution?: HasInvalidatedResolution;
/** If provided along with custom resolveModuleNames or resolveTypeReferenceDirectives, used to determine if unchanged file path needs to re-resolve modules/type reference directives */
hasInvalidatedResolution?(filePath: Path): boolean;
/* @internal */ hasChangedAutomaticTypeDirectiveNames?: HasChangedAutomaticTypeDirectiveNames;
createHash?(data: string): string;
getParsedCommandLine?(fileName: string): ParsedCommandLine | undefined;
Expand Down
9 changes: 7 additions & 2 deletions src/compiler/watchPublic.ts
View file
Open in desktop
Expand Up @@ -112,6 +112,8 @@ namespace ts {
resolveModuleNames?(moduleNames: string[], containingFile: string, reusedNames: string[] | undefined, redirectedReference: ResolvedProjectReference | undefined, options: CompilerOptions, containingSourceFile?: SourceFile): (ResolvedModule | undefined)[];
/** If provided, used to resolve type reference directives, otherwise typescript's default resolution */
resolveTypeReferenceDirectives?(typeReferenceDirectiveNames: string[] | readonly FileReference[], containingFile: string, redirectedReference: ResolvedProjectReference | undefined, options: CompilerOptions, containingFileMode?: SourceFile["impliedNodeFormat"] | undefined): (ResolvedTypeReferenceDirective | undefined)[];
/** If provided along with custom resolveModuleNames or resolveTypeReferenceDirectives, used to determine if unchanged file path needs to re-resolve modules/type reference directives */
hasInvalidatedResolution?(filePath: Path): boolean;
/**
* Returns the module resolution cache used by a provided `resolveModuleNames` implementation so that any non-name module resolution operations (eg, package.json lookup) can reuse it
*/
Expand Down Expand Up @@ -371,6 +373,10 @@ namespace ts {
maybeBind(host, host.getModuleResolutionCache) :
(() => resolutionCache.getModuleResolutionCache());
const userProvidedResolution = !!host.resolveModuleNames || !!host.resolveTypeReferenceDirectives;
// All resolutions are invalid if user provided resolutions and didnt supply hasInvalidatedResolution
const customHasInvalidatedResolution = userProvidedResolution ?
maybeBind(host, host.hasInvalidatedResolution) || returnTrue :
returnFalse;

builderProgram = readBuilderProgram(compilerOptions, compilerHost) as any as T;
synchronizeProgram();
Expand Down Expand Up @@ -443,8 +449,7 @@ namespace ts {
}
}

// All resolutions are invalid if user provided resolutions
const hasInvalidatedResolution = resolutionCache.createHasInvalidatedResolution(userProvidedResolution);
const hasInvalidatedResolution = resolutionCache.createHasInvalidatedResolution(customHasInvalidatedResolution);
const {
originalReadFile, originalFileExists, originalDirectoryExists,
originalCreateDirectory, originalWriteFile,
Expand Down
2 changes: 1 addition & 1 deletion src/server/project.ts
View file
Open in desktop
Expand Up @@ -1176,7 +1176,7 @@ namespace ts.server {
Debug.assert(!this.isClosed(), "Called update graph worker of closed project");
this.writeLog(`Starting updateGraphWorker: Project: ${this.getProjectName()}`);
const start = timestamp();
this.hasInvalidatedResolution = this.resolutionCache.createHasInvalidatedResolution();
this.hasInvalidatedResolution = this.resolutionCache.createHasInvalidatedResolution(returnFalse);
this.resolutionCache.startCachingPerDirectoryResolution();
this.program = this.languageService.getProgram(); // TODO: GH#18217
this.dirty = false;
Expand Down
91 changes: 74 additions & 17 deletions src/testRunner/unittests/tscWatch/watchApi.ts
View file
Open in desktop
@@ -1,23 +1,22 @@
namespace ts.tscWatch {
describe("unittests:: tsc-watch:: watchAPI:: tsc-watch with custom module resolution", () => {
const configFileJson: any = {
compilerOptions: { module: "commonjs", resolveJsonModule: true },
files: ["index.ts"]
};
const mainFile: File = {
path: `${projectRoot}/index.ts`,
content: "import settings from './settings.json';"
};
const config: File = {
path: `${projectRoot}/tsconfig.json`,
content: JSON.stringify(configFileJson)
};
const settingsJson: File = {
path: `${projectRoot}/settings.json`,
content: JSON.stringify({ content: "Print this" })
};

it("verify that module resolution with json extension works when returned without extension", () => {
const configFileJson: any = {
compilerOptions: { module: "commonjs", resolveJsonModule: true },
files: ["index.ts"]
};
const mainFile: File = {
path: `${projectRoot}/index.ts`,
content: "import settings from './settings.json';"
};
const config: File = {
path: `${projectRoot}/tsconfig.json`,
content: JSON.stringify(configFileJson)
};
const settingsJson: File = {
path: `${projectRoot}/settings.json`,
content: JSON.stringify({ content: "Print this" })
};
const { sys, baseline, oldSnap, cb, getPrograms } = createBaseline(createWatchedSystem(
[libFile, mainFile, config, settingsJson],
{ currentDirectory: projectRoot }),
Expand Down Expand Up @@ -50,6 +49,64 @@ namespace ts.tscWatch {
watchOrSolution: watch
});
});

describe("hasInvalidatedResolution", () => {
function verifyWatch(subScenario: string, implementHasInvalidatedResolution: boolean) {
it(subScenario, () => {
const { sys, baseline, oldSnap, cb, getPrograms } = createBaseline(createWatchedSystem({
[`${projectRoot}/tsconfig.json`]: JSON.stringify({
compilerOptions: { traceResolution: true, extendedDiagnostics: true },
files: ["main.ts"]
}),
[`${projectRoot}/main.ts`]: `import { foo } from "./other";`,
[`${projectRoot}/other.d.ts`]: "export function foo(): void;",
[libFile.path]: libFile.content,
}, { currentDirectory: projectRoot }));
const host = createWatchCompilerHostOfConfigFileForBaseline({
configFileName: `${projectRoot}/tsconfig.json`,
system: sys,
cb,
});
host.resolveModuleNames = (moduleNames, containingFile, _reusedNames, _redirectedReference, options) =>
moduleNames.map(m => resolveModuleName(m, containingFile, options, host).resolvedModule);
// Invalidate resolutions only when ts file is created
if (implementHasInvalidatedResolution) host.hasInvalidatedResolution = () => sys.fileExists(`${projectRoot}/other.ts`);
const watch = createWatchProgram(host);
runWatchBaseline({
scenario: "watchApi",
subScenario,
commandLineArgs: ["--w"],
sys,
baseline,
oldSnap,
getPrograms,
changes: [
{
caption: "write other with same contents",
change: sys => sys.appendFile(`${projectRoot}/other.d.ts`, ""),
timeouts: sys => sys.runQueuedTimeoutCallbacks(),
},
{
caption: "change other file",
change: sys => sys.appendFile(`${projectRoot}/other.d.ts`, "export function bar(): void;"),
timeouts: sys => sys.runQueuedTimeoutCallbacks(),
},
{
caption: "write other with same contents but write ts file",
change: sys => {
sys.appendFile(`${projectRoot}/other.d.ts`, "");
sys.writeFile(`${projectRoot}/other.ts`, "export function foo() {}");
},
timeouts: sys => sys.runQueuedTimeoutCallbacks(),
},
],
watchOrSolution: watch
});
});
}
verifyWatch("host implements does not implement hasInvalidatedResolution", /*implementHasInvalidatedResolution*/ false);
verifyWatch("host implements hasInvalidatedResolution", /*implementHasInvalidatedResolution*/ true);
});
});

describe("unittests:: tsc-watch:: watchAPI:: tsc-watch expose error count to watch status reporter", () => {
Expand Down
4 changes: 4 additions & 0 deletions tests/baselines/reference/api/tsserverlibrary.d.ts
View file
Open in desktop
Expand Up @@ -3320,6 +3320,8 @@ declare namespace ts {
*/
resolveTypeReferenceDirectives?(typeReferenceDirectiveNames: string[] | readonly FileReference[], containingFile: string, redirectedReference: ResolvedProjectReference | undefined, options: CompilerOptions, containingFileMode?: SourceFile["impliedNodeFormat"] | undefined): (ResolvedTypeReferenceDirective | undefined)[];
getEnvironmentVariable?(name: string): string | undefined;
/** If provided along with custom resolveModuleNames or resolveTypeReferenceDirectives, used to determine if unchanged file path needs to re-resolve modules/type reference directives */
Copy link
Member

Choose a reason for hiding this comment

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

Does that mean anything that referenced this thing needs to re-resolve, or that everything that this path references needs to be re-resolved?

Copy link
Member Author

Choose a reason for hiding this comment

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

When returned true for party, the module names in that file will be re- resolved

Copy link
Member

Choose a reason for hiding this comment

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

Should it have been hasInvalidatedResolutions?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure .. I can only submit Pr for that tomorrow though

Copy link
Member

Choose a reason for hiding this comment

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

No worries, I got you covered #50912

hasInvalidatedResolution?(filePath: Path): boolean;
sheetalkamat marked this conversation as resolved.
Show resolved Hide resolved
createHash?(data: string): string;
getParsedCommandLine?(fileName: string): ParsedCommandLine | undefined;
}
Expand Down Expand Up @@ -5442,6 +5444,8 @@ declare namespace ts {
resolveModuleNames?(moduleNames: string[], containingFile: string, reusedNames: string[] | undefined, redirectedReference: ResolvedProjectReference | undefined, options: CompilerOptions, containingSourceFile?: SourceFile): (ResolvedModule | undefined)[];
/** If provided, used to resolve type reference directives, otherwise typescript's default resolution */
resolveTypeReferenceDirectives?(typeReferenceDirectiveNames: string[] | readonly FileReference[], containingFile: string, redirectedReference: ResolvedProjectReference | undefined, options: CompilerOptions, containingFileMode?: SourceFile["impliedNodeFormat"] | undefined): (ResolvedTypeReferenceDirective | undefined)[];
/** If provided along with custom resolveModuleNames or resolveTypeReferenceDirectives, used to determine if unchanged file path needs to re-resolve modules/type reference directives */
hasInvalidatedResolution?(filePath: Path): boolean;
/**
* Returns the module resolution cache used by a provided `resolveModuleNames` implementation so that any non-name module resolution operations (eg, package.json lookup) can reuse it
*/
Expand Down
4 changes: 4 additions & 0 deletions tests/baselines/reference/api/typescript.d.ts
View file
Open in desktop
Expand Up @@ -3320,6 +3320,8 @@ declare namespace ts {
*/
resolveTypeReferenceDirectives?(typeReferenceDirectiveNames: string[] | readonly FileReference[], containingFile: string, redirectedReference: ResolvedProjectReference | undefined, options: CompilerOptions, containingFileMode?: SourceFile["impliedNodeFormat"] | undefined): (ResolvedTypeReferenceDirective | undefined)[];
getEnvironmentVariable?(name: string): string | undefined;
/** If provided along with custom resolveModuleNames or resolveTypeReferenceDirectives, used to determine if unchanged file path needs to re-resolve modules/type reference directives */
hasInvalidatedResolution?(filePath: Path): boolean;
createHash?(data: string): string;
getParsedCommandLine?(fileName: string): ParsedCommandLine | undefined;
}
Expand Down Expand Up @@ -5442,6 +5444,8 @@ declare namespace ts {
resolveModuleNames?(moduleNames: string[], containingFile: string, reusedNames: string[] | undefined, redirectedReference: ResolvedProjectReference | undefined, options: CompilerOptions, containingSourceFile?: SourceFile): (ResolvedModule | undefined)[];
/** If provided, used to resolve type reference directives, otherwise typescript's default resolution */
resolveTypeReferenceDirectives?(typeReferenceDirectiveNames: string[] | readonly FileReference[], containingFile: string, redirectedReference: ResolvedProjectReference | undefined, options: CompilerOptions, containingFileMode?: SourceFile["impliedNodeFormat"] | undefined): (ResolvedTypeReferenceDirective | undefined)[];
/** If provided along with custom resolveModuleNames or resolveTypeReferenceDirectives, used to determine if unchanged file path needs to re-resolve modules/type reference directives */
hasInvalidatedResolution?(filePath: Path): boolean;
/**
* Returns the module resolution cache used by a provided `resolveModuleNames` implementation so that any non-name module resolution operations (eg, package.json lookup) can reuse it
*/
Expand Down