Skip to content

Commit

Permalink
refactor: Remove ngcc from extension
Browse files Browse the repository at this point in the history
NGCC is being removed in Angular v16. The language service should not
longer attempt to run it. angular/angular-cli#24720
As a result, we can now set the support for running in untrusted
workspaces to "true" since we don't have to execute ngcc from the
`node_modules` folder.
  • Loading branch information
atscott committed Feb 15, 2023
1 parent 86292ea commit fc03b58
Show file tree
Hide file tree
Showing 13 changed files with 36 additions and 400 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/node.js.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ jobs:
node-version: 14.x
- run: yarn install
- run: yarn install --cwd integration/project
- run: cd integration/project && yarn ngcc
- run: cd integration/project
- run: scripts/build.sh package.json
- run: xvfb-run -a yarn run test:e2e
if: runner.os == 'Linux'
Expand Down
16 changes: 1 addition & 15 deletions client/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import * as vscode from 'vscode';
import * as lsp from 'vscode-languageclient/node';

import {OpenOutputChannel, ProjectLoadingFinish, ProjectLoadingStart, SuggestStrictMode, SuggestStrictModeParams} from '../../common/notifications';
import {GetComponentsWithTemplateFile, GetTcbRequest, GetTemplateLocationForComponent, IsInAngularProject, RunNgccRequest} from '../../common/requests';
import {GetComponentsWithTemplateFile, GetTcbRequest, GetTemplateLocationForComponent, IsInAngularProject} from '../../common/requests';
import {resolve, Version} from '../../common/resolver';

import {isInsideComponentDecorator, isInsideInlineTemplateRegion, isInsideStringLiteral} from './embedded_support';
Expand Down Expand Up @@ -281,16 +281,6 @@ export class AngularLanguageClient implements vscode.Disposable {
};
}

runNgcc(textEditor: vscode.TextEditor): void {
if (this.client === null) {
return;
}
this.client.sendRequest(RunNgccRequest, {
textDocument:
this.client.code2ProtocolConverter.asTextDocumentIdentifier(textEditor.document),
});
}

get initializeResult(): lsp.InitializeResult|undefined {
return this.client?.initializeResult;
}
Expand Down Expand Up @@ -448,10 +438,6 @@ function constructArgs(
args.push('--includeCompletionsWithSnippetText');
}

const disableAutomaticNgcc = config.get<boolean>('angular.disableAutomaticNgcc');
if (disableAutomaticNgcc) {
args.push('--disableAutomaticNgcc');
}
const disableCodeActions = config.get<boolean>('angular.disableCodeActions');
if (disableCodeActions) {
args.push('--disableCodeActions');
Expand Down
10 changes: 0 additions & 10 deletions client/src/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,15 +115,6 @@ function getTemplateTcb(
}
};
}
function runNgcc(ngClient: AngularLanguageClient): Command {
return {
id: 'angular.runNgcc',
isTextEditorCommand: true,
async execute(textEditor: vscode.TextEditor) {
ngClient.runNgcc(textEditor);
}
};
}

/**
* Command goToComponentWithTemplateFile finds components which reference an external template in
Expand Down Expand Up @@ -189,7 +180,6 @@ export function registerCommands(
restartNgServer(client),
openLogFile(client),
getTemplateTcb(client, context),
runNgcc(client),
goToComponentWithTemplateFile(client),
goToTemplateForComponent(client),
];
Expand Down
4 changes: 0 additions & 4 deletions common/notifications.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,4 @@ export interface SuggestStrictModeParams {
export const SuggestStrictMode =
new NotificationType<SuggestStrictModeParams>('angular/suggestStrictMode');

// Report the end of `NGCC` progress and Only used by the `ivy_spec.ts`.
export const NgccProgressEnd =
new NotificationType<{configFilePath: string}>('angular/NgccProgressEnd');

export const OpenOutputChannel = new NotificationType('angular/OpenOutputChannel');
7 changes: 0 additions & 7 deletions common/requests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,9 @@ export interface GetTcbParams {
position: lsp.Position;
}

export interface RunNgccParams {
textDocument: lsp.TextDocumentIdentifier;
}

export const GetTcbRequest =
new lsp.RequestType<GetTcbParams, GetTcbResponse|null, /* error */ void>('angular/getTcb');

export const RunNgccRequest =
new lsp.RequestType<RunNgccParams, void, /* error */ void>('angular/runNgcc');

export interface GetTcbResponse {
uri: lsp.DocumentUri;
content: string;
Expand Down
85 changes: 29 additions & 56 deletions integration/lsp/ivy_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {MessageConnection} from 'vscode-jsonrpc';
import * as lsp from 'vscode-languageserver-protocol';
import {URI} from 'vscode-uri';

import {NgccProgressEnd, ProjectLanguageService, ProjectLanguageServiceParams, SuggestStrictMode, SuggestStrictModeParams} from '../../common/notifications';
import {ProjectLanguageService, ProjectLanguageServiceParams, SuggestStrictMode, SuggestStrictModeParams} from '../../common/notifications';
import {GetComponentsWithTemplateFile, GetTcbRequest, GetTemplateLocationForComponent, IsInAngularProject} from '../../common/requests';
import {APP_COMPONENT, APP_COMPONENT_URI, FOO_COMPONENT, FOO_COMPONENT_URI, FOO_TEMPLATE, FOO_TEMPLATE_URI, IS_BAZEL, PROJECT_PATH, TSCONFIG} from '../test_constants';

Expand Down Expand Up @@ -41,27 +41,9 @@ describe('Angular Ivy language server', () => {
client.dispose();
});

it('should send ngcc progress after a project has finished loading', async () => {
openTextDocument(client, APP_COMPONENT);
const configFilePath = await onNgccProgress(client);
expect(configFilePath.endsWith('integration/project/tsconfig.json')).toBeTrue();
});

it('should disable language service until ngcc has completed', async () => {
openTextDocument(client, APP_COMPONENT);
const languageServiceEnabled = await onLanguageServiceStateNotification(client);
expect(languageServiceEnabled).toBeFalse();
});

it('should re-enable language service once ngcc has completed', async () => {
openTextDocument(client, APP_COMPONENT);
const languageServiceEnabled = await waitForNgcc(client);
expect(languageServiceEnabled).toBeTrue();
});

it('should handle hover on inline template', async () => {
openTextDocument(client, APP_COMPONENT);
const languageServiceEnabled = await waitForNgcc(client);
const languageServiceEnabled = await waitForLanguageService(client);
expect(languageServiceEnabled).toBeTrue();
const response = await client.sendRequest(lsp.HoverRequest.type, {
textDocument: {
Expand All @@ -77,7 +59,7 @@ describe('Angular Ivy language server', () => {

it('should show diagnostics for inline template on open', async () => {
openTextDocument(client, APP_COMPONENT);
const languageServiceEnabled = await waitForNgcc(client);
const languageServiceEnabled = await waitForLanguageService(client);
expect(languageServiceEnabled).toBeTrue();
const diagnostics = await getDiagnosticsForFile(client, APP_COMPONENT);
expect(diagnostics.length).toBe(0);
Expand All @@ -92,7 +74,7 @@ describe('Angular Ivy language server', () => {
text: `{{ doesnotexist }}`,
},
});
const languageServiceEnabled = await waitForNgcc(client);
const languageServiceEnabled = await waitForLanguageService(client);
expect(languageServiceEnabled).toBeTrue();
const diagnostics = await getDiagnosticsForFile(client, FOO_TEMPLATE);
expect(diagnostics.length).toBe(1);
Expand All @@ -107,7 +89,7 @@ describe('Angular Ivy language server', () => {

it('should support request cancellation', async () => {
openTextDocument(client, APP_COMPONENT);
const languageServiceEnabled = await waitForNgcc(client);
const languageServiceEnabled = await waitForLanguageService(client);
expect(languageServiceEnabled).toBeTrue();
// Send a request and immediately cancel it
const promise = client.sendRequest(lsp.HoverRequest.type, {
Expand All @@ -132,7 +114,7 @@ describe('Angular Ivy language server', () => {
text: `<div *ngIf="false"></div>`,
},
});
const languageServiceEnabled = await waitForNgcc(client);
const languageServiceEnabled = await waitForLanguageService(client);
expect(languageServiceEnabled).toBeTrue();
const response = await client.sendRequest(lsp.DefinitionRequest.type, {
textDocument: {
Expand Down Expand Up @@ -168,7 +150,7 @@ describe('Angular Ivy language server', () => {
text: `<lib-post></lib-post>`,
},
});
const languageServiceEnabled = await waitForNgcc(client);
const languageServiceEnabled = await waitForLanguageService(client);
expect(languageServiceEnabled).toBeTrue();
const response = await client.sendRequest(lsp.DefinitionRequest.type, {
textDocument: {
Expand Down Expand Up @@ -199,7 +181,7 @@ export class AppComponent {
@Input() appInput = '';
@Output() appOutput = new EventEmitter<string>();
}`);
const languageServiceEnabled = await waitForNgcc(client);
const languageServiceEnabled = await waitForLanguageService(client);
expect(languageServiceEnabled).toBeTrue();
const response = await client.sendRequest(lsp.FoldingRangeRequest.type, {
textDocument: {
Expand All @@ -223,7 +205,7 @@ export class AppComponent {
text: `{{ title.toString() }}`,
},
});
const languageServiceEnabled = await waitForNgcc(client);
const languageServiceEnabled = await waitForLanguageService(client);
expect(languageServiceEnabled).toBeTrue();
const response = (await client.sendRequest(lsp.SignatureHelpRequest.type, {
textDocument: {
Expand All @@ -245,7 +227,7 @@ export class AppComponent {
text: `{{ title.substr(0, ) }}`,
},
});
const languageServiceEnabled = await waitForNgcc(client);
const languageServiceEnabled = await waitForLanguageService(client);
expect(languageServiceEnabled).toBeTrue();
const response = (await client.sendRequest(lsp.SignatureHelpRequest.type, {
textDocument: {
Expand Down Expand Up @@ -278,7 +260,7 @@ export class AppComponent {

it('should retain typecheck files', async () => {
openTextDocument(client, APP_COMPONENT);
const languageServiceEnabled = await waitForNgcc(client);
const languageServiceEnabled = await waitForLanguageService(client);
expect(languageServiceEnabled).toBeTrue();
// Create a file in node_modules, this will trigger a project reload via
// the directory watcher
Expand All @@ -296,7 +278,7 @@ export class AppComponent {
describe('completions', () => {
it('for events', async () => {
openTextDocument(client, FOO_TEMPLATE, `<my-app ()></my-app>`);
const languageServiceEnabled = await waitForNgcc(client);
const languageServiceEnabled = await waitForLanguageService(client);
expect(languageServiceEnabled).toBeTrue();
const response = await client.sendRequest(lsp.CompletionRequest.type, {
textDocument: {
Expand All @@ -316,7 +298,7 @@ export class AppComponent {
describe('from template files', () => {
beforeEach(async () => {
openTextDocument(client, FOO_TEMPLATE);
const languageServiceEnabled = await waitForNgcc(client);
const languageServiceEnabled = await waitForLanguageService(client);
expect(languageServiceEnabled).toBeTrue();
});

Expand Down Expand Up @@ -368,7 +350,7 @@ export class AppComponent {
describe('from typescript files', () => {
beforeEach(async () => {
openTextDocument(client, APP_COMPONENT);
const languageServiceEnabled = await waitForNgcc(client);
const languageServiceEnabled = await waitForLanguageService(client);
expect(languageServiceEnabled).toBeTrue();
});

Expand Down Expand Up @@ -455,7 +437,7 @@ export class AppComponent {
fs.writeFileSync(TSCONFIG, JSON.stringify(config, null, 2));

openTextDocument(client, APP_COMPONENT);
const languageServiceEnabled = await waitForNgcc(client);
const languageServiceEnabled = await waitForLanguageService(client);
expect(languageServiceEnabled).toBeTrue();
});

Expand Down Expand Up @@ -489,7 +471,7 @@ export class AppComponent {

it('should handle getTcb request', async () => {
openTextDocument(client, FOO_TEMPLATE);
await waitForNgcc(client);
await waitForLanguageService(client);
const response = await client.sendRequest(GetTcbRequest, {
textDocument: {
uri: FOO_TEMPLATE_URI,
Expand All @@ -501,7 +483,7 @@ export class AppComponent {

it('should handle goToComponent request', async () => {
openTextDocument(client, FOO_TEMPLATE);
await waitForNgcc(client);
await waitForLanguageService(client);
const response = await client.sendRequest(GetComponentsWithTemplateFile, {
textDocument: {
uri: FOO_TEMPLATE_URI,
Expand All @@ -512,7 +494,7 @@ export class AppComponent {

it('should handle GetTemplateLocationForComponent request', async () => {
openTextDocument(client, FOO_TEMPLATE);
await waitForNgcc(client);
await waitForLanguageService(client);
const response = await client.sendRequest(GetTemplateLocationForComponent, {
textDocument: {
uri: FOO_COMPONENT_URI,
Expand All @@ -525,7 +507,7 @@ export class AppComponent {

it('should handle GetTemplateLocationForComponent request when not in component', async () => {
openTextDocument(client, FOO_TEMPLATE);
await waitForNgcc(client);
await waitForLanguageService(client);
const response = await client.sendRequest(GetTemplateLocationForComponent, {
textDocument: {
uri: FOO_COMPONENT_URI,
Expand All @@ -537,7 +519,7 @@ export class AppComponent {

it('should provide a "go to component" codelens', async () => {
openTextDocument(client, FOO_TEMPLATE);
await waitForNgcc(client);
await waitForLanguageService(client);
const codeLensResponse = await client.sendRequest(lsp.CodeLensRequest.type, {
textDocument: {
uri: FOO_TEMPLATE_URI,
Expand All @@ -555,7 +537,7 @@ export class AppComponent {

it('detects an Angular project', async () => {
openTextDocument(client, FOO_TEMPLATE);
await waitForNgcc(client);
await waitForLanguageService(client);
const templateResponse = await client.sendRequest(IsInAngularProject, {
textDocument: {
uri: FOO_TEMPLATE_URI,
Expand Down Expand Up @@ -603,7 +585,7 @@ describe('auto-apply optional chaining', () => {
}
`);
openTextDocument(client, FOO_TEMPLATE, `{{ person.n }}`);
const languageServiceEnabled = await waitForNgcc(client);
const languageServiceEnabled = await waitForLanguageService(client);
expect(languageServiceEnabled).toBeTrue();
const response = await client.sendRequest(lsp.CompletionRequest.type, {
textDocument: {
Expand All @@ -618,7 +600,7 @@ describe('auto-apply optional chaining', () => {

it('should work on NonNullable symbol', async () => {
openTextDocument(client, FOO_TEMPLATE, `{{ title.substr }}`);
const languageServiceEnabled = await waitForNgcc(client);
const languageServiceEnabled = await waitForLanguageService(client);
expect(languageServiceEnabled).toBeTrue();
const response = await client.sendRequest(lsp.CompletionRequest.type, {
textDocument: {
Expand Down Expand Up @@ -655,7 +637,7 @@ describe('insert snippet text', () => {

it('should be able to complete for an attribute with the value is empty', async () => {
openTextDocument(client, FOO_TEMPLATE, `<my-app appOut></my-app>`);
const languageServiceEnabled = await waitForNgcc(client);
const languageServiceEnabled = await waitForLanguageService(client);
expect(languageServiceEnabled).toBeTrue();
const response = await client.sendRequest(lsp.CompletionRequest.type, {
textDocument: {
Expand All @@ -671,7 +653,7 @@ describe('insert snippet text', () => {

it('should not be included in the completion for an attribute with a value', async () => {
openTextDocument(client, FOO_TEMPLATE, `<my-app [appInput]="1"></my-app>`);
const languageServiceEnabled = await waitForNgcc(client);
const languageServiceEnabled = await waitForLanguageService(client);
expect(languageServiceEnabled).toBeTrue();
const response = await client.sendRequest(lsp.CompletionRequest.type, {
textDocument: {
Expand Down Expand Up @@ -709,7 +691,7 @@ describe('code fixes', () => {

it('should fix error when property does not exist on type', async () => {
openTextDocument(client, FOO_TEMPLATE, `{{titl}}`);
const languageServiceEnabled = await waitForNgcc(client);
const languageServiceEnabled = await waitForLanguageService(client);
expect(languageServiceEnabled).toBeTrue();
const diags = await getDiagnosticsForFile(client, FOO_TEMPLATE);
const codeActions = await client.sendRequest(lsp.CodeActionRequest.type, {
Expand All @@ -735,7 +717,7 @@ describe('code fixes', () => {
it('should fix error when the range the user selects is larger than the diagnostic', async () => {
const template = `<span>{{titl}}</span>`;
openTextDocument(client, FOO_TEMPLATE, template);
const languageServiceEnabled = await waitForNgcc(client);
const languageServiceEnabled = await waitForLanguageService(client);
expect(languageServiceEnabled).toBeTrue();
const diags = await getDiagnosticsForFile(client, FOO_TEMPLATE);
const codeActions = await client.sendRequest(lsp.CodeActionRequest.type, {
Expand Down Expand Up @@ -771,7 +753,7 @@ describe('code fixes', () => {
banner = '';
}
`);
const languageServiceEnabled = await waitForNgcc(client);
const languageServiceEnabled = await waitForLanguageService(client);
expect(languageServiceEnabled).toBeTrue();
});

Expand Down Expand Up @@ -838,14 +820,6 @@ describe('code fixes', () => {
});
});

function onNgccProgress(client: MessageConnection): Promise<string> {
return new Promise(resolve => {
client.onNotification(NgccProgressEnd, (params) => {
resolve(params.configFilePath);
});
});
}

function onSuggestStrictMode(client: MessageConnection): Promise<string> {
return new Promise(resolve => {
client.onNotification(SuggestStrictMode, (params: SuggestStrictModeParams) => {
Expand Down Expand Up @@ -874,7 +848,6 @@ function getDiagnosticsForFile(
});
}

async function waitForNgcc(client: MessageConnection): Promise<boolean> {
await onNgccProgress(client);
async function waitForLanguageService(client: MessageConnection): Promise<boolean> {
return onLanguageServiceStateNotification(client);
}
Loading

0 comments on commit fc03b58

Please sign in to comment.