fix(ivy): warn instead of throwing for unknown elements (#34524)

Follow-up from [this discussion](https://github.com/angular/angular/pull/33419#discussion_r339296216). In Ivy we don't use the schema to validate tag names, but instead we use feature detection to figure out whether an element is supported. While this should generally be more accurate, it'll also end up throwing for some more innocent cases. E.g. now Ivy throws an error for `main` elements in IE which is accurate since IE doesn't support the element, but is annoying since there is no functionality attached.

These changes switch to logging a warning instead, similarly to what we're doing for unknown properties.

PR Close #34524
This commit is contained in:
crisbeto 2020-01-07 06:26:47 +01:00 committed by Alex Rickabaugh
parent 07ea6cf582
commit 6fda7f3da4
2 changed files with 205 additions and 87 deletions

View File

@ -40,7 +40,7 @@ function elementStartFirstCreatePass(
const hasDirectives = const hasDirectives =
resolveDirectives(tView, lView, tNode, getConstant<string[]>(tViewConsts, localRefsIndex)); resolveDirectives(tView, lView, tNode, getConstant<string[]>(tViewConsts, localRefsIndex));
ngDevMode && validateElement(lView, native, tNode, hasDirectives); ngDevMode && warnAboutUnknownElement(lView, native, tNode, hasDirectives);
if (tView.queries !== null) { if (tView.queries !== null) {
tView.queries.elementStart(tView, tNode); tView.queries.elementStart(tView, tNode);
@ -253,7 +253,7 @@ function setDirectiveStylingInput(
setInputsForProperty(lView, stylingInputs, propName, value); setInputsForProperty(lView, stylingInputs, propName, value);
} }
function validateElement( function warnAboutUnknownElement(
hostView: LView, element: RElement, tNode: TNode, hasDirectives: boolean): void { hostView: LView, element: RElement, tNode: TNode, hasDirectives: boolean): void {
const schemas = hostView[TVIEW].schemas; const schemas = hostView[TVIEW].schemas;
@ -279,17 +279,17 @@ function validateElement(
!customElements.get(tagName)); !customElements.get(tagName));
if (isUnknown && !matchingSchemas(hostView, tagName)) { if (isUnknown && !matchingSchemas(hostView, tagName)) {
let errorMessage = `'${tagName}' is not a known element:\n`; let warning = `'${tagName}' is not a known element:\n`;
errorMessage += warning +=
`1. If '${tagName}' is an Angular component, then verify that it is part of this module.\n`; `1. If '${tagName}' is an Angular component, then verify that it is part of this module.\n`;
if (tagName && tagName.indexOf('-') > -1) { if (tagName && tagName.indexOf('-') > -1) {
errorMessage += warning +=
`2. If '${tagName}' is a Web Component then add 'CUSTOM_ELEMENTS_SCHEMA' to the '@NgModule.schemas' of this component to suppress this message.`; `2. If '${tagName}' is a Web Component then add 'CUSTOM_ELEMENTS_SCHEMA' to the '@NgModule.schemas' of this component to suppress this message.`;
} else { } else {
errorMessage += warning +=
`2. To allow any element add 'NO_ERRORS_SCHEMA' to the '@NgModule.schemas' of this component.`; `2. To allow any element add 'NO_ERRORS_SCHEMA' to the '@NgModule.schemas' of this component.`;
} }
throw new Error(errorMessage); console.warn(warning);
} }
} }
} }

View File

@ -166,36 +166,69 @@ describe('NgModule', () => {
}).not.toThrow(); }).not.toThrow();
}); });
it('should throw unknown element error without CUSTOM_ELEMENTS_SCHEMA for element with dash in tag name', onlyInIvy('unknown element check logs a warning rather than throwing')
() => { .it('should warn about unknown element without CUSTOM_ELEMENTS_SCHEMA for element with dash in tag name',
@Component({template: `<custom-el></custom-el>`}) () => {
class MyComp {
}
TestBed.configureTestingModule({declarations: [MyComp]}); @Component({template: `<custom-el></custom-el>`})
class MyComp {
}
expect(() => { const spy = spyOn(console, 'warn');
const fixture = TestBed.createComponent(MyComp); TestBed.configureTestingModule({declarations: [MyComp]});
fixture.detectChanges(); const fixture = TestBed.createComponent(MyComp);
}).toThrowError(/'custom-el' is not a known element/); fixture.detectChanges();
}); expect(spy.calls.mostRecent().args[0]).toMatch(/'custom-el' is not a known element/);
});
it('should throw unknown element error without CUSTOM_ELEMENTS_SCHEMA for element without dash in tag name', modifiedInIvy('unknown element error thrown instead of warning')
() => { .it('should throw unknown element error without CUSTOM_ELEMENTS_SCHEMA for element with dash in tag name',
@Component({template: `<custom></custom>`}) () => {
class MyComp { @Component({template: `<custom-el></custom-el>`})
} class MyComp {
}
TestBed.configureTestingModule({declarations: [MyComp]}); TestBed.configureTestingModule({declarations: [MyComp]});
expect(() => { expect(() => {
const fixture = TestBed.createComponent(MyComp); const fixture = TestBed.createComponent(MyComp);
fixture.detectChanges(); fixture.detectChanges();
}).toThrowError(/'custom' is not a known element/); }).toThrowError(/'custom-el' is not a known element/);
}); });
onlyInIvy('unknown element check logs a warning rather than throwing')
.it('should warn about unknown element without CUSTOM_ELEMENTS_SCHEMA for element without dash in tag name',
() => {
@Component({template: `<custom></custom>`})
class MyComp {
}
const spy = spyOn(console, 'warn');
TestBed.configureTestingModule({declarations: [MyComp]});
const fixture = TestBed.createComponent(MyComp);
fixture.detectChanges();
expect(spy.calls.mostRecent().args[0]).toMatch(/'custom' is not a known element/);
});
modifiedInIvy('unknown element error thrown instead of warning')
.it('should throw unknown element error without CUSTOM_ELEMENTS_SCHEMA for element without dash in tag name',
() => {
@Component({template: `<custom></custom>`})
class MyComp {
}
TestBed.configureTestingModule({declarations: [MyComp]});
expect(() => {
const fixture = TestBed.createComponent(MyComp);
fixture.detectChanges();
}).toThrowError(/'custom' is not a known element/);
});
onlyInIvy('test relies on Ivy-specific AOT format') onlyInIvy('test relies on Ivy-specific AOT format')
.it('should not throw unknown element error for AOT-compiled components', () => { .it('should not log unknown element warning for AOT-compiled components', () => {
const spy = spyOn(console, 'warn');
/* /*
* @Component({ * @Component({
* selector: 'comp', * selector: 'comp',
@ -253,88 +286,173 @@ describe('NgModule', () => {
imports: [MyModule], imports: [MyModule],
}); });
const fixture = TestBed.createComponent(MyComp);
fixture.detectChanges();
expect(spy).not.toHaveBeenCalled();
});
onlyInIvy('unknown element check logs a warning rather than throwing')
.it('should not warn about unknown elements with CUSTOM_ELEMENTS_SCHEMA', () => {
@Component({template: `<custom-el></custom-el>`})
class MyComp {
}
const spy = spyOn(console, 'warn');
TestBed.configureTestingModule({
declarations: [MyComp],
schemas: [CUSTOM_ELEMENTS_SCHEMA],
});
const fixture = TestBed.createComponent(MyComp);
fixture.detectChanges();
expect(spy).not.toHaveBeenCalled();
});
modifiedInIvy('unknown element error thrown instead of warning')
.it('should not throw unknown element error with CUSTOM_ELEMENTS_SCHEMA', () => {
@Component({template: `<custom-el></custom-el>`})
class MyComp {
}
TestBed.configureTestingModule({
declarations: [MyComp],
schemas: [CUSTOM_ELEMENTS_SCHEMA],
});
expect(() => { expect(() => {
const fixture = TestBed.createComponent(MyComp); const fixture = TestBed.createComponent(MyComp);
fixture.detectChanges(); fixture.detectChanges();
}).not.toThrow(); }).not.toThrow();
}); });
it('should not throw unknown element error with CUSTOM_ELEMENTS_SCHEMA', () => { onlyInIvy('unknown element check logs a warning rather than throwing')
@Component({template: `<custom-el></custom-el>`}) .it('should not warn about unknown elements with NO_ERRORS_SCHEMA', () => {
class MyComp { @Component({template: `<custom-el></custom-el>`})
} class MyComp {
}
TestBed.configureTestingModule({ const spy = spyOn(console, 'warn');
declarations: [MyComp], TestBed.configureTestingModule({
schemas: [CUSTOM_ELEMENTS_SCHEMA], declarations: [MyComp],
}); schemas: [NO_ERRORS_SCHEMA],
});
expect(() => { const fixture = TestBed.createComponent(MyComp);
const fixture = TestBed.createComponent(MyComp); fixture.detectChanges();
fixture.detectChanges(); expect(spy).not.toHaveBeenCalled();
}).not.toThrow(); });
});
it('should not throw unknown element error with NO_ERRORS_SCHEMA', () => { modifiedInIvy('unknown element error thrown instead of warning')
@Component({template: `<custom-el></custom-el>`}) .it('should not throw unknown element error with NO_ERRORS_SCHEMA', () => {
class MyComp { @Component({template: `<custom-el></custom-el>`})
} class MyComp {
}
TestBed.configureTestingModule({ TestBed.configureTestingModule({
declarations: [MyComp], declarations: [MyComp],
schemas: [NO_ERRORS_SCHEMA], schemas: [NO_ERRORS_SCHEMA],
}); });
expect(() => { expect(() => {
const fixture = TestBed.createComponent(MyComp); const fixture = TestBed.createComponent(MyComp);
fixture.detectChanges(); fixture.detectChanges();
}).not.toThrow(); }).not.toThrow();
}); });
it('should not throw unknown element error if element matches a directive', () => { onlyInIvy('unknown element check logs a warning rather than throwing')
@Component({ .it('should not warn about unknown elements if element matches a directive', () => {
selector: 'custom-el', @Component({
template: '', selector: 'custom-el',
}) template: '',
class CustomEl { })
} class CustomEl {
}
@Component({template: `<custom-el></custom-el>`}) @Component({template: `<custom-el></custom-el>`})
class MyComp { class MyComp {
} }
TestBed.configureTestingModule({declarations: [MyComp, CustomEl]}); const spy = spyOn(console, 'warn');
TestBed.configureTestingModule({declarations: [MyComp, CustomEl]});
expect(() => { const fixture = TestBed.createComponent(MyComp);
const fixture = TestBed.createComponent(MyComp); fixture.detectChanges();
fixture.detectChanges(); expect(spy).not.toHaveBeenCalled();
}).not.toThrow(); });
});
it('should not throw for HTML elements inside an SVG foreignObject', () => { modifiedInIvy('unknown element error thrown instead of warning')
@Component({ .it('should not throw unknown element error if element matches a directive', () => {
template: ` @Component({
selector: 'custom-el',
template: '',
})
class CustomEl {
}
@Component({template: `<custom-el></custom-el>`})
class MyComp {
}
TestBed.configureTestingModule({declarations: [MyComp, CustomEl]});
expect(() => {
const fixture = TestBed.createComponent(MyComp);
fixture.detectChanges();
}).not.toThrow();
});
onlyInIvy('unknown element check logs a warning rather than throwing')
.it('should not warn for HTML elements inside an SVG foreignObject', () => {
@Component({
template: `
<svg> <svg>
<svg:foreignObject> <svg:foreignObject>
<xhtml:div>Hello</xhtml:div> <xhtml:div>Hello</xhtml:div>
</svg:foreignObject> </svg:foreignObject>
</svg> </svg>
`, `,
}) })
class MyComp { class MyComp {
} }
@NgModule({declarations: [MyComp]}) @NgModule({declarations: [MyComp]})
class MyModule { class MyModule {
} }
TestBed.configureTestingModule({imports: [MyModule]}); const spy = spyOn(console, 'warn');
TestBed.configureTestingModule({imports: [MyModule]});
expect(() => { const fixture = TestBed.createComponent(MyComp);
const fixture = TestBed.createComponent(MyComp); fixture.detectChanges();
fixture.detectChanges(); expect(spy).not.toHaveBeenCalled();
}).not.toThrow(); });
});
modifiedInIvy('unknown element error thrown instead of warning')
.it('should not throw for HTML elements inside an SVG foreignObject', () => {
@Component({
template: `
<svg>
<svg:foreignObject>
<xhtml:div>Hello</xhtml:div>
</svg:foreignObject>
</svg>
`,
})
class MyComp {
}
@NgModule({declarations: [MyComp]})
class MyModule {
}
TestBed.configureTestingModule({imports: [MyModule]});
expect(() => {
const fixture = TestBed.createComponent(MyComp);
fixture.detectChanges();
}).not.toThrow();
});
}); });
}); });