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.

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

fix: match external templates to project on startup #988

Merged
merged 1 commit into from
Dec 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 32 additions & 6 deletions integration/lsp/ivy_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,47 +11,44 @@ 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(() => {
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;
}
ayazhafiz marked this conversation as resolved.
Show resolved Hide resolved
const fileName = project.getRootScriptInfos()[0].fileName;
// Getting semantic diagnostics will trigger a global analysis.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it that getting semantic diagnostics always triggers global analysis or is this just because we're triggering diagnostics on a root file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Getting semantic diagnostics always triggers global analysis, because a new compiler is created every time. Technically any file will do, not just root files, but the file must be part of the project, so root file is a convenient candidate.

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;
}