Skip to content

Commit

Permalink
feat(core): support styles and styleUrl as strings (#51715)
Browse files Browse the repository at this point in the history
Adds support for passing in `@Component.styles` as a string. Also introduces a new `styleUrl` property on `@Component` for providing a single stylesheet. This is more convenient for the most common case where a component only has one stylesheet associated with it.

PR Close #51715
  • Loading branch information
crisbeto authored and AndrewKushnir committed Sep 12, 2023
1 parent f993975 commit 59387ee
Show file tree
Hide file tree
Showing 12 changed files with 239 additions and 42 deletions.
1 change: 1 addition & 0 deletions goldens/public-api/compiler-cli/error_code.md
Expand Up @@ -8,6 +8,7 @@
export enum ErrorCode {
COMPONENT_IMPORT_NOT_STANDALONE = 2011,
COMPONENT_INVALID_SHADOW_DOM_SELECTOR = 2009,
COMPONENT_INVALID_STYLE_URLS = 2021,
// (undocumented)
COMPONENT_MISSING_TEMPLATE = 2001,
COMPONENT_NOT_STANDALONE = 2010,
Expand Down
3 changes: 2 additions & 1 deletion goldens/public-api/core/index.md
Expand Up @@ -218,7 +218,8 @@ export interface Component extends Directive {
preserveWhitespaces?: boolean;
schemas?: SchemaMetadata[];
standalone?: boolean;
styles?: string[];
styles?: string | string[];
styleUrl?: string;
styleUrls?: string[];
template?: string;
templateUrl?: string;
Expand Down
Expand Up @@ -452,7 +452,8 @@ export function transformDecoratorResources(

// If no external resources are referenced, preserve the original decorator
// for the best source map experience when the decorator is emitted in TS.
if (!component.has('templateUrl') && !component.has('styleUrls') && !component.has('styles')) {
if (!component.has('templateUrl') && !component.has('styleUrls') && !component.has('styleUrl') &&
!component.has('styles')) {
return dec;
}

Expand All @@ -464,9 +465,10 @@ export function transformDecoratorResources(
metadata.set('template', ts.factory.createStringLiteral(template.content));
}

if (metadata.has('styleUrls') || metadata.has('styles')) {
if (metadata.has('styleUrls') || metadata.has('styleUrl') || metadata.has('styles')) {
metadata.delete('styles');
metadata.delete('styleUrls');
metadata.delete('styleUrl');

if (styles.length > 0) {
const styleNodes = styles.reduce((result, style) => {
Expand Down Expand Up @@ -496,11 +498,35 @@ export function extractComponentStyleUrls(
evaluator: PartialEvaluator,
component: Map<string, ts.Expression>,
): StyleUrlMeta[] {
if (!component.has('styleUrls')) {
return [];
const styleUrlsExpr = component.get('styleUrls');
const styleUrlExpr = component.get('styleUrl');

if (styleUrlsExpr !== undefined && styleUrlExpr !== undefined) {
throw new FatalDiagnosticError(
ErrorCode.COMPONENT_INVALID_STYLE_URLS, styleUrlExpr,
'@Component cannot define both `styleUrl` and `styleUrls`. ' +
'Use `styleUrl` if the component has one stylesheet, or `styleUrls` if it has multiple');
}

return extractStyleUrlsFromExpression(evaluator, component.get('styleUrls')!);
if (styleUrlsExpr !== undefined) {
return extractStyleUrlsFromExpression(evaluator, component.get('styleUrls')!);
}

if (styleUrlExpr !== undefined) {
const styleUrl = evaluator.evaluate(styleUrlExpr);

if (typeof styleUrl !== 'string') {
throw createValueHasWrongTypeError(styleUrlExpr, styleUrl, 'styleUrl must be a string');
}

return [{
url: styleUrl,
source: ResourceTypeForDiagnostics.StylesheetFromDecorator,
nodeForError: styleUrlExpr,
}];
}

return [];
}

function extractStyleUrlsFromExpression(
Expand Down Expand Up @@ -543,42 +569,63 @@ function extractStyleUrlsFromExpression(

return styleUrls;
}

export function extractStyleResources(
resourceLoader: ResourceLoader, component: Map<string, ts.Expression>,
containingFile: string): ReadonlySet<Resource> {
const styles = new Set<Resource>();
function stringLiteralElements(array: ts.ArrayLiteralExpression): ts.StringLiteralLike[] {
return array.elements.filter(
(e: ts.Expression): e is ts.StringLiteralLike => ts.isStringLiteralLike(e));
return array.elements.filter((e): e is ts.StringLiteralLike => ts.isStringLiteralLike(e));
}

// If styleUrls is a literal array, process each resource url individually and
// register ones that are string literals.
// If styleUrls is a literal array, process each resource url individually and register ones that
// are string literals. If `styleUrl` is specified, register a single stylesheet. Note that
// `styleUrl` and `styleUrls` are mutually-exclusive. This is validated in
// `extractComponentStyleUrls`.
const styleUrlExpr = component.get('styleUrl');
const styleUrlsExpr = component.get('styleUrls');
if (styleUrlsExpr !== undefined && ts.isArrayLiteralExpression(styleUrlsExpr)) {
for (const expression of stringLiteralElements(styleUrlsExpr)) {
try {
const resourceUrl = resourceLoader.resolve(expression.text, containingFile);
styles.add({path: absoluteFrom(resourceUrl), expression});
} catch {
// Errors in style resource extraction do not need to be handled here. We will produce
// diagnostics for each one that fails in the analysis, after we evaluate the
// `styleUrls` expression to determine _all_ style resources, not just the string
// literals.
const resource = stringLiteralUrlToResource(resourceLoader, expression, containingFile);
if (resource !== null) {
styles.add(resource);
}
}
} else if (styleUrlExpr !== undefined && ts.isStringLiteralLike(styleUrlExpr)) {
const resource = stringLiteralUrlToResource(resourceLoader, styleUrlExpr, containingFile);
if (resource !== null) {
styles.add(resource);
}
}

const stylesExpr = component.get('styles');
if (stylesExpr !== undefined && ts.isArrayLiteralExpression(stylesExpr)) {
for (const expression of stringLiteralElements(stylesExpr)) {
styles.add({path: null, expression});
if (stylesExpr !== undefined) {
if (ts.isArrayLiteralExpression(stylesExpr)) {
for (const expression of stringLiteralElements(stylesExpr)) {
styles.add({path: null, expression});
}
} else if (ts.isStringLiteralLike(stylesExpr)) {
styles.add({path: null, expression: stylesExpr});
}
}

return styles;
}

function stringLiteralUrlToResource(
resourceLoader: ResourceLoader, expression: ts.StringLiteralLike,
containingFile: string): Resource|null {
try {
const resourceUrl = resourceLoader.resolve(expression.text, containingFile);
return {path: absoluteFrom(resourceUrl), expression};
} catch {
// Errors in style resource extraction do not need to be handled here. We will produce
// diagnostics for each one that fails in the analysis, after we evaluate the `styleUrls`
// expression to determine _all_ style resources, not just the string literals.
return null;
}
}

export function _extractTemplateStyleUrls(template: ParsedTemplateWithSource): StyleUrlMeta[] {
if (template.styleUrls === null) {
return [];
Expand Down
Expand Up @@ -470,7 +470,8 @@ export function parseDirectiveStyles(
return null;
}

const value = evaluator.evaluate(expression);
const evaluated = evaluator.evaluate(expression);
const value = typeof evaluated === 'string' ? [evaluated] : evaluated;

// Create specific error if any string is imported from external file in local compilation mode
if (compilationMode === CompilationMode.LOCAL && Array.isArray(value)) {
Expand All @@ -481,7 +482,7 @@ export function parseDirectiveStyles(
const chain: ts.DiagnosticMessageChain = {
messageText: `Unknown identifier used as styles string: ${
entry.node
.getText()} (did you import this string from another file? This is not allowed in local compilation mode. Please either inline it or move it to a separate file and include it using'styleUrls')`,
.getText()} (did you import this string from another file? This is not allowed in local compilation mode. Please either inline it or move it to a separate file and include it using 'styleUrl')`,
category: ts.DiagnosticCategory.Error,
code: 0,
};
Expand All @@ -495,7 +496,8 @@ export function parseDirectiveStyles(

if (!isStringArrayOrDie(value, 'styles', expression)) {
throw createValueHasWrongTypeError(
expression, value, `Failed to resolve @Directive.styles to a string array`);
expression, value,
`Failed to resolve @Component.styles to a string or an array of strings`);
}

return value;
Expand Down
2 changes: 2 additions & 0 deletions packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts
Expand Up @@ -113,6 +113,8 @@ export enum ErrorCode {
*/
CONFLICTING_INPUT_TRANSFORM = 2020,

/** Raised when a component has both `styleUrls` and `styleUrl`. */
COMPONENT_INVALID_STYLE_URLS = 2021,

SYMBOL_NOT_EXPORTED = 3001,
/**
Expand Down
55 changes: 55 additions & 0 deletions packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
Expand Up @@ -7436,6 +7436,61 @@ function allTests(os: string) {
// Large string should be called from function definition.
'_c0()]');
});

it('should process `styles` as a string', () => {
env.write('test.ts', `
import {Component} from '@angular/core';
@Component({
template: '',
styles: 'h2 {width: 10px}'
})
export class TestCmp {}
`);

env.driveMain();
const jsContents = env.getContents('test.js');
expect(jsContents).toContain('styles: ["h2[_ngcontent-%COMP%] {width: 10px}"]');
});

it('should process `styleUrl`', () => {
env.write('dir/styles.css', 'h2 {width: 10px}');
env.write('test.ts', `
import {Component} from '@angular/core';
@Component({
selector: 'test-cmp',
styleUrl: 'dir/styles.css',
template: '',
})
export class TestCmp {}
`);
env.driveMain();

const jsContents = env.getContents('test.js');
expect(jsContents).not.toContain('styleUrl');
expect(jsContents).toContain('styles: ["h2[_ngcontent-%COMP%] {width: 10px}"]');
});

it('should produce a diagnostic if both `styleUrls` and `styleUrl` are defined', () => {
env.write('dir/styles.css', 'h2 {width: 10px}');
env.write('test.ts', `
import {Component} from '@angular/core';
@Component({
selector: 'test-cmp',
styleUrl: 'dir/styles.css',
styleUrls: ['dir/styles.css'],
template: '',
})
export class TestCmp {}
`);

const diags = env.driveDiagnostics();
expect(diags.length).toBe(1);
expect(diags[0].messageText)
.toContain('@Component cannot define both `styleUrl` and `styleUrls`');
});
});

describe('empty resources', () => {
Expand Down
9 changes: 7 additions & 2 deletions packages/core/src/metadata/directives.ts
Expand Up @@ -574,16 +574,21 @@ export interface Component extends Directive {
template?: string;

/**
* One or more relative paths or absolute URLs for files containing CSS stylesheets to use
* One relative paths or an absolute URL for files containing CSS stylesheet to use
* in this component.
*/
styleUrl?: string;

/**
* Relative paths or absolute URLs for files containing CSS stylesheets to use in this component.
*/
styleUrls?: string[];

/**
* One or more inline CSS stylesheets to use
* in this component.
*/
styles?: string[];
styles?: string|string[];

/**
* One or more animation `trigger()` calls, containing
Expand Down
42 changes: 29 additions & 13 deletions packages/core/src/metadata/resource_loading.ts
Expand Up @@ -7,6 +7,7 @@
*/

import {Type} from '../interface/type';

import {Component} from './directives';


Expand Down Expand Up @@ -65,19 +66,34 @@ export function resolveComponentResources(
component.template = template;
}));
}
const styleUrls = component.styleUrls;
const styles = component.styles || (component.styles = []);
const styleOffset = component.styles.length;
styleUrls && styleUrls.forEach((styleUrl, index) => {
styles.push(''); // pre-allocate array.
promises.push(cachedResourceResolve(styleUrl).then((style) => {
styles[styleOffset + index] = style;
styleUrls.splice(styleUrls.indexOf(styleUrl), 1);
if (styleUrls.length == 0) {
component.styleUrls = undefined;
}
const styles =
typeof component.styles === 'string' ? [component.styles] : (component.styles || []);
component.styles = styles;

if (component.styleUrl && component.styleUrls?.length) {
throw new Error(
'@Component cannot define both `styleUrl` and `styleUrls`. ' +
'Use `styleUrl` if the component has one stylesheet, or `styleUrls` if it has multiple');
} else if (component.styleUrls?.length) {
const styleOffset = component.styles.length;
const styleUrls = component.styleUrls;
component.styleUrls.forEach((styleUrl, index) => {
styles.push(''); // pre-allocate array.
promises.push(cachedResourceResolve(styleUrl).then((style) => {
styles[styleOffset + index] = style;
styleUrls.splice(styleUrls.indexOf(styleUrl), 1);
if (styleUrls.length == 0) {
component.styleUrls = undefined;
}
}));
});
} else if (component.styleUrl) {
promises.push(cachedResourceResolve(component.styleUrl).then((style) => {
styles.push(style);
component.styleUrl = undefined;
}));
});
}

const fullyResolved = Promise.all(promises).then(() => componentDefResolved(type));
componentResolved.push(fullyResolved);
});
Expand All @@ -104,7 +120,7 @@ export function isComponentDefPendingResolution(type: Type<any>): boolean {
export function componentNeedsResolution(component: Component): boolean {
return !!(
(component.templateUrl && !component.hasOwnProperty('template')) ||
component.styleUrls && component.styleUrls.length);
(component.styleUrls && component.styleUrls.length) || component.styleUrl);
}
export function clearResolutionOfComponentResourcesQueue(): Map<Type<any>, Component> {
const old = componentResourceResolutionQueue;
Expand Down
6 changes: 5 additions & 1 deletion packages/core/src/render3/jit/directive.ts
Expand Up @@ -82,6 +82,9 @@ export function compileComponent(type: Type<any>, metadata: Component): void {
if (metadata.styleUrls && metadata.styleUrls.length) {
error.push(` - styleUrls: ${JSON.stringify(metadata.styleUrls)}`);
}
if (metadata.styleUrl) {
error.push(` - styleUrl: ${metadata.styleUrl}`);
}
error.push(`Did you run and wait for 'resolveComponentResources()'?`);
throw new Error(error.join('\n'));
}
Expand Down Expand Up @@ -114,7 +117,8 @@ export function compileComponent(type: Type<any>, metadata: Component): void {
typeSourceSpan: compiler.createParseSourceSpan('Component', type.name, templateUrl),
template: metadata.template || '',
preserveWhitespaces,
styles: metadata.styles || EMPTY_ARRAY,
styles: typeof metadata.styles === 'string' ? [metadata.styles] :
(metadata.styles || EMPTY_ARRAY),
animations: metadata.animations,
// JIT components are always compiled against an empty set of `declarations`. Instead, the
// `directiveDefs` and `pipeDefs` are updated at a later point:
Expand Down
19 changes: 19 additions & 0 deletions packages/core/test/acceptance/styling_spec.ts
Expand Up @@ -3696,6 +3696,25 @@ describe('styling', () => {
expect(logs).toEqual([]);
});

it('should support `styles` as a string', () => {
if (!isBrowser) {
return;
}

@Component({
template: `<span>Hello</span>`,
styles: `span {font-size: 10px}`,
})
class Cmp {
}
TestBed.configureTestingModule({declarations: [Cmp]});
const fixture = TestBed.createComponent(Cmp);
fixture.detectChanges();

const span = fixture.nativeElement.querySelector('span') as HTMLElement;
expect(getComputedStyle(span).getPropertyValue('font-size')).toBe('10px');
});

describe('regression', () => {
it('should support sanitizer value in the [style] bindings', () => {
@Component({template: `<div [style]="style"></div>`})
Expand Down

0 comments on commit 59387ee

Please sign in to comment.