From dbe612f2cd59adecdab3abb270b014c4b26e472c Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Tue, 29 Oct 2024 13:49:42 +0100 Subject: [PATCH] fix(compiler-cli): disable standalone by default on older versions of Angular (#58405) Disables the standalone by default behavior in the compiler when running against and older version of Angular. This is necessary, because the language service may be using the latest version of the compiler against and older version of core in a particular workspace. PR Close #58405 --- .../common/src/standalone-default-value.ts | 13 ---- .../annotations/component/src/handler.ts | 2 + .../component/test/component_spec.ts | 1 + .../annotations/directive/src/handler.ts | 2 + .../ngtsc/annotations/directive/src/shared.ts | 4 +- .../directive/test/directive_spec.ts | 1 + .../src/ngtsc/annotations/src/pipe.ts | 4 +- .../src/ngtsc/core/src/compiler.ts | 12 ++- .../compiler-cli/test/ngtsc/ngtsc_spec.ts | 78 ++++++++++++++++++- 9 files changed, 98 insertions(+), 19 deletions(-) delete mode 100644 packages/compiler-cli/src/ngtsc/annotations/common/src/standalone-default-value.ts diff --git a/packages/compiler-cli/src/ngtsc/annotations/common/src/standalone-default-value.ts b/packages/compiler-cli/src/ngtsc/annotations/common/src/standalone-default-value.ts deleted file mode 100644 index 71d670e3655..00000000000 --- a/packages/compiler-cli/src/ngtsc/annotations/common/src/standalone-default-value.ts +++ /dev/null @@ -1,13 +0,0 @@ -/** - * @license - * Copyright Google LLC 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 - */ - -/** - * A constant defining the default value for the standalone attribute in Directive and Pipes decorators. - * Extracted to a separate file to facilitate G3 patches. - */ -export const NG_STANDALONE_DEFAULT_VALUE = true; diff --git a/packages/compiler-cli/src/ngtsc/annotations/component/src/handler.ts b/packages/compiler-cli/src/ngtsc/annotations/component/src/handler.ts index 42ff83e42e6..5289aeade34 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/component/src/handler.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/component/src/handler.ts @@ -258,6 +258,7 @@ export class ComponentDecoratorHandler private readonly i18nPreserveSignificantWhitespace: boolean, private readonly strictStandalone: boolean, private readonly enableHmr: boolean, + private readonly implicitStandaloneValue: boolean, ) { this.extractTemplateOptions = { enableI18nLegacyMessageIdFormat: this.enableI18nLegacyMessageIdFormat, @@ -464,6 +465,7 @@ export class ComponentDecoratorHandler this.compilationMode, this.elementSchemaRegistry.getDefaultComponentElementName(), this.strictStandalone, + this.implicitStandaloneValue, ); // `extractDirectiveMetadata` returns `jitForced = true` when the `@Component` has // set `jit: true`. In this case, compilation of the decorator is skipped. Returning diff --git a/packages/compiler-cli/src/ngtsc/annotations/component/test/component_spec.ts b/packages/compiler-cli/src/ngtsc/annotations/component/test/component_spec.ts index b1fbe2bbeca..af1808cf529 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/component/test/component_spec.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/component/test/component_spec.ts @@ -159,6 +159,7 @@ function setup( /* i18nPreserveSignificantWhitespace */ true, /* strictStandalone */ false, /* enableHmr */ false, + /* implicitStandaloneValue */ true, ); return {reflectionHost, handler, resourceLoader, metaRegistry}; } diff --git a/packages/compiler-cli/src/ngtsc/annotations/directive/src/handler.ts b/packages/compiler-cli/src/ngtsc/annotations/directive/src/handler.ts index 9b35fe9b50e..c2e1a181f39 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/directive/src/handler.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/directive/src/handler.ts @@ -137,6 +137,7 @@ export class DirectiveDecoratorHandler private readonly compilationMode: CompilationMode, private readonly jitDeclarationRegistry: JitDeclarationRegistry, private readonly strictStandalone: boolean, + private readonly implicitStandaloneValue: boolean, ) {} readonly precedence = HandlerPrecedence.PRIMARY; @@ -192,6 +193,7 @@ export class DirectiveDecoratorHandler this.compilationMode, /* defaultSelector */ null, this.strictStandalone, + this.implicitStandaloneValue, ); // `extractDirectiveMetadata` returns `jitForced = true` when the `@Directive` has // set `jit: true`. In this case, compilation of the decorator is skipped. Returning diff --git a/packages/compiler-cli/src/ngtsc/annotations/directive/src/shared.ts b/packages/compiler-cli/src/ngtsc/annotations/directive/src/shared.ts index a31b3e4f3f0..ef194b378d2 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/directive/src/shared.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/directive/src/shared.ts @@ -84,7 +84,6 @@ import {tryParseSignalInputMapping} from './input_function'; import {tryParseSignalModelMapping} from './model_function'; import {tryParseInitializerBasedOutput} from './output_function'; import {tryParseSignalQueryFromInitializer} from './query_functions'; -import {NG_STANDALONE_DEFAULT_VALUE} from '../../common/src/standalone-default-value'; const EMPTY_OBJECT: {[key: string]: string} = {}; @@ -117,6 +116,7 @@ export function extractDirectiveMetadata( compilationMode: CompilationMode, defaultSelector: string | null, strictStandalone: boolean, + implicitStandaloneValue: boolean, ): | { jitForced: false; @@ -336,7 +336,7 @@ export function extractDirectiveMetadata( dep.token.value.name === 'TemplateRef', ); - let isStandalone = NG_STANDALONE_DEFAULT_VALUE; + let isStandalone = implicitStandaloneValue; if (directive.has('standalone')) { const expr = directive.get('standalone')!; const resolved = evaluator.evaluate(expr); diff --git a/packages/compiler-cli/src/ngtsc/annotations/directive/test/directive_spec.ts b/packages/compiler-cli/src/ngtsc/annotations/directive/test/directive_spec.ts index 981fc5610c8..be335289c56 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/directive/test/directive_spec.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/directive/test/directive_spec.ts @@ -215,6 +215,7 @@ runInEachFileSystem(() => { /*compilationMode */ CompilationMode.FULL, jitDeclarationRegistry, /* strictStandalone */ false, + /* implicitStandaloneValue */ true, ); const DirNode = getDeclaration(program, _('/entry.ts'), dirName, isNamedClassDeclaration); diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/pipe.ts b/packages/compiler-cli/src/ngtsc/annotations/src/pipe.ts index a3793e4c5b2..36db6447a5a 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/pipe.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/pipe.ts @@ -49,7 +49,6 @@ import { unwrapExpression, wrapTypeReference, } from '../common'; -import {NG_STANDALONE_DEFAULT_VALUE} from '../common/src/standalone-default-value'; export interface PipeHandlerData { meta: R3PipeMetadata; @@ -97,6 +96,7 @@ export class PipeDecoratorHandler private readonly compilationMode: CompilationMode, private readonly generateExtraImportsInLocalMode: boolean, private readonly strictStandalone: boolean, + private readonly implicitStandaloneValue: boolean, ) {} readonly precedence = HandlerPrecedence.PRIMARY; @@ -177,7 +177,7 @@ export class PipeDecoratorHandler pure = pureValue; } - let isStandalone = NG_STANDALONE_DEFAULT_VALUE; + let isStandalone = this.implicitStandaloneValue; if (pipe.has('standalone')) { const expr = pipe.get('standalone')!; const resolved = this.evaluator.evaluate(expr); diff --git a/packages/compiler-cli/src/ngtsc/core/src/compiler.ts b/packages/compiler-cli/src/ngtsc/core/src/compiler.ts index cd5cd280645..7e451478398 100644 --- a/packages/compiler-cli/src/ngtsc/core/src/compiler.ts +++ b/packages/compiler-cli/src/ngtsc/core/src/compiler.ts @@ -391,6 +391,7 @@ export class NgCompiler { private readonly enableLetSyntax: boolean; private readonly angularCoreVersion: string | null; private readonly enableHmr: boolean; + private readonly implicitStandaloneValue: boolean; /** * `NgCompiler` can be reused for multiple compilations (for resource-only changes), and each @@ -455,6 +456,7 @@ export class NgCompiler { readonly usePoisonedData: boolean, private livePerfRecorder: ActivePerfRecorder, ) { + this.angularCoreVersion = options['_angularCoreVersion'] ?? null; this.delegatingPerfRecorder = new DelegatingPerfRecorder(this.perfRecorder); this.usePoisonedData = usePoisonedData || !!options._compilePoisonedComponents; this.enableTemplateTypeChecker = @@ -462,7 +464,12 @@ export class NgCompiler { // TODO(crisbeto): remove this flag and base `enableBlockSyntax` on the `angularCoreVersion`. this.enableBlockSyntax = options['_enableBlockSyntax'] ?? true; this.enableLetSyntax = options['_enableLetSyntax'] ?? true; - this.angularCoreVersion = options['_angularCoreVersion'] ?? null; + // Standalone by default is enabled since v19. We need to toggle it here, + // because the language service extension may be running with the latest + // version of the compiler against an older version of Angular. + this.implicitStandaloneValue = + this.angularCoreVersion === null || + coreVersionSupportsFeature(this.angularCoreVersion, '>= 19.0.0-0'); this.enableHmr = !!options['_enableHmr']; this.constructionDiagnostics.push( ...this.adapter.constructionDiagnostics, @@ -1494,6 +1501,7 @@ export class NgCompiler { this.options.i18nPreserveWhitespaceForLegacyExtraction ?? true, !!this.options.strictStandalone, this.enableHmr, + this.implicitStandaloneValue, ), // TODO(alxhub): understand why the cast here is necessary (something to do with `null` @@ -1517,6 +1525,7 @@ export class NgCompiler { compilationMode, jitDeclarationRegistry, !!this.options.strictStandalone, + this.implicitStandaloneValue, ) as Readonly>, // Pipe handler must be before injectable handler in list so pipe factories are printed // before injectable factories (so injectable factories can delegate to them) @@ -1532,6 +1541,7 @@ export class NgCompiler { compilationMode, !!this.options.generateExtraImportsInLocalMode, !!this.options.strictStandalone, + this.implicitStandaloneValue, ), new InjectableDecoratorHandler( reflector, diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts index eb75efd92a4..769757a0385 100644 --- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts +++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts @@ -13,7 +13,7 @@ import {platform} from 'os'; import ts from 'typescript'; import {ErrorCode, ngErrorCode} from '../../src/ngtsc/diagnostics'; -import {absoluteFrom, NgtscCompilerHost} from '../../src/ngtsc/file_system'; +import {absoluteFrom} from '../../src/ngtsc/file_system'; import {runInEachFileSystem} from '../../src/ngtsc/file_system/testing'; import {loadStandardTestFiles} from '../../src/ngtsc/testing'; import { @@ -10928,6 +10928,82 @@ runInEachFileSystem((os: string) => { expect(env.getContents('/test.js')).toContain(`* @fileoverview Closure comment`); }); }); + + describe('standalone by default opt-out', () => { + it('should consider declarations as standalone by default', () => { + env.write( + '/test.ts', + ` + import {Directive, Component, Pipe, NgModule} from '@angular/core'; + + @Directive() + export class TestDir {} + + @Component({template: ''}) + export class TestComp {} + + @Pipe({name: 'test'}) + export class TestPipe {} + + @NgModule({ + declarations: [TestDir, TestComp, TestPipe] + }) + export class TestModule {} + `, + ); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(3); + expect(diags[0].messageText).toContain( + 'Directive TestDir is standalone, and cannot be declared in an NgModule.', + ); + expect(diags[1].messageText).toContain( + 'Component TestComp is standalone, and cannot be declared in an NgModule.', + ); + expect(diags[2].messageText).toContain( + 'Pipe TestPipe is standalone, and cannot be declared in an NgModule.', + ); + }); + + it('should disable standalone by default on versions older than 19', () => { + env.tsconfig({ + _angularCoreVersion: '18.2.10', + }); + + env.write( + '/test.ts', + ` + import {Directive, Component, Pipe, NgModule} from '@angular/core'; + + @Directive() + export class TestDir {} + + @Component({template: ''}) + export class TestComp {} + + @Pipe({name: 'test'}) + export class TestPipe {} + + @NgModule({ + imports: [TestDir, TestComp, TestPipe] + }) + export class TestModule {} + `, + ); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(3); + expect(diags[0].messageText).toContain( + `The directive 'TestDir' appears in 'imports', but is not standalone`, + ); + expect(diags[1].messageText).toContain( + `The component 'TestComp' appears in 'imports', but is not standalone`, + ); + expect(diags[2].messageText).toContain( + `The pipe 'TestPipe' appears in 'imports', but is not standalone`, + ); + }); + }); }); function expectTokenAtPosition(