From fc03b585af0b8dbb230d34984f721f6ba765b3bf Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Wed, 15 Feb 2023 14:35:28 -0800 Subject: [PATCH] refactor: Remove ngcc from extension NGCC is being removed in Angular v16. The language service should not longer attempt to run it. https://github.com/angular/angular-cli/pull/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. --- .github/workflows/node.js.yml | 2 +- client/src/client.ts | 16 +---- client/src/commands.ts | 10 --- common/notifications.ts | 4 -- common/requests.ts | 7 -- integration/lsp/ivy_spec.ts | 85 +++++++++--------------- package.json | 17 +---- server/src/cmdline_utils.ts | 5 -- server/src/ngcc.ts | 77 ---------------------- server/src/server.ts | 1 - server/src/session.ts | 116 ++------------------------------- server/src/tests/ngcc_spec.ts | 70 -------------------- server/src/version_provider.ts | 26 -------- 13 files changed, 36 insertions(+), 400 deletions(-) delete mode 100644 server/src/ngcc.ts delete mode 100644 server/src/tests/ngcc_spec.ts diff --git a/.github/workflows/node.js.yml b/.github/workflows/node.js.yml index a9d9fffa7a..0c990d32c6 100644 --- a/.github/workflows/node.js.yml +++ b/.github/workflows/node.js.yml @@ -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' diff --git a/client/src/client.ts b/client/src/client.ts index d60291c9fe..a1af91b049 100644 --- a/client/src/client.ts +++ b/client/src/client.ts @@ -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'; @@ -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; } @@ -448,10 +438,6 @@ function constructArgs( args.push('--includeCompletionsWithSnippetText'); } - const disableAutomaticNgcc = config.get('angular.disableAutomaticNgcc'); - if (disableAutomaticNgcc) { - args.push('--disableAutomaticNgcc'); - } const disableCodeActions = config.get('angular.disableCodeActions'); if (disableCodeActions) { args.push('--disableCodeActions'); diff --git a/client/src/commands.ts b/client/src/commands.ts index 0e09f6ea39..9546ed7a1d 100644 --- a/client/src/commands.ts +++ b/client/src/commands.ts @@ -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 @@ -189,7 +180,6 @@ export function registerCommands( restartNgServer(client), openLogFile(client), getTemplateTcb(client, context), - runNgcc(client), goToComponentWithTemplateFile(client), goToTemplateForComponent(client), ]; diff --git a/common/notifications.ts b/common/notifications.ts index 4f3c4477f2..f427ffd387 100644 --- a/common/notifications.ts +++ b/common/notifications.ts @@ -27,8 +27,4 @@ export interface SuggestStrictModeParams { export const SuggestStrictMode = new NotificationType('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'); \ No newline at end of file diff --git a/common/requests.ts b/common/requests.ts index 400ec2cd8a..197bb49448 100644 --- a/common/requests.ts +++ b/common/requests.ts @@ -30,16 +30,9 @@ export interface GetTcbParams { position: lsp.Position; } -export interface RunNgccParams { - textDocument: lsp.TextDocumentIdentifier; -} - export const GetTcbRequest = new lsp.RequestType('angular/getTcb'); -export const RunNgccRequest = - new lsp.RequestType('angular/runNgcc'); - export interface GetTcbResponse { uri: lsp.DocumentUri; content: string; diff --git a/integration/lsp/ivy_spec.ts b/integration/lsp/ivy_spec.ts index 8bf01c4790..f6dfa4ae87 100644 --- a/integration/lsp/ivy_spec.ts +++ b/integration/lsp/ivy_spec.ts @@ -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'; @@ -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: { @@ -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); @@ -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); @@ -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, { @@ -132,7 +114,7 @@ describe('Angular Ivy language server', () => { text: `
`, }, }); - const languageServiceEnabled = await waitForNgcc(client); + const languageServiceEnabled = await waitForLanguageService(client); expect(languageServiceEnabled).toBeTrue(); const response = await client.sendRequest(lsp.DefinitionRequest.type, { textDocument: { @@ -168,7 +150,7 @@ describe('Angular Ivy language server', () => { text: ``, }, }); - const languageServiceEnabled = await waitForNgcc(client); + const languageServiceEnabled = await waitForLanguageService(client); expect(languageServiceEnabled).toBeTrue(); const response = await client.sendRequest(lsp.DefinitionRequest.type, { textDocument: { @@ -199,7 +181,7 @@ export class AppComponent { @Input() appInput = ''; @Output() appOutput = new EventEmitter(); }`); - const languageServiceEnabled = await waitForNgcc(client); + const languageServiceEnabled = await waitForLanguageService(client); expect(languageServiceEnabled).toBeTrue(); const response = await client.sendRequest(lsp.FoldingRangeRequest.type, { textDocument: { @@ -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: { @@ -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: { @@ -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 @@ -296,7 +278,7 @@ export class AppComponent { describe('completions', () => { it('for events', async () => { openTextDocument(client, FOO_TEMPLATE, ``); - const languageServiceEnabled = await waitForNgcc(client); + const languageServiceEnabled = await waitForLanguageService(client); expect(languageServiceEnabled).toBeTrue(); const response = await client.sendRequest(lsp.CompletionRequest.type, { textDocument: { @@ -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(); }); @@ -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(); }); @@ -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(); }); @@ -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, @@ -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, @@ -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, @@ -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, @@ -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, @@ -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, @@ -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: { @@ -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: { @@ -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, ``); - const languageServiceEnabled = await waitForNgcc(client); + const languageServiceEnabled = await waitForLanguageService(client); expect(languageServiceEnabled).toBeTrue(); const response = await client.sendRequest(lsp.CompletionRequest.type, { textDocument: { @@ -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, ``); - const languageServiceEnabled = await waitForNgcc(client); + const languageServiceEnabled = await waitForLanguageService(client); expect(languageServiceEnabled).toBeTrue(); const response = await client.sendRequest(lsp.CompletionRequest.type, { textDocument: { @@ -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, { @@ -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 = `{{titl}}`; 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, { @@ -771,7 +753,7 @@ describe('code fixes', () => { banner = ''; } `); - const languageServiceEnabled = await waitForNgcc(client); + const languageServiceEnabled = await waitForLanguageService(client); expect(languageServiceEnabled).toBeTrue(); }); @@ -838,14 +820,6 @@ describe('code fixes', () => { }); }); -function onNgccProgress(client: MessageConnection): Promise { - return new Promise(resolve => { - client.onNotification(NgccProgressEnd, (params) => { - resolve(params.configFilePath); - }); - }); -} - function onSuggestStrictMode(client: MessageConnection): Promise { return new Promise(resolve => { client.onNotification(SuggestStrictMode, (params: SuggestStrictModeParams) => { @@ -874,7 +848,6 @@ function getDiagnosticsForFile( }); } -async function waitForNgcc(client: MessageConnection): Promise { - await onNgccProgress(client); +async function waitForLanguageService(client: MessageConnection): Promise { return onLanguageServiceStateNotification(client); } diff --git a/package.json b/package.json index 1cdae14e65..9651f8b0a9 100644 --- a/package.json +++ b/package.json @@ -15,8 +15,7 @@ }, "capabilities": { "untrustedWorkspaces": { - "supported": "limited", - "description": "This extension requires workspace trust because it needs to execute ngcc from the node_modules in the workspace. Projects do not require ngcc if all library dependencies are published with partial-Ivy or Full-Ivy." + "supported": true }, "virtualWorkspaces": { "supported": "limited", @@ -52,11 +51,6 @@ "command": "angular.goToTemplateForComponent", "title": "Go to template", "category": "Angular" - }, - { - "command": "angular.runNgcc", - "title": "Run ngcc", - "category": "Angular" } ], "menus": { @@ -69,10 +63,6 @@ "command": "angular.goToTemplateForComponent", "when": "editorLangId == typescript && !virtualWorkspace" }, - { - "command": "angular.runNgcc", - "when": "!virtualWorkspace && isWorkspaceTrusted" - }, { "command": "angular.getTemplateTcb", "when": "!virtualWorkspace" @@ -141,11 +131,6 @@ "default": true, "markdownDescription": "Enable/disable snippet completions from Angular language server. Requires using TypeScript 4.3+ in the workspace and the `legacy View Engine` option to be disabled." }, - "angular.disableAutomaticNgcc": { - "type": "boolean", - "default": false, - "markdownDescription": "Disable the step to automatically run ngcc. [ngcc](https://github.com/angular/angular/blob/main/packages/compiler/design/architecture.md#high-level-proposal) is required to run and gather metadata from libraries not published with Ivy instructions. This can be run outside of VSCode instead (for example, as part of the build/rebuild in the CLI). Note that ngcc needs to run not only at startup, but also whenever the dependencies change. Failing to run ngcc when required can result in incomplete information and spurious errors reported by the language service." - }, "angular.disableCodeActions": { "type": "boolean", "default": false, diff --git a/server/src/cmdline_utils.ts b/server/src/cmdline_utils.ts index 91d63a034e..c296017fb8 100644 --- a/server/src/cmdline_utils.ts +++ b/server/src/cmdline_utils.ts @@ -32,10 +32,6 @@ interface CommandLineOptions { * If true, use Ivy LS, otherwise use legacy View Engine LS. */ ivy: boolean; - /** - * If true, skips the running ngcc when using Ivy LS. - */ - disableAutomaticNgcc: boolean; disableCodeActions: boolean; logFile?: string; logVerbosity?: string; @@ -52,7 +48,6 @@ export function parseCommandLine(argv: string[]): CommandLineOptions { return { help: hasArgument(argv, '--help'), ivy: !hasArgument(argv, '--viewEngine'), - disableAutomaticNgcc: hasArgument(argv, '--disableAutomaticNgcc'), disableCodeActions: hasArgument(argv, '--disableCodeActions'), logFile: findArgument(argv, '--logFile'), logVerbosity: findArgument(argv, '--logVerbosity'), diff --git a/server/src/ngcc.ts b/server/src/ngcc.ts deleted file mode 100644 index a62713de40..0000000000 --- a/server/src/ngcc.ts +++ /dev/null @@ -1,77 +0,0 @@ -/** - * @license - * Copyright Google Inc. All Rights Reserved. - * - * Use of this source code is governed by an MIT-style license that can be - * found in the LICENSE file at https://angular.io/license - */ - -import {fork} from 'child_process'; -import {dirname, resolve} from 'path'; - -import {Version} from '../../common/resolver'; - -import {resolveNgcc} from './version_provider'; - -interface Progress { - report(msg: string): void; -} - -/** - * Resolve ngcc from the directory that contains the specified `tsconfig` and - * run ngcc. - */ -export async function resolveAndRunNgcc(tsconfig: string, progress: Progress): Promise { - const directory = dirname(tsconfig); - const ngcc = await resolveNgcc(directory); - if (!ngcc) { - throw new Error(`Failed to resolve ngcc from ${directory}`); - } - const index = ngcc.resolvedPath.indexOf('node_modules'); - // By default, ngcc assumes the node_modules directory that it needs to process - // is in the cwd. In our case, we should set cwd to the directory where ngcc - // is resolved to, not the directory where tsconfig.json is located. See - // https://github.com/angular/angular/blob/e23fd1f38205410e0ecb601ec73847cea2dea2a8/packages/compiler-cli/ngcc/src/command_line_options.ts#L18-L24 - const cwd = index > 0 ? ngcc.resolvedPath.slice(0, index) : process.cwd(); - const args: string[] = [ - '--tsconfig', - tsconfig, - ]; - if (ngcc.version.greaterThanOrEqual(new Version('11.2.4'))) { - // See https://github.com/angular/angular/commit/241784bde8582bcbc00b8a95acdeb3b0d38fbec6 - args.push('--typings-only'); - } - const childProcess = fork(ngcc.resolvedPath, args, { - cwd: resolve(cwd), - silent: true, // pipe stderr and stdout so that we can report progress - execArgv: [], // do not inherit flags like --inspect from parent process - }); - - let stderr = ''; - childProcess.stderr?.on('data', (data: Buffer) => { - stderr += data.toString(); - }); - - childProcess.stdout?.on('data', (data: Buffer) => { - for (let entry of data.toString().split('\n')) { - entry = entry.trim(); - if (entry) { - progress.report(entry); - } - } - }); - - return new Promise((resolve, reject) => { - childProcess.on('error', (error: Error) => { - reject(error); - }); - childProcess.on('close', (code: number) => { - if (code === 0) { - resolve(); - } else { - reject( - new Error(`ngcc for ${tsconfig} returned exit code ${code}, stderr: ${stderr.trim()}`)); - } - }); - }); -} diff --git a/server/src/server.ts b/server/src/server.ts index f4dfe744ef..5bcc3b3030 100644 --- a/server/src/server.ts +++ b/server/src/server.ts @@ -43,7 +43,6 @@ function main() { ngPlugin: '@angular/language-service', resolvedNgLsPath: ng.resolvedPath, ivy: isG3 ? true : options.ivy, - disableAutomaticNgcc: options.disableAutomaticNgcc || options.untrustedWorkspace, disableCodeActions: options.disableCodeActions, logToConsole: options.logToConsole, includeAutomaticOptionalChainCompletions: options.includeAutomaticOptionalChainCompletions, diff --git a/server/src/session.ts b/server/src/session.ts index 983431ba0c..f68c953f16 100644 --- a/server/src/session.ts +++ b/server/src/session.ts @@ -14,13 +14,12 @@ import {TextDocument} from 'vscode-languageserver-textdocument'; import * as lsp from 'vscode-languageserver/node'; import {ServerOptions} from '../../common/initialize'; -import {NgccProgressEnd, OpenOutputChannel, ProjectLanguageService, ProjectLoadingFinish, ProjectLoadingStart, SuggestStrictMode} from '../../common/notifications'; -import {GetComponentsWithTemplateFile, GetTcbParams, GetTcbRequest, GetTcbResponse, GetTemplateLocationForComponent, GetTemplateLocationForComponentParams, IsInAngularProject, IsInAngularProjectParams, RunNgccParams, RunNgccRequest} from '../../common/requests'; +import {OpenOutputChannel, ProjectLanguageService, ProjectLoadingFinish, ProjectLoadingStart, SuggestStrictMode} from '../../common/notifications'; +import {GetComponentsWithTemplateFile, GetTcbParams, GetTcbRequest, GetTcbResponse, GetTemplateLocationForComponent, GetTemplateLocationForComponentParams, IsInAngularProject, IsInAngularProjectParams} from '../../common/requests'; import {readNgCompletionData, tsCompletionEntryToLspCompletionItem} from './completion'; import {tsDiagnosticToLspDiagnostic} from './diagnostic'; import {getHTMLVirtualContent} from './embedded_support'; -import {resolveAndRunNgcc} from './ngcc'; import {ServerHost} from './server_host'; import {filePathToUri, getMappedDefinitionInfo, isConfiguredProject, isDebugMode, lspPositionToTsPosition, lspRangeToTsPositions, MruTracker, tsDisplayPartsToText, tsFileTextChangesToLspWorkspaceEdit, tsTextSpanToLspRange, uriToFilePath} from './utils'; @@ -30,7 +29,6 @@ export interface SessionOptions { ngPlugin: string; resolvedNgLsPath: string; ivy: boolean; - disableAutomaticNgcc: boolean; disableCodeActions: boolean; logToConsole: boolean; includeAutomaticOptionalChainCompletions: boolean; @@ -48,10 +46,6 @@ enum LanguageId { const EMPTY_RANGE = lsp.Range.create(0, 0, 0, 0); const setImmediateP = promisify(setImmediate); -enum NgccErrorMessageAction { - showOutput, -} - const defaultFormatOptions: ts.FormatCodeSettings = {}; const defaultPreferences: ts.UserPreferences = {}; @@ -66,16 +60,12 @@ export class Session { private readonly projectService: ts.server.ProjectService; private readonly logger: ts.server.Logger; private readonly ivy: boolean; - private readonly disableAutomaticNgcc: boolean; private readonly logToConsole: boolean; private readonly openFiles = new MruTracker(); private readonly includeAutomaticOptionalChainCompletions: boolean; private readonly includeCompletionsWithSnippetText: boolean; private readonly untrustedWorkspace: boolean; private snippetSupport: boolean|undefined; - // Tracks the spawn order and status of the `ngcc` processes. This allows us to ensure we enable - // the LS in the same order the projects were created in. - private projectNgccQueue: Array<{project: ts.server.Project, done: boolean}> = []; private diagnosticsTimeout: NodeJS.Timeout|null = null; private isProjectLoading = false; /** @@ -93,7 +83,6 @@ export class Session { this.includeCompletionsWithSnippetText = options.includeCompletionsWithSnippetText; this.logger = options.logger; this.ivy = options.ivy; - this.disableAutomaticNgcc = options.disableAutomaticNgcc; this.logToConsole = options.logToConsole; this.untrustedWorkspace = options.untrustedWorkspace; // Create a connection for the server. The connection uses Node's IPC as a transport. @@ -202,7 +191,6 @@ export class Session { conn.onRequest(GetComponentsWithTemplateFile, p => this.onGetComponentsWithTemplateFile(p)); conn.onRequest(GetTemplateLocationForComponent, p => this.onGetTemplateLocationForComponent(p)); conn.onRequest(GetTcbRequest, p => this.onGetTcb(p)); - conn.onRequest(RunNgccRequest, p => this.onRunNgcc(p)); conn.onRequest(IsInAngularProject, p => this.isInAngularProject(p)); conn.onCodeLens(p => this.onCodeLens(p)); conn.onCodeLensResolve(p => this.onCodeLensResolve(p)); @@ -323,18 +311,6 @@ export class Session { }; } - private onRunNgcc(params: RunNgccParams): void { - const lsInfo = this.getLSAndScriptInfo(params.textDocument); - if (lsInfo === null) { - return; - } - const project = this.getDefaultProjectForScriptInfo(lsInfo.scriptInfo); - if (!project) { - return; - } - this.runNgcc(project); - } - private onGetTemplateLocationForComponent(params: GetTemplateLocationForComponentParams): lsp.Location|null { const lsInfo = this.getLSAndScriptInfo(params.textDocument); @@ -482,8 +458,7 @@ export class Session { } this.info(`Enabling Ivy language service for ${projectName}.`); this.handleCompilerOptionsDiagnostics(project); - // Send diagnostics since we skipped this step when opening the file - // (because language service was disabled while waiting for ngcc). + // Send diagnostics since we skipped this step when opening the file. // First, make sure the Angular project is complete. this.runGlobalAnalysisForNewlyLoadedProject(project); project.refreshDiagnostics(); @@ -559,12 +534,7 @@ export class Session { const {project} = event.data; const angularCore = this.findAngularCore(project); if (angularCore) { - if (this.ivy && isExternalAngularCore(angularCore) && !this.disableAutomaticNgcc) { - // Do not wait on this promise otherwise we'll be blocking other requests - this.runNgcc(project); - } else { - this.enableLanguageServiceForProject(project); - } + this.enableLanguageServiceForProject(project); } else { this.disableLanguageServiceForProject( project, `project is not an Angular project ('@angular/core' could not be found)`); @@ -1246,84 +1216,6 @@ export class Session { return angularCore ?? null; } - /** - * Disable the language service, run ngcc, then re-enable language service. - */ - private async runNgcc(project: ts.server.Project): Promise { - if (this.untrustedWorkspace) { - this.error('Cannot run ngcc in an untrusted workspace.'); - return; - } - if (!isConfiguredProject(project) || this.projectNgccQueue.some(p => p.project === project)) { - return; - } - // Disable language service until ngcc is completed. - this.disableLanguageServiceForProject(project, 'ngcc is running'); - const configFilePath = project.getConfigFilePath(); - - const progressReporter = await this.connection.window.createWorkDoneProgress(); - - progressReporter.begin('Angular', undefined, `Running ngcc for ${configFilePath}`); - - try { - this.projectNgccQueue.push({project, done: false}); - await resolveAndRunNgcc(configFilePath, { - report: (msg: string) => { - progressReporter.report(msg); - }, - }); - } catch (e) { - this.error( - `Failed to run ngcc for ${ - configFilePath}, language service may not operate correctly:\n` + - ` ${e.message}`); - - this.connection.window - .showErrorMessage<{ - action: NgccErrorMessageAction, - title: string, - }>(`Angular extension might not work correctly because ngcc operation failed. ` + - `Try to invoke ngcc manually by running 'npm/yarn run ngcc'. ` + - `Please see the extension output for more information.`, - {title: 'Show output', action: NgccErrorMessageAction.showOutput}) - .then((selection) => { - if (selection?.action === NgccErrorMessageAction.showOutput) { - this.connection.sendNotification(OpenOutputChannel, {}); - } - }); - } finally { - const loadingStatus = this.projectNgccQueue.find(p => p.project === project); - if (loadingStatus !== undefined) { - loadingStatus.done = true; - } - this.connection.sendNotification(NgccProgressEnd, { - configFilePath, - }); - progressReporter.done(); - } - - // ngcc processes might finish out of order, but we need to re-enable the language service for - // the projects in the same order that the ngcc processes were spawned in. With solution-style - // configs, we need to ensure that the language service enabling respects the order that the - // projects were defined in the references list. If we enable the language service out of order, - // the second project in the list will request diagnostics first and then be the project that's - // prioritized for that project's set of files. This will cause issues if the second project is, - // for example, one that only includes `*.spec.ts` files and not the entire set of files needed - // to compile the app (i.e. `*.module.ts`). - const enabledProjects = new Set(); - for (let i = 0; i < this.projectNgccQueue.length && this.projectNgccQueue[i].done; i++) { - // Re-enable language service even if ngcc fails, because users could fix - // the problem by running ngcc themselves. If we keep language service - // disabled, there's no way users could use the extension even after - // resolving ngcc issues. On the client side, we will warn users about - // potentially degraded experience. - const p = this.projectNgccQueue[i].project; - this.enableLanguageServiceForProject(p); - enabledProjects.add(p); - } - this.projectNgccQueue = - this.projectNgccQueue.filter(({project}) => !enabledProjects.has(project)); - } } function toArray(it: ts.Iterator): T[] { diff --git a/server/src/tests/ngcc_spec.ts b/server/src/tests/ngcc_spec.ts deleted file mode 100644 index 735be1cc46..0000000000 --- a/server/src/tests/ngcc_spec.ts +++ /dev/null @@ -1,70 +0,0 @@ -/** - * @license - * Copyright Google Inc. All Rights Reserved. - * - * Use of this source code is governed by an MIT-style license that can be - * found in the LICENSE file at https://angular.io/license - */ - -import * as child_process from 'child_process'; -import {EventEmitter} from 'events'; -import {join, resolve} from 'path'; -import {interval, Subject} from 'rxjs'; -import {takeUntil} from 'rxjs/operators'; - -import {resolveAndRunNgcc} from '../ngcc'; - -const IS_BAZEL = !!process.env['TEST_TARGET']; - -// Outside of bazel there is an additional `dist/` path segment -const PACKAGE_ROOT = IS_BAZEL ? resolve(__dirname, '../../..') : resolve(__dirname, '../../../..'); -const WORKSPACE_ROOT = join(PACKAGE_ROOT, 'integration', 'workspace'); -const PROJECT = join(WORKSPACE_ROOT, 'projects', 'demo'); -const PRE_APF_PROJECT = join(PACKAGE_ROOT, 'integration', 'pre_apf_project'); - -describe('resolveAndRunNgcc', () => { - const afterEach$ = new Subject(); - afterEach(() => { - afterEach$.next(); - }); - - it('runs ngcc from node_modules where ngcc is resolved to', async () => { - const fakeChild = new EventEmitter(); - const spy = spyOn(child_process, 'fork').and.returnValue(fakeChild as any); - // Note that tsconfig.json is in the project directory - const tsconfig = join(PROJECT, 'tsconfig.json'); - // Because resolveNgcc is async, we need to periodically emit `close` from the child since - // `resolveAndRunNgcc` subscribes after the async resolveNgcc. - interval(500).pipe(takeUntil(afterEach$)).subscribe(() => { - fakeChild.emit('close', 0 /* exit code */); - }); - const promise = resolveAndRunNgcc(tsconfig, {report() {}}); - await expectAsync(promise).toBeResolved(); - expect(spy.calls.count()).toBe(1); - // First arg is the ngcc binary, second arg is program arguments, third - // arg is fork options. - const {cwd} = spy.calls.argsFor(0)[2]!; - // cwd for ngcc should be in the workspace directory - expect(cwd).toBe(WORKSPACE_ROOT); - }); - - it('runs ngcc from node_modules for v12 project where ngcc is resolved to', async () => { - const fakeChild = new EventEmitter(); - const spy = spyOn(child_process, 'fork').and.returnValue(fakeChild as any); - // Note that tsconfig.json is in the project directory - const tsconfig = join(PRE_APF_PROJECT, 'tsconfig.json'); - // Because resolveNgcc is async, we need to periodically emit `close` from the child since - // `resolveAndRunNgcc` subscribes after the async resolveNgcc. - interval(500).pipe(takeUntil(afterEach$)).subscribe(() => { - fakeChild.emit('close', 0 /* exit code */); - }); - const promise = resolveAndRunNgcc(tsconfig, {report() {}}); - await expectAsync(promise).toBeResolved(); - expect(spy.calls.count()).toBe(1); - // First arg is the ngcc binary, second arg is program arguments, third - // arg is fork options. - const {cwd} = spy.calls.argsFor(0)[2]!; - // cwd for ngcc should be in the workspace directory - expect(cwd).toBe(PRE_APF_PROJECT); - }); -}); diff --git a/server/src/version_provider.ts b/server/src/version_provider.ts index e7db4cb1b6..b78394e2a3 100644 --- a/server/src/version_provider.ts +++ b/server/src/version_provider.ts @@ -116,29 +116,3 @@ export function resolveNgLangSvc(probeLocations: string[]): NodeModule { const ngls = '@angular/language-service'; return resolveWithMinVersion(ngls, MIN_NG_VERSION, probeLocations, ngls); } - -export async function resolveNgcc(directory: string): Promise { - // Try to resolve ngcc from the new package format since the v13 release - try { - const ngcc = resolve('@angular/compiler-cli/ngcc', directory, '@angular/compiler-cli'); - if (ngcc === undefined) { - throw new Error('Could not resolve ngcc'); - } - - // The Angular compiler-CLI package is strict ESM as of v13. - const ngccModule = await loadEsmModule<{ngccMainFilePath: string | undefined}>( - url.pathToFileURL(ngcc.resolvedPath)); - const resolvedPath = ngccModule.ngccMainFilePath; - if (resolvedPath === undefined) { - throw new Error('could not resolve ngcc path.'); - } - - return { - ...ngcc, - resolvedPath, - }; - } catch (e) { - // Try to resolve ngcc through with pre-v13 package format - return resolve('@angular/compiler-cli/ngcc/main-ngcc.js', directory, '@angular/compiler-cli'); - } -}