Skip to content

Commit

Permalink
feat(core): allow to throw on unknown elements in tests (#45479)
Browse files Browse the repository at this point in the history
Allows to provide a TestBed option to throw on unknown elements in templates:

```ts
getTestBed().initTestEnvironment(
  BrowserDynamicTestingModule,
  platformBrowserDynamicTesting(), {
    errorOnUnknownElements: true
  }
);
```

The default value of `errorOnUnknownElements` is `false`, so this is not a breaking change.

PR Close #45479
  • Loading branch information
cexbrayat authored and dylhunn committed May 2, 2022
1 parent 6a3ca0e commit e702caf
Show file tree
Hide file tree
Showing 8 changed files with 231 additions and 8 deletions.
2 changes: 2 additions & 0 deletions goldens/public-api/core/testing/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ export class TestComponentRenderer {

// @public (undocumented)
export interface TestEnvironmentOptions {
errorOnUnknownElements?: boolean;
teardown?: ModuleTeardownOptions;
}

Expand All @@ -225,6 +226,7 @@ export type TestModuleMetadata = {
imports?: any[];
schemas?: Array<SchemaMetadata | any[]>;
teardown?: ModuleTeardownOptions;
errorOnUnknownElements?: boolean;
};

// @public
Expand Down
2 changes: 2 additions & 0 deletions packages/core/src/core_render3_private_export.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,8 @@ export {
ɵɵtextInterpolate8,
ɵɵtextInterpolateV,
ɵɵviewQuery,
ɵgetUnknownElementStrictMode,
ɵsetUnknownElementStrictMode
} from './render3/index';
export {
LContext as ɵLContext,
Expand Down
2 changes: 2 additions & 0 deletions packages/core/src/render3/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,8 @@ export {
ɵɵtextInterpolate7,
ɵɵtextInterpolate8,
ɵɵtextInterpolateV,
ɵgetUnknownElementStrictMode,
ɵsetUnknownElementStrictMode
} from './instructions/all';
export {ɵɵi18n, ɵɵi18nApply, ɵɵi18nAttributes, ɵɵi18nEnd, ɵɵi18nExp,ɵɵi18nPostprocess, ɵɵi18nStart} from './instructions/i18n';
export {RenderFlags} from './interfaces/definition';
Expand Down
24 changes: 22 additions & 2 deletions packages/core/src/render3/instructions/element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {formatRuntimeError, RuntimeErrorCode} from '../../errors';
import {formatRuntimeError, RuntimeError, RuntimeErrorCode} from '../../errors';
import {SchemaMetadata} from '../../metadata/schema';
import {assertDefined, assertEqual, assertIndexInRange} from '../../util/assert';
import {assertFirstCreatePass, assertHasParent} from '../assert';
Expand All @@ -26,7 +26,23 @@ import {getConstant} from '../util/view_utils';
import {setDirectiveInputsWhichShadowsStyling} from './property';
import {createDirectivesInstances, executeContentQueries, getOrCreateTNode, matchingSchemas, resolveDirectives, saveResolvedLocalsInData} from './shared';

let shouldThrowErrorOnUnknownElement = false;

/**
* Sets a strict mode for JIT-compiled components to throw an error on unknown elements,
* instead of just logging the error.
* (for AOT-compiled ones this check happens at build time).
*/
export function ɵsetUnknownElementStrictMode(shouldThrow: boolean) {
shouldThrowErrorOnUnknownElement = shouldThrow;
}

/**
* Gets the current value of the strict mode.
*/
export function ɵgetUnknownElementStrictMode() {
return shouldThrowErrorOnUnknownElement;
}

function elementStartFirstCreatePass(
index: number, tView: TView, lView: LView, native: RElement, name: string,
Expand Down Expand Up @@ -241,7 +257,11 @@ function validateElementIsKnown(
message +=
`2. To allow any element add 'NO_ERRORS_SCHEMA' to the '@NgModule.schemas' of this component.`;
}
console.error(formatRuntimeError(RuntimeErrorCode.UNKNOWN_ELEMENT, message));
if (shouldThrowErrorOnUnknownElement) {
throw new RuntimeError(RuntimeErrorCode.UNKNOWN_ELEMENT, message);
} else {
console.error(formatRuntimeError(RuntimeErrorCode.UNKNOWN_ELEMENT, message));
}
}
}
}
111 changes: 111 additions & 0 deletions packages/core/test/acceptance/ng_module_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,19 @@ describe('NgModule', () => {
expect(spy).not.toHaveBeenCalled();
});

it('should throw an error 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], errorOnUnknownElements: true});
expect(() => {
const fixture = TestBed.createComponent(MyComp);
fixture.detectChanges();
}).toThrowError(/NG0304: 'custom-el' is not a known element/g);
});

it('should log an error about unknown element without CUSTOM_ELEMENTS_SCHEMA for element without dash in tag name',
() => {
@Component({template: `<custom></custom>`})
Expand All @@ -344,6 +357,19 @@ describe('NgModule', () => {
expect(spy.calls.mostRecent().args[0]).toMatch(/'custom' is not a known element/);
});

it('should throw an error about unknown element without CUSTOM_ELEMENTS_SCHEMA for element without dash in tag name',
() => {
@Component({template: `<custom></custom>`})
class MyComp {
}

TestBed.configureTestingModule({declarations: [MyComp], errorOnUnknownElements: true});
expect(() => {
const fixture = TestBed.createComponent(MyComp);
fixture.detectChanges();
}).toThrowError(/NG0304: 'custom' is not a known element/g);
});

it('should report unknown property bindings on ng-content', () => {
@Component({template: `<ng-content *unknownProp="123"></ng-content>`})
class App {
Expand Down Expand Up @@ -505,6 +531,25 @@ describe('NgModule', () => {
expect(spy).not.toHaveBeenCalled();
});

it('should not throw an error about unknown elements with CUSTOM_ELEMENTS_SCHEMA', () => {
@Component({template: `<custom-el></custom-el>`})
class MyComp {
}

const spy = spyOn(console, 'error');
TestBed.configureTestingModule({
declarations: [MyComp],
schemas: [CUSTOM_ELEMENTS_SCHEMA],
errorOnUnknownElements: true
});

const fixture = TestBed.createComponent(MyComp);
fixture.detectChanges();
// We do not expect any errors being thrown or logged in a console,
// since the `CUSTOM_ELEMENTS_SCHEMA` is applied.
expect(spy).not.toHaveBeenCalled();
});

it('should not log an error about unknown elements with NO_ERRORS_SCHEMA', () => {
@Component({template: `<custom-el></custom-el>`})
class MyComp {
Expand All @@ -521,6 +566,22 @@ describe('NgModule', () => {
expect(spy).not.toHaveBeenCalled();
});

it('should not throw an error about unknown elements with NO_ERRORS_SCHEMA', () => {
@Component({template: `<custom-el></custom-el>`})
class MyComp {
}

const spy = spyOn(console, 'error');
TestBed.configureTestingModule(
{declarations: [MyComp], schemas: [NO_ERRORS_SCHEMA], errorOnUnknownElements: true});

const fixture = TestBed.createComponent(MyComp);
fixture.detectChanges();
// We do not expect any errors being thrown or logged in a console,
// since the `NO_ERRORS_SCHEMA` is applied.
expect(spy).not.toHaveBeenCalled();
});

it('should not log an error about unknown elements if element matches a directive', () => {
@Component({
selector: 'custom-el',
Expand All @@ -541,6 +602,29 @@ describe('NgModule', () => {
expect(spy).not.toHaveBeenCalled();
});

it('should not throw an error about unknown elements if element matches a directive', () => {
@Component({
selector: 'custom-el',
template: '',
})
class CustomEl {
}

@Component({template: `<custom-el></custom-el>`})
class MyComp {
}

const spy = spyOn(console, 'error');
TestBed.configureTestingModule(
{declarations: [MyComp, CustomEl], errorOnUnknownElements: true});

const fixture = TestBed.createComponent(MyComp);
fixture.detectChanges();
// We do not expect any errors being thrown or logged in a console,
// since the element matches a directive.
expect(spy).not.toHaveBeenCalled();
});

it('should not log an error for HTML elements inside an SVG foreignObject', () => {
@Component({
template: `
Expand All @@ -565,6 +649,33 @@ describe('NgModule', () => {
fixture.detectChanges();
expect(spy).not.toHaveBeenCalled();
});

it('should not throw an error 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 {
}

const spy = spyOn(console, 'error');
TestBed.configureTestingModule({imports: [MyModule], errorOnUnknownElements: true});

const fixture = TestBed.createComponent(MyComp);
fixture.detectChanges();
// We do not expect any errors being thrown or logged in a console,
// since the element is inside an SVG foreignObject.
expect(spy).not.toHaveBeenCalled();
});
});

describe('createNgModuleRef function', () => {
Expand Down
33 changes: 32 additions & 1 deletion packages/core/test/test_bed_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {expect} from '@angular/platform-browser/testing/src/matchers';

import {getNgModuleById} from '../public_api';
import {TestBedRender3} from '../testing/src/r3_test_bed';
import {TEARDOWN_TESTING_MODULE_ON_DESTROY_DEFAULT} from '../testing/src/test_bed_common';
import {TEARDOWN_TESTING_MODULE_ON_DESTROY_DEFAULT, THROW_ON_UNKNOWN_ELEMENTS_DEFAULT} from '../testing/src/test_bed_common';

const NAME = new InjectionToken<string>('name');

Expand Down Expand Up @@ -1714,3 +1714,34 @@ describe('TestBed module teardown', () => {
expect(TestBed.shouldRethrowTeardownErrors()).toBe(false);
});
});

describe('TestBed module `errorOnUnknownElements`', () => {
// Cast the `TestBed` to the internal data type since we're testing private APIs.
let TestBed: TestBedRender3;

beforeEach(() => {
TestBed = getTestBed() as unknown as TestBedRender3;
TestBed.resetTestingModule();
});

it('should not throw based on the default behavior', () => {
expect(TestBed.shouldThrowErrorOnUnknownElements()).toBe(THROW_ON_UNKNOWN_ELEMENTS_DEFAULT);
});

it('should not throw if the option is omitted', () => {
TestBed.configureTestingModule({});
expect(TestBed.shouldThrowErrorOnUnknownElements()).toBe(false);
});

it('should be able to configure the option', () => {
TestBed.configureTestingModule({errorOnUnknownElements: true});
expect(TestBed.shouldThrowErrorOnUnknownElements()).toBe(true);
});

it('should reset the option back to the default when TestBed is reset', () => {
TestBed.configureTestingModule({errorOnUnknownElements: true});
expect(TestBed.shouldThrowErrorOnUnknownElements()).toBe(true);
TestBed.resetTestingModule();
expect(TestBed.shouldThrowErrorOnUnknownElements()).toBe(false);
});
});
48 changes: 43 additions & 5 deletions packages/core/testing/src/r3_test_bed.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,12 @@ import {
ProviderToken,
Type,
ɵflushModuleScopingQueueAsMuchAsPossible as flushModuleScopingQueueAsMuchAsPossible,
ɵgetUnknownElementStrictMode as getUnknownElementStrictMode,
ɵRender3ComponentFactory as ComponentFactory,
ɵRender3NgModuleRef as NgModuleRef,
ɵresetCompiledComponents as resetCompiledComponents,
ɵsetAllowDuplicateNgModuleIdsForTest as setAllowDuplicateNgModuleIdsForTest,
ɵsetUnknownElementStrictMode as setUnknownElementStrictMode,
ɵstringify as stringify,
} from '@angular/core';

Expand All @@ -37,7 +39,7 @@ import {ComponentFixture} from './component_fixture';
import {MetadataOverride} from './metadata_override';
import {R3TestBedCompiler} from './r3_test_bed_compiler';
import {TestBed} from './test_bed';
import {ComponentFixtureAutoDetect, ComponentFixtureNoNgZone, ModuleTeardownOptions, TEARDOWN_TESTING_MODULE_ON_DESTROY_DEFAULT, TestBedStatic, TestComponentRenderer, TestEnvironmentOptions, TestModuleMetadata} from './test_bed_common';
import {ComponentFixtureAutoDetect, ComponentFixtureNoNgZone, ModuleTeardownOptions, TEARDOWN_TESTING_MODULE_ON_DESTROY_DEFAULT, TestBedStatic, TestComponentRenderer, TestEnvironmentOptions, TestModuleMetadata, THROW_ON_UNKNOWN_ELEMENTS_DEFAULT} from './test_bed_common';

let _nextRootElementId = 0;

Expand All @@ -59,12 +61,30 @@ export class TestBedRender3 implements TestBed {
*/
private static _environmentTeardownOptions: ModuleTeardownOptions|undefined;

/**
* "Error on unknown elements" option that has been configured at the environment level.
* Used as a fallback if no instance-level option has been provided.
*/
private static _environmentErrorOnUnknownElementsOption: boolean|undefined;

/**
* Teardown options that have been configured at the `TestBed` instance level.
* These options take precedence over the environemnt-level ones.
* These options take precedence over the environment-level ones.
*/
private _instanceTeardownOptions: ModuleTeardownOptions|undefined;

/**
* "Error on unknown elements" option that has been configured at the `TestBed` instance level.
* This option takes precedence over the environment-level one.
*/
private _instanceErrorOnUnknownElementsOption: boolean|undefined;

/**
* Stores the previous "Error on unknown elements" option value,
* allowing to restore it in the reset testing module logic.
*/
private _previousErrorOnUnknownElementsOption: boolean|undefined;

/**
* Initialize the environment for testing with a compiler factory, a PlatformRef, and an
* angular module. These are common to every test in the suite.
Expand Down Expand Up @@ -237,6 +257,8 @@ export class TestBedRender3 implements TestBed {

TestBedRender3._environmentTeardownOptions = options?.teardown;

TestBedRender3._environmentErrorOnUnknownElementsOption = options?.errorOnUnknownElements;

this.platform = platform;
this.ngModule = ngModule;
this._compiler = new R3TestBedCompiler(this.platform, this.ngModule);
Expand Down Expand Up @@ -269,6 +291,9 @@ export class TestBedRender3 implements TestBed {
this.compiler.restoreOriginalState();
}
this._compiler = new R3TestBedCompiler(this.platform, this.ngModule);
// Restore the previous value of the "error on unknown elements" option
setUnknownElementStrictMode(
this._previousErrorOnUnknownElementsOption ?? THROW_ON_UNKNOWN_ELEMENTS_DEFAULT);

// We have to chain a couple of try/finally blocks, because each step can
// throw errors and we don't want it to interrupt the next step and we also
Expand All @@ -283,6 +308,7 @@ export class TestBedRender3 implements TestBed {
} finally {
this._testModuleRef = null;
this._instanceTeardownOptions = undefined;
this._instanceErrorOnUnknownElementsOption = undefined;
}
}
}
Expand All @@ -306,9 +332,14 @@ export class TestBedRender3 implements TestBed {
// description for additional info.
this.checkGlobalCompilationFinished();

// Always re-assign the teardown options, even if they're undefined.
// This ensures that we don't carry the options between tests.
// Always re-assign the options, even if they're undefined.
// This ensures that we don't carry them between tests.
this._instanceTeardownOptions = moduleDef.teardown;
this._instanceErrorOnUnknownElementsOption = moduleDef.errorOnUnknownElements;
// Store the current value of the strict mode option,
// so we can restore it later
this._previousErrorOnUnknownElementsOption = getUnknownElementStrictMode();
setUnknownElementStrictMode(this.shouldThrowErrorOnUnknownElements());
this.compiler.configureTestingModule(moduleDef);
}

Expand Down Expand Up @@ -481,7 +512,7 @@ export class TestBedRender3 implements TestBed {
}
}

shouldRethrowTeardownErrors() {
shouldRethrowTeardownErrors(): boolean {
const instanceOptions = this._instanceTeardownOptions;
const environmentOptions = TestBedRender3._environmentTeardownOptions;

Expand All @@ -495,6 +526,13 @@ export class TestBedRender3 implements TestBed {
this.shouldTearDownTestingModule();
}

shouldThrowErrorOnUnknownElements(): boolean {
// Check if a configuration has been provided to throw when an unknown element is found
return this._instanceErrorOnUnknownElementsOption ??
TestBedRender3._environmentErrorOnUnknownElementsOption ??
THROW_ON_UNKNOWN_ELEMENTS_DEFAULT;
}

shouldTearDownTestingModule(): boolean {
return this._instanceTeardownOptions?.destroyAfterEach ??
TestBedRender3._environmentTeardownOptions?.destroyAfterEach ??
Expand Down
Loading

0 comments on commit e702caf

Please sign in to comment.