feat(compiler): emit diagnostic for shadow dom components with an invalid selector (#42245)
This is based on a discussion we had a few weeks ago. Currently if a component uses `ViewEncapsulation.ShadowDom` and its selector doesn't meet the requirements for a custom element tag name, a vague error will be thrown at runtime saying something like "Element does not support attachShadowRoot". These changes add a new diagnostic to the compiler that validates the component selector and gives a better error message during compilation. PR Close #42245
This commit is contained in:
parent
cc904b5226
commit
afd68e5674
|
@ -0,0 +1,32 @@
|
||||||
|
@name Invalid Shadow DOM selector
|
||||||
|
@category compiler
|
||||||
|
@shortDescription Component selector does not match shadow DOM requirements
|
||||||
|
|
||||||
|
@description
|
||||||
|
The selector of a component using `ViewEncapsulation.ShadowDom` doesn't match the custom element tag name requirements.
|
||||||
|
|
||||||
|
In order for a tag name to be considered a valid custom element name, it has to:
|
||||||
|
* Be in lower case.
|
||||||
|
* Contain a hyphen.
|
||||||
|
* Start with a letter (a-z).
|
||||||
|
|
||||||
|
@debugging
|
||||||
|
Rename your component's selector so that it matches the requirements.
|
||||||
|
|
||||||
|
**Before:**
|
||||||
|
```typescript
|
||||||
|
@Component({
|
||||||
|
selector: 'comp',
|
||||||
|
encapsulation: ViewEncapsulation.ShadowDom
|
||||||
|
...
|
||||||
|
})
|
||||||
|
```
|
||||||
|
|
||||||
|
**After:**
|
||||||
|
```typescript
|
||||||
|
@Component({
|
||||||
|
selector: 'app-comp',
|
||||||
|
encapsulation: ViewEncapsulation.ShadowDom
|
||||||
|
...
|
||||||
|
})
|
||||||
|
```
|
|
@ -15,6 +15,7 @@ export declare enum ErrorCode {
|
||||||
DIRECTIVE_INHERITS_UNDECORATED_CTOR = 2006,
|
DIRECTIVE_INHERITS_UNDECORATED_CTOR = 2006,
|
||||||
UNDECORATED_CLASS_USING_ANGULAR_FEATURES = 2007,
|
UNDECORATED_CLASS_USING_ANGULAR_FEATURES = 2007,
|
||||||
COMPONENT_RESOURCE_NOT_FOUND = 2008,
|
COMPONENT_RESOURCE_NOT_FOUND = 2008,
|
||||||
|
COMPONENT_INVALID_SHADOW_DOM_SELECTOR = 2009,
|
||||||
SYMBOL_NOT_EXPORTED = 3001,
|
SYMBOL_NOT_EXPORTED = 3001,
|
||||||
SYMBOL_EXPORTED_UNDER_DIFFERENT_NAME = 3002,
|
SYMBOL_EXPORTED_UNDER_DIFFERENT_NAME = 3002,
|
||||||
IMPORT_CYCLE_DETECTED = 3003,
|
IMPORT_CYCLE_DETECTED = 3003,
|
||||||
|
|
|
@ -7,10 +7,11 @@
|
||||||
*/
|
*/
|
||||||
|
|
||||||
import {compileClassMetadata, compileComponentFromMetadata, compileDeclareClassMetadata, compileDeclareComponentFromMetadata, ConstantPool, CssSelector, DeclarationListEmitMode, DeclareComponentTemplateInfo, DEFAULT_INTERPOLATION_CONFIG, DomElementSchemaRegistry, Expression, ExternalExpr, FactoryTarget, InterpolationConfig, LexerRange, makeBindingParser, ParsedTemplate, ParseSourceFile, parseTemplate, R3ClassMetadata, R3ComponentMetadata, R3TargetBinder, R3UsedDirectiveMetadata, SelectorMatcher, Statement, TmplAstNode, WrappedNodeExpr} from '@angular/compiler';
|
import {compileClassMetadata, compileComponentFromMetadata, compileDeclareClassMetadata, compileDeclareComponentFromMetadata, ConstantPool, CssSelector, DeclarationListEmitMode, DeclareComponentTemplateInfo, DEFAULT_INTERPOLATION_CONFIG, DomElementSchemaRegistry, Expression, ExternalExpr, FactoryTarget, InterpolationConfig, LexerRange, makeBindingParser, ParsedTemplate, ParseSourceFile, parseTemplate, R3ClassMetadata, R3ComponentMetadata, R3TargetBinder, R3UsedDirectiveMetadata, SelectorMatcher, Statement, TmplAstNode, WrappedNodeExpr} from '@angular/compiler';
|
||||||
|
import {ViewEncapsulation} from '@angular/compiler/src/core';
|
||||||
import * as ts from 'typescript';
|
import * as ts from 'typescript';
|
||||||
|
|
||||||
import {Cycle, CycleAnalyzer, CycleHandlingStrategy} from '../../cycles';
|
import {Cycle, CycleAnalyzer, CycleHandlingStrategy} from '../../cycles';
|
||||||
import {ErrorCode, FatalDiagnosticError, makeRelatedInformation} from '../../diagnostics';
|
import {ErrorCode, FatalDiagnosticError, makeDiagnostic, makeRelatedInformation} from '../../diagnostics';
|
||||||
import {absoluteFrom, relative} from '../../file_system';
|
import {absoluteFrom, relative} from '../../file_system';
|
||||||
import {ImportedFile, ModuleResolver, Reference, ReferenceEmitter} from '../../imports';
|
import {ImportedFile, ModuleResolver, Reference, ReferenceEmitter} from '../../imports';
|
||||||
import {DependencyTracker} from '../../incremental/api';
|
import {DependencyTracker} from '../../incremental/api';
|
||||||
|
@ -338,6 +339,16 @@ export class ComponentDecoratorHandler implements
|
||||||
|
|
||||||
// Next, read the `@Component`-specific fields.
|
// Next, read the `@Component`-specific fields.
|
||||||
const {decorator: component, metadata, inputs, outputs} = directiveResult;
|
const {decorator: component, metadata, inputs, outputs} = directiveResult;
|
||||||
|
const encapsulation: number =
|
||||||
|
this._resolveEnumValue(component, 'encapsulation', 'ViewEncapsulation') ??
|
||||||
|
ViewEncapsulation.Emulated;
|
||||||
|
const changeDetection: number|null =
|
||||||
|
this._resolveEnumValue(component, 'changeDetection', 'ChangeDetectionStrategy');
|
||||||
|
|
||||||
|
let animations: Expression|null = null;
|
||||||
|
if (component.has('animations')) {
|
||||||
|
animations = new WrappedNodeExpr(component.get('animations')!);
|
||||||
|
}
|
||||||
|
|
||||||
// Go through the root directories for this project, and select the one with the smallest
|
// Go through the root directories for this project, and select the one with the smallest
|
||||||
// relative path representation.
|
// relative path representation.
|
||||||
|
@ -426,6 +437,18 @@ export class ComponentDecoratorHandler implements
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (encapsulation === ViewEncapsulation.ShadowDom && metadata.selector !== null) {
|
||||||
|
const selectorError = checkCustomElementSelectorForErrors(metadata.selector);
|
||||||
|
if (selectorError !== null) {
|
||||||
|
if (diagnostics === undefined) {
|
||||||
|
diagnostics = [];
|
||||||
|
}
|
||||||
|
diagnostics.push(makeDiagnostic(
|
||||||
|
ErrorCode.COMPONENT_INVALID_SHADOW_DOM_SELECTOR, component.get('selector')!,
|
||||||
|
selectorError));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// If inline styles were preprocessed use those
|
// If inline styles were preprocessed use those
|
||||||
let inlineStyles: string[]|null = null;
|
let inlineStyles: string[]|null = null;
|
||||||
if (this.preanalyzeStylesCache.has(node)) {
|
if (this.preanalyzeStylesCache.has(node)) {
|
||||||
|
@ -455,17 +478,6 @@ export class ComponentDecoratorHandler implements
|
||||||
styles.push(...template.styles);
|
styles.push(...template.styles);
|
||||||
}
|
}
|
||||||
|
|
||||||
const encapsulation: number =
|
|
||||||
this._resolveEnumValue(component, 'encapsulation', 'ViewEncapsulation') || 0;
|
|
||||||
|
|
||||||
const changeDetection: number|null =
|
|
||||||
this._resolveEnumValue(component, 'changeDetection', 'ChangeDetectionStrategy');
|
|
||||||
|
|
||||||
let animations: Expression|null = null;
|
|
||||||
if (component.has('animations')) {
|
|
||||||
animations = new WrappedNodeExpr(component.get('animations')!);
|
|
||||||
}
|
|
||||||
|
|
||||||
const output: AnalysisOutput<ComponentAnalysisData> = {
|
const output: AnalysisOutput<ComponentAnalysisData> = {
|
||||||
analysis: {
|
analysis: {
|
||||||
baseClass: readBaseClass(node, this.reflector, this.evaluator),
|
baseClass: readBaseClass(node, this.reflector, this.evaluator),
|
||||||
|
@ -1431,3 +1443,31 @@ function makeCyclicImportInfo(
|
||||||
`The ${type} '${name}' is used in the template but importing it would create a cycle: `;
|
`The ${type} '${name}' is used in the template but importing it would create a cycle: `;
|
||||||
return makeRelatedInformation(ref.node, message + path);
|
return makeRelatedInformation(ref.node, message + path);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Checks whether a selector is a valid custom element tag name.
|
||||||
|
* Based loosely on https://github.com/sindresorhus/validate-element-name.
|
||||||
|
*/
|
||||||
|
function checkCustomElementSelectorForErrors(selector: string): string|null {
|
||||||
|
// Avoid flagging components with an attribute or class selector. This isn't bulletproof since it
|
||||||
|
// won't catch cases like `foo[]bar`, but we don't need it to be. This is mainly to avoid flagging
|
||||||
|
// something like `foo-bar[baz]` incorrectly.
|
||||||
|
if (selector.includes('.') || (selector.includes('[') && selector.includes(']'))) {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (!(/^[a-z]/.test(selector))) {
|
||||||
|
return 'Selector of a ShadowDom-encapsulated component must start with a lower case letter.';
|
||||||
|
}
|
||||||
|
|
||||||
|
if (/[A-Z]/.test(selector)) {
|
||||||
|
return 'Selector of a ShadowDom-encapsulated component must all be in lower case.';
|
||||||
|
}
|
||||||
|
|
||||||
|
if (!selector.includes('-')) {
|
||||||
|
return 'Selector of a component that uses ViewEncapsulation.ShadowDom must contain a hyphen.';
|
||||||
|
}
|
||||||
|
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
|
@ -50,6 +50,12 @@ export enum ErrorCode {
|
||||||
*/
|
*/
|
||||||
COMPONENT_RESOURCE_NOT_FOUND = 2008,
|
COMPONENT_RESOURCE_NOT_FOUND = 2008,
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Raised when a component uses `ShadowDom` view encapsulation, but its selector
|
||||||
|
* does not match the shadow DOM tag name requirements.
|
||||||
|
*/
|
||||||
|
COMPONENT_INVALID_SHADOW_DOM_SELECTOR = 2009,
|
||||||
|
|
||||||
SYMBOL_NOT_EXPORTED = 3001,
|
SYMBOL_NOT_EXPORTED = 3001,
|
||||||
SYMBOL_EXPORTED_UNDER_DIFFERENT_NAME = 3002,
|
SYMBOL_EXPORTED_UNDER_DIFFERENT_NAME = 3002,
|
||||||
/**
|
/**
|
||||||
|
@ -215,6 +221,7 @@ export const COMPILER_ERRORS_WITH_GUIDES = new Set([
|
||||||
ErrorCode.SCHEMA_INVALID_ELEMENT,
|
ErrorCode.SCHEMA_INVALID_ELEMENT,
|
||||||
ErrorCode.SCHEMA_INVALID_ATTRIBUTE,
|
ErrorCode.SCHEMA_INVALID_ATTRIBUTE,
|
||||||
ErrorCode.MISSING_REFERENCE_TARGET,
|
ErrorCode.MISSING_REFERENCE_TARGET,
|
||||||
|
ErrorCode.COMPONENT_INVALID_SHADOW_DOM_SELECTOR,
|
||||||
]);
|
]);
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|
|
@ -7864,6 +7864,111 @@ export const Foo = Foo__PRE_R3__;
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('shadow DOM selector diagnostics', () => {
|
||||||
|
it('should emit a diagnostic when a selector does not include a hyphen', () => {
|
||||||
|
env.write('test.ts', `
|
||||||
|
import {Component, ViewEncapsulation} from '@angular/core';
|
||||||
|
@Component({
|
||||||
|
template: '',
|
||||||
|
selector: 'cmp',
|
||||||
|
encapsulation: ViewEncapsulation.ShadowDom
|
||||||
|
})
|
||||||
|
export class TestCmp {}
|
||||||
|
`);
|
||||||
|
const diags = env.driveDiagnostics();
|
||||||
|
|
||||||
|
expect(diags.length).toBe(1);
|
||||||
|
expect(diags[0].messageText)
|
||||||
|
.toBe(
|
||||||
|
'Selector of a component that uses ViewEncapsulation.ShadowDom must contain a hyphen.');
|
||||||
|
expect(getDiagnosticSourceCode(diags[0])).toBe(`'cmp'`);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should emit a diagnostic when a selector includes uppercase letters', () => {
|
||||||
|
env.write('test.ts', `
|
||||||
|
import {Component, ViewEncapsulation} from '@angular/core';
|
||||||
|
@Component({
|
||||||
|
template: '',
|
||||||
|
selector: 'my-Comp',
|
||||||
|
encapsulation: ViewEncapsulation.ShadowDom
|
||||||
|
})
|
||||||
|
export class TestCmp {}
|
||||||
|
`);
|
||||||
|
const diags = env.driveDiagnostics();
|
||||||
|
|
||||||
|
expect(diags.length).toBe(1);
|
||||||
|
expect(diags[0].messageText)
|
||||||
|
.toBe('Selector of a ShadowDom-encapsulated component must all be in lower case.');
|
||||||
|
expect(getDiagnosticSourceCode(diags[0])).toBe(`'my-Comp'`);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should emit a diagnostic when a selector starts with a digit', () => {
|
||||||
|
env.write('test.ts', `
|
||||||
|
import {Component, ViewEncapsulation} from '@angular/core';
|
||||||
|
@Component({
|
||||||
|
template: '',
|
||||||
|
selector: '123-comp',
|
||||||
|
encapsulation: ViewEncapsulation.ShadowDom
|
||||||
|
})
|
||||||
|
export class TestCmp {}
|
||||||
|
`);
|
||||||
|
const diags = env.driveDiagnostics();
|
||||||
|
|
||||||
|
expect(diags.length).toBe(1);
|
||||||
|
expect(diags[0].messageText)
|
||||||
|
.toBe(
|
||||||
|
'Selector of a ShadowDom-encapsulated component must start with a lower case letter.');
|
||||||
|
expect(getDiagnosticSourceCode(diags[0])).toBe(`'123-comp'`);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should emit a diagnostic when a selector starts with a hyphen', () => {
|
||||||
|
env.write('test.ts', `
|
||||||
|
import {Component, ViewEncapsulation} from '@angular/core';
|
||||||
|
@Component({
|
||||||
|
template: '',
|
||||||
|
selector: '-comp',
|
||||||
|
encapsulation: ViewEncapsulation.ShadowDom
|
||||||
|
})
|
||||||
|
export class TestCmp {}
|
||||||
|
`);
|
||||||
|
const diags = env.driveDiagnostics();
|
||||||
|
|
||||||
|
expect(diags.length).toBe(1);
|
||||||
|
expect(diags[0].messageText)
|
||||||
|
.toBe(
|
||||||
|
'Selector of a ShadowDom-encapsulated component must start with a lower case letter.');
|
||||||
|
expect(getDiagnosticSourceCode(diags[0])).toBe(`'-comp'`);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should not emit a diagnostic for a component using an attribute selector', () => {
|
||||||
|
env.write('test.ts', `
|
||||||
|
import {Component, ViewEncapsulation} from '@angular/core';
|
||||||
|
@Component({
|
||||||
|
template: '',
|
||||||
|
selector: '[button]',
|
||||||
|
encapsulation: ViewEncapsulation.ShadowDom
|
||||||
|
})
|
||||||
|
export class TestCmp {}
|
||||||
|
`);
|
||||||
|
const diags = env.driveDiagnostics();
|
||||||
|
expect(diags.length).toBe(0);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should not emit a diagnostic for a component using a class selector', () => {
|
||||||
|
env.write('test.ts', `
|
||||||
|
import {Component, ViewEncapsulation} from '@angular/core';
|
||||||
|
@Component({
|
||||||
|
template: '',
|
||||||
|
selector: '.button',
|
||||||
|
encapsulation: ViewEncapsulation.ShadowDom
|
||||||
|
})
|
||||||
|
export class TestCmp {}
|
||||||
|
`);
|
||||||
|
const diags = env.driveDiagnostics();
|
||||||
|
expect(diags.length).toBe(0);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
describe('i18n errors', () => {
|
describe('i18n errors', () => {
|
||||||
it('reports a diagnostics on nested i18n sections', () => {
|
it('reports a diagnostics on nested i18n sections', () => {
|
||||||
env.write('test.ts', `
|
env.write('test.ts', `
|
||||||
|
|
Loading…
Reference in New Issue