From 1cffa6a0b0fdb7a0794f2d419d2821f4ecf1c061 Mon Sep 17 00:00:00 2001 From: Keen Yee Liau Date: Thu, 19 Nov 2020 21:16:07 -0800 Subject: [PATCH] fix: match external templates to respective projects on startup Currently, Ivy LS is not able to match external projects to their external templates if no TS files are open. This is because Ivy LS does not implement `getExternalFiles()`. To fix this, we take a different route: After ngcc has run, we send diagnostics for all the open files. But in the case where all open files are external templates, we first get diagnostics for a root TS file, then process the rest. Processing a root TS file triggers a global analysis, during which we match the external templates to their project. For a more detailed explanation, see https://github.com/angular/vscode-ng-language-service/wiki/Project-Matching-for-External-Templates Close https://github.com/angular/vscode-ng-language-service/issues/976 --- integration/lsp/ivy_spec.ts | 38 +++++++++++++++++++++++++++++++------ server/src/session.ts | 28 ++++++++++++++++++++------- server/src/utils.ts | 5 +++++ 3 files changed, 58 insertions(+), 13 deletions(-) diff --git a/integration/lsp/ivy_spec.ts b/integration/lsp/ivy_spec.ts index 3ce5078873..0eceb51ee8 100644 --- a/integration/lsp/ivy_spec.ts +++ b/integration/lsp/ivy_spec.ts @@ -11,18 +11,19 @@ import * as lsp from 'vscode-languageserver-protocol'; import {NgccComplete, ProjectLanguageService, ProjectLanguageServiceParams, RunNgcc, RunNgccParams} from '../../common/notifications'; -import {APP_COMPONENT, createConnection, initializeServer, openTextDocument} from './test_utils'; +import {APP_COMPONENT, createConnection, FOO_TEMPLATE, initializeServer, openTextDocument} from './test_utils'; describe('Angular Ivy language server', () => { jasmine.DEFAULT_TIMEOUT_INTERVAL = 10000; /* 10 seconds */ let client: MessageConnection; - beforeEach(() => { + beforeEach(async () => { client = createConnection({ ivy: true, }); client.listen(); + await initializeServer(client); }); afterEach(() => { @@ -30,28 +31,24 @@ describe('Angular Ivy language server', () => { }); it('should send ngcc notification after a project has finished loading', async () => { - await initializeServer(client); openTextDocument(client, APP_COMPONENT); const configFilePath = await onRunNgccNotification(client); expect(configFilePath.endsWith('integration/project/tsconfig.json')).toBeTrue(); }); it('should disable language service until ngcc has completed', async () => { - await initializeServer(client); openTextDocument(client, APP_COMPONENT); const languageServiceEnabled = await onLanguageServiceStateNotification(client); expect(languageServiceEnabled).toBeFalse(); }); it('should re-enable language service once ngcc has completed', async () => { - await initializeServer(client); openTextDocument(client, APP_COMPONENT); const languageServiceEnabled = await waitForNgcc(client); expect(languageServiceEnabled).toBeTrue(); }); it('should handle hover on inline template', async () => { - await initializeServer(client); openTextDocument(client, APP_COMPONENT); const languageServiceEnabled = await waitForNgcc(client); expect(languageServiceEnabled).toBeTrue(); @@ -66,6 +63,23 @@ describe('Angular Ivy language server', () => { value: '(property) AppComponent.name: string', }); }); + + it('should show existing diagnostics on external template', async () => { + client.sendNotification(lsp.DidOpenTextDocumentNotification.type, { + textDocument: { + uri: `file://${FOO_TEMPLATE}`, + languageId: 'typescript', + version: 1, + text: `{{ doesnotexist }}`, + }, + }); + const languageServiceEnabled = await waitForNgcc(client); + expect(languageServiceEnabled).toBeTrue(); + const diagnostics = await getDiagnosticsForFile(client, FOO_TEMPLATE); + expect(diagnostics.length).toBe(1); + expect(diagnostics[0].message) + .toBe(`Property 'doesnotexist' does not exist on type 'FooComponent'.`); + }); }); function onRunNgccNotification(client: MessageConnection): Promise { @@ -84,6 +98,18 @@ function onLanguageServiceStateNotification(client: MessageConnection): Promise< }); } +function getDiagnosticsForFile( + client: MessageConnection, fileName: string): Promise { + return new Promise(resolve => { + client.onNotification( + lsp.PublishDiagnosticsNotification.type, (params: lsp.PublishDiagnosticsParams) => { + if (params.uri === `file://${fileName}`) { + resolve(params.diagnostics); + } + }); + }); +} + async function waitForNgcc(client: MessageConnection): Promise { const configFilePath = await onRunNgccNotification(client); // We run ngcc before the test, so no need to do anything here. diff --git a/server/src/session.ts b/server/src/session.ts index f464b46303..8511ee0e08 100644 --- a/server/src/session.ts +++ b/server/src/session.ts @@ -15,7 +15,7 @@ import * as notification from '../common/notifications'; import {tsCompletionEntryToLspCompletionItem} from './completion'; import {tsDiagnosticToLspDiagnostic} from './diagnostic'; import {ServerHost} from './server_host'; -import {filePathToUri, lspPositionToTsPosition, lspRangeToTsPositions, tsTextSpanToLspRange, uriToFilePath} from './utils'; +import {filePathToUri, isConfiguredProject, lspPositionToTsPosition, lspRangeToTsPositions, tsTextSpanToLspRange, uriToFilePath} from './utils'; export interface SessionOptions { host: ServerHost; @@ -131,6 +131,25 @@ export class Session { // project as dirty to force update the graph. project.markAsDirty(); this.info(`Enabling Ivy language service for ${project.projectName}.`); + + // Send diagnostics since we skipped this step when opening the file + // (because language service was disabled while waiting for ngcc). + // First, make sure the Angular project is complete. + this.runGlobalAnalysisForNewlyLoadedProject(project); + project.refreshDiagnostics(); // Show initial diagnostics + } + + /** + * Invoke the compiler for the first time so that external templates get + * matched to the project they belong to. + */ + private runGlobalAnalysisForNewlyLoadedProject(project: ts.server.Project) { + if (!project.hasRoots()) { + return; + } + const fileName = project.getRootScriptInfos()[0].fileName; + // Getting semantic diagnostics will trigger a global analysis. + project.getLanguageService().getSemanticDiagnostics(fileName); } /** @@ -293,7 +312,6 @@ export class Session { this.projectService.findProject(configFileName) : this.projectService.getScriptInfo(filePath)?.containingProjects.find(isConfiguredProject); if (!project) { - this.error(`Failed to find project for ${filePath}`); return; } if (project.languageServiceEnabled) { @@ -553,7 +571,7 @@ export class Session { return; } - if (this.ivy && project instanceof ts.server.ConfiguredProject) { + if (this.ivy && isConfiguredProject(project)) { // Keep language service disabled until ngcc is completed. project.disableLanguageService(); this.connection.sendNotification(notification.RunNgcc, { @@ -594,7 +612,3 @@ export class Session { return false; } } - -function isConfiguredProject(project: ts.server.Project): project is ts.server.ConfiguredProject { - return project.projectKind === ts.server.ProjectKind.Configured; -} diff --git a/server/src/utils.ts b/server/src/utils.ts index 28981c61b7..d070683afd 100644 --- a/server/src/utils.ts +++ b/server/src/utils.ts @@ -74,3 +74,8 @@ export function lspRangeToTsPositions( const end = lspPositionToTsPosition(scriptInfo, range.end); return [start, end]; } + +export function isConfiguredProject(project: ts.server.Project): + project is ts.server.ConfiguredProject { + return project.projectKind === ts.server.ProjectKind.Configured; +}