Skip to content

Add editor configurable filename-based ATA #40952

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

Merged
merged 11 commits into from
Oct 19, 2020
Merged
6 changes: 5 additions & 1 deletion src/compiler/commandLineParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1133,7 +1133,11 @@ namespace ts {
name: "exclude",
type: "string"
}
}
},
{
name: "inferTypingsFromFilenames",
type: "boolean",
},
];

/* @internal */
Expand Down
3 changes: 2 additions & 1 deletion src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5844,7 +5844,8 @@ namespace ts {
enable?: boolean;
include?: string[];
exclude?: string[];
[option: string]: string[] | boolean | undefined;
inferTypingsFromFilenames?: boolean;
Copy link
Member

Choose a reason for hiding this comment

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

I think you want apposite of this option so that current type acquisitions in config file behave as if this option is set to false. Ideally you want new behavior to kick in only when the option is set

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I accounted for that, making so that the behavior is only disabled if explicitly set to false. I was imagining something like Enable/disable filename-based ATA as the text of the setting on the editor side, with it checked by default.

I suppose disableFilenameBasedTypeAcquisition works just as well.

[option: string]: CompilerOptionsValue | undefined;
}

export enum ModuleKind {
Expand Down
6 changes: 4 additions & 2 deletions src/jsTyping/jsTyping.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,10 @@ namespace ts.JsTyping {
const nodeModulesPath = combinePaths(searchDir, "node_modules");
getTypingNamesFromPackagesFolder(nodeModulesPath, filesToWatch);
});
getTypingNamesFromSourceFileNames(fileNames);

// filename-based ATA must be explicitly disabled
if(typeAcquisition.inferTypingsFromFilenames !== false) {
getTypingNamesFromSourceFileNames(fileNames);
}
// add typings for unresolved imports
if (unresolvedImports) {
const module = deduplicate<string>(
Expand Down
25 changes: 23 additions & 2 deletions src/server/editorServices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,16 @@ namespace ts.server {
return result;
}

export function convertTypeAcquisition(protocolOptions: protocol.ExternalProjectCompilerOptions): TypeAcquisition | undefined {
let result: TypeAcquisition | undefined;
typeAcquisitionDeclarations.forEach((option) => {
const propertyValue = protocolOptions[option.name];
if (propertyValue === undefined) return;
(result || (result = {}))[option.name] = propertyValue;
});
return result;
}

export function tryConvertScriptKindName(scriptKindName: protocol.ScriptKindName | ScriptKind): ScriptKind {
return isString(scriptKindName) ? convertScriptKindName(scriptKindName) : scriptKindName;
}
Expand Down Expand Up @@ -294,6 +304,7 @@ namespace ts.server {
hostInfo: string;
extraFileExtensions?: FileExtensionInfo[];
watchOptions?: WatchOptions;
typeAcquisition?: TypeAcquisition;
Copy link
Member

Choose a reason for hiding this comment

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

You shouldn’t need this since the options are per project and there isn’t anything in host

}

export interface OpenConfiguredProjectResult {
Expand Down Expand Up @@ -642,6 +653,8 @@ namespace ts.server {
private compilerOptionsForInferredProjectsPerProjectRoot = new Map<string, CompilerOptions>();
private watchOptionsForInferredProjects: WatchOptions | undefined;
private watchOptionsForInferredProjectsPerProjectRoot = new Map<string, WatchOptions | false>();
private typeAcquisitionForInferredProjects: TypeAcquisition | undefined;
private typeAcquisitionForInferredProjectsPerProjectRoot = new Map<string, TypeAcquisition | undefined>();
/**
* Project size for configured or external projects
*/
Expand Down Expand Up @@ -988,6 +1001,7 @@ namespace ts.server {

const compilerOptions = convertCompilerOptions(projectCompilerOptions);
const watchOptions = convertWatchOptions(projectCompilerOptions);
const typeAcquisition = convertTypeAcquisition(projectCompilerOptions);

// always set 'allowNonTsExtensions' for inferred projects since user cannot configure it from the outside
// previously we did not expose a way for user to change these settings and this option was enabled by default
Expand All @@ -996,10 +1010,12 @@ namespace ts.server {
if (canonicalProjectRootPath) {
this.compilerOptionsForInferredProjectsPerProjectRoot.set(canonicalProjectRootPath, compilerOptions);
this.watchOptionsForInferredProjectsPerProjectRoot.set(canonicalProjectRootPath, watchOptions || false);
this.typeAcquisitionForInferredProjectsPerProjectRoot.set(canonicalProjectRootPath, typeAcquisition);
}
else {
this.compilerOptionsForInferredProjects = compilerOptions;
this.watchOptionsForInferredProjects = watchOptions;
this.typeAcquisitionForInferredProjects = typeAcquisition;
}

for (const project of this.inferredProjects) {
Expand Down Expand Up @@ -2299,13 +2315,18 @@ namespace ts.server {
private createInferredProject(currentDirectory: string | undefined, isSingleInferredProject?: boolean, projectRootPath?: NormalizedPath): InferredProject {
const compilerOptions = projectRootPath && this.compilerOptionsForInferredProjectsPerProjectRoot.get(projectRootPath) || this.compilerOptionsForInferredProjects!; // TODO: GH#18217
let watchOptions: WatchOptions | false | undefined;
let typeAcquisition: TypeAcquisition | undefined;
if (projectRootPath) {
watchOptions = this.watchOptionsForInferredProjectsPerProjectRoot.get(projectRootPath);
typeAcquisition = this.typeAcquisitionForInferredProjectsPerProjectRoot.get(projectRootPath);
}
if (watchOptions === undefined) {
watchOptions = this.watchOptionsForInferredProjects;
}
const project = new InferredProject(this, this.documentRegistry, compilerOptions, watchOptions || undefined, projectRootPath, currentDirectory, this.currentPluginConfigOverrides);
if (typeAcquisition === undefined) {
typeAcquisition = this.typeAcquisitionForInferredProjects;
}
const project = new InferredProject(this, this.documentRegistry, compilerOptions, watchOptions || undefined, projectRootPath, currentDirectory, this.currentPluginConfigOverrides, typeAcquisition);
if (isSingleInferredProject) {
this.inferredProjects.unshift(project);
}
Expand Down Expand Up @@ -3515,7 +3536,7 @@ namespace ts.server {
const typeAcquisition = proj.typeAcquisition!;
Debug.assert(!!typeAcquisition, "proj.typeAcquisition should be set by now");
// If type acquisition has been explicitly disabled, do not exclude anything from the project
if (typeAcquisition.enable === false) {
if (typeAcquisition.enable === false || typeAcquisition.inferTypingsFromFilenames === false) {
return [];
}

Expand Down
15 changes: 12 additions & 3 deletions src/server/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1732,6 +1732,8 @@ namespace ts.server {

private _isJsInferredProject = false;

private typeAcquisition: TypeAcquisition | undefined;

toggleJsInferredProject(isJsInferredProject: boolean) {
if (isJsInferredProject !== this._isJsInferredProject) {
this._isJsInferredProject = isJsInferredProject;
Expand Down Expand Up @@ -1770,7 +1772,8 @@ namespace ts.server {
watchOptions: WatchOptions | undefined,
projectRootPath: NormalizedPath | undefined,
currentDirectory: string | undefined,
pluginConfigOverrides: ESMap<string, any> | undefined) {
pluginConfigOverrides: ESMap<string, any> | undefined,
typeAcquisition: TypeAcquisition | undefined) {
super(InferredProject.newName(),
ProjectKind.Inferred,
projectService,
Expand All @@ -1783,6 +1786,7 @@ namespace ts.server {
watchOptions,
projectService.host,
currentDirectory);
this.typeAcquisition = typeAcquisition;
this.projectRootPath = projectRootPath && projectService.toCanonicalFileName(projectRootPath);
if (!projectRootPath && !projectService.useSingleInferredProject) {
this.canonicalCurrentDirectory = projectService.toCanonicalFileName(this.currentDirectory);
Expand Down Expand Up @@ -1827,11 +1831,16 @@ namespace ts.server {
super.close();
}

setTypeAcquisition(newTypeAcquisition: TypeAcquisition): void {
Copy link
Member

Choose a reason for hiding this comment

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

Wonder if if this.typeAcquisition and set method should be moved to Project since 3 project types use the same method.

this.typeAcquisition = this.removeLocalTypingsFromTypeAcquisition(newTypeAcquisition);
}

getTypeAcquisition(): TypeAcquisition {
return {
return this.typeAcquisition || {
enable: allRootFilesAreJsOrDts(this),
include: ts.emptyArray,
exclude: ts.emptyArray
exclude: ts.emptyArray,
inferTypingsFromFilenames: true
};
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/server/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1333,7 +1333,7 @@ namespace ts.server.protocol {
* For external projects, some of the project settings are sent together with
* compiler settings.
*/
export type ExternalProjectCompilerOptions = CompilerOptions & CompileOnSaveMixin & WatchOptions;
export type ExternalProjectCompilerOptions = CompilerOptions & CompileOnSaveMixin & WatchOptions & TypeAcquisition;
Copy link
Member

Choose a reason for hiding this comment

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

ExternalProject has options called typeAcquisition so this type shouldnt be changed.

Copy link
Member

Choose a reason for hiding this comment

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

You can create different type instead you should create a type
export type InferredProjectCompilerOptions = ExternalProjectCompilerOptions & TypeAcquisition and update the usage.


/**
* Contains information about current project version
Expand Down
89 changes: 89 additions & 0 deletions src/testRunner/unittests/tsserver/typingsInstaller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,50 @@ namespace ts.projectSystem {
checkProjectActualFiles(p, [file1.path, jquery.path]);
});

it("inferred project - type acquisition with inferTypingsFromFile:false", () => {
// Tests:
// Exclude file from filename-based type acquisition with inferTypingsFromFile:false
const jqueryJs = {
path: "/a/b/jquery.js",
content: ""
};

const messages: string[] = [];
const host = createServerHost([jqueryJs]);
const installer = new (class extends Installer {
constructor() {
super(host, { typesRegistry: createTypesRegistry("jquery") }, { isEnabled: () => true, writeLine: msg => messages.push(msg) });
}
enqueueInstallTypingsRequest(project: server.Project, typeAcquisition: TypeAcquisition, unresolvedImports: SortedReadonlyArray<string>) {
super.enqueueInstallTypingsRequest(project, typeAcquisition, unresolvedImports);
}
installWorker(_requestId: number, _args: string[], _cwd: string, cb: TI.RequestCompletedAction): void {
const installedTypings: string[] = [];
const typingFiles: File[] = [];
executeCommand(this, host, installedTypings, typingFiles, cb);
}
})();

const projectService = createProjectService(host, { typingsInstaller: installer });
projectService.setCompilerOptionsForInferredProjects({
allowJs: true,
enable: true,
inferTypingsFromFilenames: false
});
projectService.openClientFile(jqueryJs.path);

checkNumberOfProjects(projectService, { inferredProjects: 1 });
const p = projectService.inferredProjects[0];
checkProjectActualFiles(p, [jqueryJs.path]);

installer.installAll(/*expectedCount*/ 0);
host.checkTimeoutQueueLengthAndRun(2);
checkNumberOfProjects(projectService, { inferredProjects: 1 });
// files should not be removed from project if ATA is skipped
checkProjectActualFiles(p, [jqueryJs.path]);
assert.isTrue(messages.indexOf("No new typings were requested as a result of typings discovery") > 0, "Should not request filename-based typings");
});

it("external project - no type acquisition, no .d.ts/js files", () => {
const file1 = {
path: "/a/b/app.ts",
Expand Down Expand Up @@ -434,6 +478,51 @@ namespace ts.projectSystem {

installer.checkPendingCommands(/*expectedCount*/ 0);
});

it("external project - type acquisition with inferTypingsFromFilenames: false", () => {
// Tests:
// Exclude file from filename-based type acquisition with inferTypingsFromFile:false
const jqueryJs = {
path: "/a/b/jquery.js",
content: ""
};

const messages: string[] = [];
const host = createServerHost([jqueryJs]);
const installer = new (class extends Installer {
constructor() {
super(host, { typesRegistry: createTypesRegistry("jquery") }, { isEnabled: () => true, writeLine: msg => messages.push(msg) });
}
enqueueInstallTypingsRequest(project: server.Project, typeAcquisition: TypeAcquisition, unresolvedImports: SortedReadonlyArray<string>) {
super.enqueueInstallTypingsRequest(project, typeAcquisition, unresolvedImports);
}
installWorker(_requestId: number, _args: string[], _cwd: string, cb: TI.RequestCompletedAction): void {
const installedTypings: string[] = [];
const typingFiles: File[] = [];
executeCommand(this, host, installedTypings, typingFiles, cb);
}
})();

const projectFileName = "/a/app/test.csproj";
const projectService = createProjectService(host, { typingsInstaller: installer });
projectService.openExternalProject({
projectFileName,
options: { allowJS: true, moduleResolution: ModuleResolutionKind.NodeJs },
rootFiles: [toExternalFile(jqueryJs.path)],
typeAcquisition: { enable: true, inferTypingsFromFilenames: false }
});

const p = projectService.externalProjects[0];
projectService.checkNumberOfProjects({ externalProjects: 1 });
checkProjectActualFiles(p, [jqueryJs.path]);

installer.installAll(/*expectedCount*/ 0);
projectService.checkNumberOfProjects({ externalProjects: 1 });
// files should not be removed from project if ATA is skipped
checkProjectActualFiles(p, [jqueryJs.path]);
assert.isTrue(messages.indexOf("No new typings were requested as a result of typings discovery") > 0, "Should not request filename-based typings");
});

it("external project - no type acquisition, with js & ts files", () => {
// Tests:
// 1. No typings are included for JS projects when the project contains ts files
Expand Down
11 changes: 9 additions & 2 deletions tests/baselines/reference/api/tsserverlibrary.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2891,7 +2891,8 @@ declare namespace ts {
enable?: boolean;
include?: string[];
exclude?: string[];
[option: string]: string[] | boolean | undefined;
inferTypingsFromFilenames?: boolean;
[option: string]: CompilerOptionsValue | undefined;
}
export enum ModuleKind {
None = 0,
Expand Down Expand Up @@ -7501,7 +7502,7 @@ declare namespace ts.server.protocol {
* For external projects, some of the project settings are sent together with
* compiler settings.
*/
type ExternalProjectCompilerOptions = CompilerOptions & CompileOnSaveMixin & WatchOptions;
type ExternalProjectCompilerOptions = CompilerOptions & CompileOnSaveMixin & WatchOptions & TypeAcquisition;
interface FileWithProjectReferenceRedirectInfo {
/**
* Name of file
Expand Down Expand Up @@ -9353,6 +9354,7 @@ declare namespace ts.server {
class InferredProject extends Project {
private static readonly newName;
private _isJsInferredProject;
private typeAcquisition;
toggleJsInferredProject(isJsInferredProject: boolean): void;
setCompilerOptions(options?: CompilerOptions): void;
/** this is canonical project root path */
Expand All @@ -9361,6 +9363,7 @@ declare namespace ts.server {
removeRoot(info: ScriptInfo): void;
isProjectWithSingleRoot(): boolean;
close(): void;
setTypeAcquisition(newTypeAcquisition: TypeAcquisition): void;
getTypeAcquisition(): TypeAcquisition;
}
class AutoImportProviderProject extends Project {
Expand Down Expand Up @@ -9559,6 +9562,7 @@ declare namespace ts.server {
export function convertFormatOptions(protocolOptions: protocol.FormatCodeSettings): FormatCodeSettings;
export function convertCompilerOptions(protocolOptions: protocol.ExternalProjectCompilerOptions): CompilerOptions & protocol.CompileOnSaveMixin;
export function convertWatchOptions(protocolOptions: protocol.ExternalProjectCompilerOptions): WatchOptions | undefined;
export function convertTypeAcquisition(protocolOptions: protocol.ExternalProjectCompilerOptions): TypeAcquisition | undefined;
export function tryConvertScriptKindName(scriptKindName: protocol.ScriptKindName | ScriptKind): ScriptKind;
export function convertScriptKindName(scriptKindName: protocol.ScriptKindName): ScriptKind.Unknown | ScriptKind.JS | ScriptKind.JSX | ScriptKind.TS | ScriptKind.TSX;
export interface HostConfiguration {
Expand All @@ -9567,6 +9571,7 @@ declare namespace ts.server {
hostInfo: string;
extraFileExtensions?: FileExtensionInfo[];
watchOptions?: WatchOptions;
typeAcquisition?: TypeAcquisition;
}
export interface OpenConfiguredProjectResult {
configFileName?: NormalizedPath;
Expand Down Expand Up @@ -9627,6 +9632,8 @@ declare namespace ts.server {
private compilerOptionsForInferredProjectsPerProjectRoot;
private watchOptionsForInferredProjects;
private watchOptionsForInferredProjectsPerProjectRoot;
private typeAcquisitionForInferredProjects;
private typeAcquisitionForInferredProjectsPerProjectRoot;
/**
* Project size for configured or external projects
*/
Expand Down
3 changes: 2 additions & 1 deletion tests/baselines/reference/api/typescript.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2891,7 +2891,8 @@ declare namespace ts {
enable?: boolean;
include?: string[];
exclude?: string[];
[option: string]: string[] | boolean | undefined;
inferTypingsFromFilenames?: boolean;
[option: string]: CompilerOptionsValue | undefined;
}
export enum ModuleKind {
None = 0,
Expand Down