From 4405bb89df5b8a42469f9fb07b1a4a7d7b8e52d1 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Mon, 2 Dec 2019 19:58:34 +0100 Subject: [PATCH] fix(ivy): incorrectly validating html foreign objects inside svg Fixes ngtsc incorrectly logging an unknown element diagnostic for HTML elements that are inside an SVG `foreignObject` with the `xhtml` namespace. Fixes #34171. --- .../src/ngtsc/typecheck/src/dom.ts | 16 ++- .../test/ngtsc/template_typecheck_spec.ts | 97 ++++++++++++++----- .../core/test/acceptance/ng_module_spec.ts | 32 ++++++ 3 files changed, 116 insertions(+), 29 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/dom.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/dom.ts index 73d67ad68b717..c50a727bc7c78 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/dom.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/dom.ts @@ -14,6 +14,7 @@ import {ErrorCode} from '../../diagnostics'; import {TcbSourceResolver, makeTemplateDiagnostic} from './diagnostics'; const REGISTRY = new DomElementSchemaRegistry(); +const REMOVE_XHTML_REGEX = /^:xhtml:/; /** * Checks every non-Angular element/property processed in a template and potentially produces @@ -69,15 +70,20 @@ export class RegistryDomSchemaChecker implements DomSchemaChecker { constructor(private resolver: TcbSourceResolver) {} checkElement(id: string, element: TmplAstElement, schemas: SchemaMetadata[]): void { - if (!REGISTRY.hasElement(element.name, schemas)) { + // HTML elements inside an SVG `foreignObject` are declared in the `xhtml` namespace. + // We need to strip it before handing it over to the registry because all HTML tag names + // in the registry are without a namespace. + const name = element.name.replace(REMOVE_XHTML_REGEX, ''); + + if (!REGISTRY.hasElement(name, schemas)) { const mapping = this.resolver.getSourceMapping(id); - let errorMsg = `'${element.name}' is not a known element:\n`; + let errorMsg = `'${name}' is not a known element:\n`; errorMsg += - `1. If '${element.name}' is an Angular component, then verify that it is part of this module.\n`; - if (element.name.indexOf('-') > -1) { + `1. If '${name}' is an Angular component, then verify that it is part of this module.\n`; + if (name.indexOf('-') > -1) { errorMsg += - `2. If '${element.name}' is a Web Component then add 'CUSTOM_ELEMENTS_SCHEMA' to the '@NgModule.schemas' of this component to suppress this message.`; + `2. If '${name}' is a Web Component then add 'CUSTOM_ELEMENTS_SCHEMA' to the '@NgModule.schemas' of this component to suppress this message.`; } else { errorMsg += `2. To allow any element add 'NO_ERRORS_SCHEMA' to the '@NgModule.schemas' of this component.`; diff --git a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts index d102c204b481d..97c4680defb30 100644 --- a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts +++ b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts @@ -100,18 +100,18 @@ export declare class AnimationEvent { {fullTemplateTypeCheck: true, strictInputTypes: true, strictAttributeTypes: true}); env.write('test.ts', ` import {Component, Directive, NgModule, Input} from '@angular/core'; - + @Component({ selector: 'test', template: '
', }) class TestCmp {} - + @Directive({selector: '[dir]'}) class TestDir { @Input() foo: number; } - + @NgModule({ declarations: [TestCmp, TestDir], }) @@ -128,7 +128,7 @@ export declare class AnimationEvent { {fullTemplateTypeCheck: true, strictInputTypes: true, strictOutputEventTypes: true}); env.write('test.ts', ` import {Component, Directive, NgModule, EventEmitter} from '@angular/core'; - + @Component({ selector: 'test', template: '
', @@ -136,7 +136,7 @@ export declare class AnimationEvent { class TestCmp { handleEvent(event: number): void {} } - + @Directive({ selector: '[dir]', inputs: ['some-input.xs'], @@ -146,7 +146,7 @@ export declare class AnimationEvent { 'some-input.xs': string; 'some-output': EventEmitter; } - + @NgModule({ declarations: [TestCmp, TestDir], }) @@ -164,7 +164,7 @@ export declare class AnimationEvent { env.tsconfig({fullTemplateTypeCheck: true, strictOutputEventTypes: true}); env.write('test.ts', ` import {Component, Directive, EventEmitter, NgModule, Output} from '@angular/core'; - + @Component({ selector: 'test', template: '
', @@ -172,12 +172,12 @@ export declare class AnimationEvent { class TestCmp { update(data: string) {} } - + @Directive({selector: '[dir]'}) class TestDir { @Output() update = new EventEmitter(); } - + @NgModule({ declarations: [TestCmp, TestDir], }) @@ -589,7 +589,7 @@ export declare class AnimationEvent { beforeEach(() => { env.write('test.ts', ` import {Component, NgModule} from '@angular/core'; - + @Component({ selector: 'test', template: '
', @@ -597,7 +597,7 @@ export declare class AnimationEvent { class TestCmp { update(data: string) {} } - + @NgModule({ declarations: [TestCmp], }) @@ -915,7 +915,7 @@ export declare class AnimationEvent { env.write('test.ts', ` import {CommonModule} from '@angular/common'; import {Component, NgModule} from '@angular/core'; - + @Component({ selector: 'test', template: \`
@@ -926,7 +926,7 @@ export declare class AnimationEvent { export class TestCmp { foos: {name: string}[]; } - + @NgModule({ declarations: [TestCmp], imports: [CommonModule], @@ -1015,7 +1015,7 @@ export declare class AnimationEvent { static ɵdir: i0.ɵɵDirectiveDefWithMeta; } - + export declare class ExternalModule { static ɵmod: i0.ɵɵNgModuleDefWithMeta; } @@ -1024,20 +1024,20 @@ export declare class AnimationEvent { env.write('test.ts', ` import {Component, Directive, Input, NgModule} from '@angular/core'; import {BaseDir, ExternalModule} from 'external'; - + @Directive({ selector: '[child]', }) class ChildDir extends BaseDir { @Input() fromChild!: boolean; } - + @Component({ selector: 'test', template: '
', }) class TestCmp {} - + @NgModule({ declarations: [TestCmp, ChildDir], imports: [ExternalModule], @@ -1260,13 +1260,13 @@ export declare class AnimationEvent { () => { env.write('test.ts', ` import {Component, NgModule, CUSTOM_ELEMENTS_SCHEMA} from '@angular/core'; - + @Component({ selector: 'blah', template: 'test', }) export class FooCmp {} - + @NgModule({ declarations: [FooCmp], schemas: [CUSTOM_ELEMENTS_SCHEMA], @@ -1280,13 +1280,13 @@ export declare class AnimationEvent { it('should not produce diagnostics when using the NO_ERRORS_SCHEMA', () => { env.write('test.ts', ` import {Component, NgModule, NO_ERRORS_SCHEMA} from '@angular/core'; - + @Component({ selector: 'blah', template: '', }) export class FooCmp {} - + @NgModule({ declarations: [FooCmp], schemas: [NO_ERRORS_SCHEMA], @@ -1296,6 +1296,55 @@ export declare class AnimationEvent { const diags = env.driveDiagnostics(); expect(diags).toEqual([]); }); + + it('should allow HTML elements inside SVG foreignObject', () => { + env.write('test.ts', ` + import {Component, NgModule} from '@angular/core'; + @Component({ + selector: 'blah', + template: \` + + + Hello + + + \`, + }) + export class FooCmp {} + @NgModule({ + declarations: [FooCmp], + }) + export class FooModule {} + `); + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(0); + }); + + it('should check for unknown elements inside an SVG foreignObject', () => { + env.write('test.ts', ` + import {Component, NgModule} from '@angular/core'; + @Component({ + selector: 'blah', + template: \` + + + Hello + + + \`, + }) + export class FooCmp {} + @NgModule({ + declarations: [FooCmp], + }) + export class FooModule {} + `); + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(diags[0].messageText).toBe(`'foo' is not a known element: +1. If 'foo' is an Angular component, then verify that it is part of this module. +2. To allow any element add 'NO_ERRORS_SCHEMA' to the '@NgModule.schemas' of this component.`); + }); }); // Test both sync and async compilations, see https://github.com/angular/angular/issues/32538 @@ -1314,7 +1363,7 @@ export declare class AnimationEvent { it('should be correct for direct templates', async() => { env.write('test.ts', ` import {Component, NgModule} from '@angular/core'; - + @Component({ selector: 'test', template: \`

@@ -1334,7 +1383,7 @@ export declare class AnimationEvent { it('should be correct for indirect templates', async() => { env.write('test.ts', ` import {Component, NgModule} from '@angular/core'; - + const TEMPLATE = \`

{{user.does_not_exist}}

\`; @@ -1360,7 +1409,7 @@ export declare class AnimationEvent {

`); env.write('test.ts', ` import {Component, NgModule} from '@angular/core'; - + @Component({ selector: 'test', diff --git a/packages/core/test/acceptance/ng_module_spec.ts b/packages/core/test/acceptance/ng_module_spec.ts index 78f1457ea5bec..6cae8fa3fecd9 100644 --- a/packages/core/test/acceptance/ng_module_spec.ts +++ b/packages/core/test/acceptance/ng_module_spec.ts @@ -311,5 +311,37 @@ describe('NgModule', () => { }).not.toThrow(); }); + onlyInIvy('Unknown property warning logged instead of throwing an error') + .it('should not warn about HTML elements inside an SVG foreignObject', () => { + @Component({ + selector: 'my-comp', + template: ` + + + Hello + + + `, + }) + class MyComp { + condition = true; + } + + @NgModule({ + imports: [CommonModule], + declarations: [MyComp], + }) + class MyModule { + } + + TestBed.configureTestingModule({imports: [MyModule]}); + + const spy = spyOn(console, 'warn'); + const fixture = TestBed.createComponent(MyComp); + fixture.detectChanges(); + expect(spy).not.toHaveBeenCalled(); + }); + + }); });