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://github.com/angular/vscode-ng-language-service/wiki/Project-Matching-for-External-Templates

Close #976
  • Loading branch information
kyliau committed Dec 1, 2020
1 parent 2a7f59a commit fede6e6
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 12 deletions.
36 changes: 31 additions & 5 deletions integration/lsp/ivy_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,40 +18,37 @@ describe('Angular Ivy language server', () => {

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 @@ -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<string> {
Expand All @@ -84,6 +98,18 @@ function onLanguageServiceStateNotification(client: MessageConnection): Promise<
});
}

function getDiagnosticsForFile(
client: MessageConnection, fileName: string): Promise<lsp.Diagnostic[]> {
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<boolean> {
const configFilePath = await onRunNgccNotification(client);
// We run ngcc before the test, so no need to do anything here.
Expand Down
28 changes: 21 additions & 7 deletions server/src/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}

/**
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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, {
Expand Down Expand Up @@ -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;
}
5 changes: 5 additions & 0 deletions server/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

0 comments on commit fede6e6

Please sign in to comment.