diff --git a/goldens/public-api/compiler-cli/error_code.api.md b/goldens/public-api/compiler-cli/error_code.api.md index 979393176de..252c4c57014 100644 --- a/goldens/public-api/compiler-cli/error_code.api.md +++ b/goldens/public-api/compiler-cli/error_code.api.md @@ -81,6 +81,7 @@ export enum ErrorCode { INLINE_TYPE_CTOR_REQUIRED = 8901, INTERPOLATED_SIGNAL_NOT_INVOKED = 8109, INVALID_BANANA_IN_BOX = 8101, + INVALID_CONTENT_PLACEMENT = 8026, LET_USED_BEFORE_DEFINITION = 8016, LOCAL_COMPILATION_UNRESOLVED_CONST = 11001, LOCAL_COMPILATION_UNSUPPORTED_EXPRESSION = 11003, diff --git a/packages/compiler-cli/src/ngtsc/annotations/component/src/foreign_component.ts b/packages/compiler-cli/src/ngtsc/annotations/component/src/foreign_component.ts index 8c3a1a81112..84cbf1e3dfc 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/component/src/foreign_component.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/component/src/foreign_component.ts @@ -9,8 +9,22 @@ import { BindingType, SelectorlessMatcher, + TmplAstContent, + TmplAstContentBlock, + TmplAstDeferredBlock, + TmplAstDeferredBlockError, + TmplAstDeferredBlockLoading, + TmplAstDeferredBlockPlaceholder, TmplAstElement, + TmplAstForLoopBlock, + TmplAstForLoopBlockEmpty, + TmplAstIfBlock, + TmplAstIfBlockBranch, + TmplAstNode, TmplAstRecursiveVisitor, + TmplAstSwitchBlock, + TmplAstSwitchBlockCaseGroup, + TmplAstTemplate, tmplAstVisitAll, TypeCheckId, } from '@angular/compiler'; @@ -39,6 +53,7 @@ export function analyzeForeignComponentFeatures( } class ForeignComponentFeatureAnalyzer extends TmplAstRecursiveVisitor { + private currentParent: TmplAstNode | null = null; readonly diagnostics: ts.Diagnostic[] = []; constructor( @@ -52,6 +67,14 @@ class ForeignComponentFeatureAnalyzer extends TmplAstRecursiveVisitor { return this.foreignMatcher !== null && this.foreignMatcher.match(tagName).length > 0; } + private parentNodeIsForeignComponent(): boolean { + return ( + this.currentParent !== null && + this.currentParent instanceof TmplAstElement && + this.elementIsForeignComponent(this.currentParent.name) + ); + } + override visitElement(element: TmplAstElement): void { if (this.elementIsForeignComponent(element.name)) { if (element.outputs.length > 0) { @@ -92,6 +115,112 @@ class ForeignComponentFeatureAnalyzer extends TmplAstRecursiveVisitor { } } + const prevParent = this.currentParent; + this.currentParent = element; super.visitElement(element); + this.currentParent = prevParent; + } + + override visitTemplate(template: TmplAstTemplate): void { + const prevParent = this.currentParent; + this.currentParent = template; + super.visitTemplate(template); + this.currentParent = prevParent; + } + + override visitDeferredBlock(deferred: TmplAstDeferredBlock): void { + const prevParent = this.currentParent; + this.currentParent = deferred; + super.visitDeferredBlock(deferred); + this.currentParent = prevParent; + } + + override visitDeferredBlockPlaceholder(block: TmplAstDeferredBlockPlaceholder): void { + const prevParent = this.currentParent; + this.currentParent = block; + super.visitDeferredBlockPlaceholder(block); + this.currentParent = prevParent; + } + + override visitDeferredBlockError(block: TmplAstDeferredBlockError): void { + const prevParent = this.currentParent; + this.currentParent = block; + super.visitDeferredBlockError(block); + this.currentParent = prevParent; + } + + override visitDeferredBlockLoading(block: TmplAstDeferredBlockLoading): void { + const prevParent = this.currentParent; + this.currentParent = block; + super.visitDeferredBlockLoading(block); + this.currentParent = prevParent; + } + + override visitSwitchBlock(block: TmplAstSwitchBlock): void { + const prevParent = this.currentParent; + this.currentParent = block; + super.visitSwitchBlock(block); + this.currentParent = prevParent; + } + + override visitSwitchBlockCaseGroup(block: TmplAstSwitchBlockCaseGroup): void { + const prevParent = this.currentParent; + this.currentParent = block; + super.visitSwitchBlockCaseGroup(block); + this.currentParent = prevParent; + } + + override visitForLoopBlock(block: TmplAstForLoopBlock): void { + const prevParent = this.currentParent; + this.currentParent = block; + super.visitForLoopBlock(block); + this.currentParent = prevParent; + } + + override visitForLoopBlockEmpty(block: TmplAstForLoopBlockEmpty): void { + const prevParent = this.currentParent; + this.currentParent = block; + super.visitForLoopBlockEmpty(block); + this.currentParent = prevParent; + } + + override visitIfBlock(block: TmplAstIfBlock): void { + const prevParent = this.currentParent; + this.currentParent = block; + super.visitIfBlock(block); + this.currentParent = prevParent; + } + + override visitIfBlockBranch(block: TmplAstIfBlockBranch): void { + const prevParent = this.currentParent; + this.currentParent = block; + super.visitIfBlockBranch(block); + this.currentParent = prevParent; + } + + override visitContent(content: TmplAstContent): void { + const prevParent = this.currentParent; + this.currentParent = content; + super.visitContent(content); + this.currentParent = prevParent; + } + + override visitContentBlock(block: TmplAstContentBlock): void { + if (!this.parentNodeIsForeignComponent()) { + this.diagnostics.push( + makeTemplateDiagnostic( + '' as TypeCheckId, + this.sourceMapping, + block.sourceSpan, + ts.DiagnosticCategory.Error, + ngErrorCode(ErrorCode.INVALID_CONTENT_PLACEMENT), + '@content blocks are only valid as direct children of foreign components.', + ), + ); + } + const prevParent = this.currentParent; + this.currentParent = block; + super.visitContentBlock(block); + this.currentParent = prevParent; } } diff --git a/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts b/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts index 504de50ccfe..dff4b9c65f7 100644 --- a/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts +++ b/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts @@ -464,6 +464,11 @@ export enum ErrorCode { */ FOREIGN_COMPONENT_UNSUPPORTED_BINDING = 8025, + /** + * Raised when a `@content` block is not used as a direct child of a foreign component. + */ + INVALID_CONTENT_PLACEMENT = 8026, + /** * A two way binding in a template has an incorrect syntax, * parentheses outside brackets. For example: diff --git a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts index 1cf9fbfefd2..a08f040d7bb 100644 --- a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts +++ b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts @@ -2320,6 +2320,90 @@ runInEachFileSystem(() => { ); env.driveMain(); }); + + it('should allow @content block when used as a direct child of a foreign component', () => { + env.write( + 'test.ts', + ` + ${foreignSetupCode} + + @Component({ + selector: 'test', + template: ' @content(icon) {} ', + foreignImports: [frameworkImport(FancyButton)], + }) + export class TestCmp {} + `, + ); + const diags = env.driveDiagnostics(); + expect(diags.length).toEqual(0); + }); + + it('should detect @content block used outside of a foreign component', () => { + env.write( + 'test.ts', + ` + ${foreignSetupCode} + + @Component({ + selector: 'test', + template: '
@content(icon) {}
', + foreignImports: [frameworkImport(FancyButton)], + }) + export class TestCmp {} + `, + ); + const diags = env.driveDiagnostics(); + expect(diags.length).toEqual(1); + expect(diags[0].code).toEqual(ngErrorCode(ErrorCode.INVALID_CONTENT_PLACEMENT)); + expect(diags[0].messageText).toEqual( + '@content blocks are only valid as direct children of foreign components.', + ); + }); + + it('should detect @content block nested inside a foreign component but not as a direct child', () => { + env.write( + 'test.ts', + ` + ${foreignSetupCode} + + @Component({ + selector: 'test', + template: '
@content(icon) {}
', + foreignImports: [frameworkImport(FancyButton)], + }) + export class TestCmp {} + `, + ); + const diags = env.driveDiagnostics(); + expect(diags.length).toEqual(1); + expect(diags[0].code).toEqual(ngErrorCode(ErrorCode.INVALID_CONTENT_PLACEMENT)); + expect(diags[0].messageText).toEqual( + '@content blocks are only valid as direct children of foreign components.', + ); + }); + + it('should detect @content block nested inside a block (like @if) within a foreign component', () => { + env.write( + 'test.ts', + ` + ${foreignSetupCode} + + @Component({ + selector: 'test', + template: ' @if (true) { @content (icon) {} } ', + foreignImports: [frameworkImport(FancyButton)], + }) + export class TestCmp {} + `, + ); + const diags = env.driveDiagnostics(); + expect(diags.length).toEqual(1); + expect(diags[0].code).toEqual(ngErrorCode(ErrorCode.INVALID_CONTENT_PLACEMENT)); + expect(diags[0].messageText).toEqual( + '@content blocks are only valid as direct children of foreign components.', + ); + }); }); it('should detect a duplicate variable declaration', () => { diff --git a/packages/compiler/src/template/pipeline/src/ingest.ts b/packages/compiler/src/template/pipeline/src/ingest.ts index 45ef4273907..91d18a98176 100644 --- a/packages/compiler/src/template/pipeline/src/ingest.ts +++ b/packages/compiler/src/template/pipeline/src/ingest.ts @@ -268,8 +268,6 @@ function ingestNodes(unit: ViewCompilationUnit, template: t.Node[]): void { ingestForBlock(unit, node); } else if (node instanceof t.LetDeclaration) { ingestLetDeclaration(unit, node); - } else if (node instanceof t.ContentBlock) { - throw new Error(`@content blocks are only valid as direct children of foreign components.`); } else if (node instanceof t.Component) { // TODO(crisbeto): account for selectorless nodes. } else {