Skip to content

Commit

Permalink
fix: match external templates to respective projects on startup
Browse files Browse the repository at this point in the history
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://docs.google.com/document/d/1rrgPvQG6G9KfywhVfFhNvNOlwyRlx7c0TWw5WZKSvY8/edit?usp=sharing

Close #976
  • Loading branch information
kyliau committed Nov 20, 2020
1 parent ff5dcda commit 072534c
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 13 deletions.
34 changes: 28 additions & 6 deletions integration/lsp/ivy_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,47 +8,45 @@

import {MessageConnection} from 'vscode-jsonrpc';
import * as lsp from 'vscode-languageserver-protocol';
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(() => {
client.dispose();
});

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();
Expand All @@ -63,6 +61,30 @@ 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: lsp.Diagnostic[] = await new Promise(resolve => {
client.onNotification(
lsp.PublishDiagnosticsNotification.type, (params: lsp.PublishDiagnosticsParams) => {
if (params.uri === `file://${FOO_TEMPLATE}`) {
resolve(params.diagnostics);
}
});
});
expect(diagnostics.length).toBe(1);
expect(diagnostics[0].message)
.toBe(`Property 'doesnotexist' does not exist on type 'FooComponent'.`);
});
});

function onRunNgccNotification(client: MessageConnection): Promise<string> {
Expand Down
27 changes: 20 additions & 7 deletions server/src/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {tsCompletionEntryToLspCompletionItem} from './completion';
import {tsDiagnosticToLspDiagnostic} from './diagnostic';
import {Logger} from './logger';
import {ServerHost} from './server_host';
import {filePathToUri, lspPositionToTsPosition, lspRangeToTsPositions, tsTextSpanToLspRange, uriToFilePath} from './utils';
import {filePathToUri, isConfiguredProject, isTypeScriptFile, lspPositionToTsPosition, lspRangeToTsPositions, tsTextSpanToLspRange, uriToFilePath} from './utils';

export interface SessionOptions {
host: ServerHost;
Expand Down Expand Up @@ -131,6 +131,24 @@ 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).
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);
}

/**
Expand Down Expand Up @@ -289,7 +307,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) {
Expand Down Expand Up @@ -549,7 +566,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, {
Expand Down Expand Up @@ -590,7 +607,3 @@ export class Session {
return false;
}
}

function isConfiguredProject(project: ts.server.Project): project is ts.server.ConfiguredProject {
return project.projectKind === ts.server.ProjectKind.Configured;
}
9 changes: 9 additions & 0 deletions server/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,3 +74,12 @@ 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;
}

export function isTypeScriptFile(fileName: string): boolean {
return fileName.endsWith('.ts');
}

0 comments on commit 072534c

Please sign in to comment.