From 59387ee476dff1a893a01fe5cbee3c95b93c0cdb Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Mon, 11 Sep 2023 13:56:37 +0200 Subject: [PATCH] feat(core): support styles and styleUrl as strings (#51715) 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 --- goldens/public-api/compiler-cli/error_code.md | 1 + goldens/public-api/core/index.md | 3 +- .../annotations/component/src/resources.ts | 87 ++++++++++++++----- .../ngtsc/annotations/directive/src/shared.ts | 8 +- .../src/ngtsc/diagnostics/src/error_code.ts | 2 + .../compiler-cli/test/ngtsc/ngtsc_spec.ts | 55 ++++++++++++ packages/core/src/metadata/directives.ts | 9 +- .../core/src/metadata/resource_loading.ts | 42 ++++++--- packages/core/src/render3/jit/directive.ts | 6 +- packages/core/test/acceptance/styling_spec.ts | 19 ++++ .../test/metadata/resource_loading_spec.ts | 44 ++++++++++ .../core/testing/src/test_bed_compiler.ts | 5 +- 12 files changed, 239 insertions(+), 42 deletions(-) diff --git a/goldens/public-api/compiler-cli/error_code.md b/goldens/public-api/compiler-cli/error_code.md index 71f623ab9a9..c88c5d2ac80 100644 --- a/goldens/public-api/compiler-cli/error_code.md +++ b/goldens/public-api/compiler-cli/error_code.md @@ -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, diff --git a/goldens/public-api/core/index.md b/goldens/public-api/core/index.md index dbe5a442d05..0a700e089e0 100644 --- a/goldens/public-api/core/index.md +++ b/goldens/public-api/core/index.md @@ -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; diff --git a/packages/compiler-cli/src/ngtsc/annotations/component/src/resources.ts b/packages/compiler-cli/src/ngtsc/annotations/component/src/resources.ts index f998a6e86af..0aed3529629 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/component/src/resources.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/component/src/resources.ts @@ -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; } @@ -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) => { @@ -496,11 +498,35 @@ export function extractComponentStyleUrls( evaluator: PartialEvaluator, component: Map, ): 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( @@ -543,42 +569,63 @@ function extractStyleUrlsFromExpression( return styleUrls; } + export function extractStyleResources( resourceLoader: ResourceLoader, component: Map, containingFile: string): ReadonlySet { const styles = new Set(); 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 []; diff --git a/packages/compiler-cli/src/ngtsc/annotations/directive/src/shared.ts b/packages/compiler-cli/src/ngtsc/annotations/directive/src/shared.ts index ce061f4017f..d9ad67b0a34 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/directive/src/shared.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/directive/src/shared.ts @@ -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)) { @@ -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, }; @@ -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; diff --git a/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts b/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts index ea0a92c7a0c..89affb1321d 100644 --- a/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts +++ b/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts @@ -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, /** diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts index 29d1e8815b7..04570f783c4 100644 --- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts +++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts @@ -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', () => { diff --git a/packages/core/src/metadata/directives.ts b/packages/core/src/metadata/directives.ts index be83a3c43a7..ad047a330d1 100644 --- a/packages/core/src/metadata/directives.ts +++ b/packages/core/src/metadata/directives.ts @@ -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 diff --git a/packages/core/src/metadata/resource_loading.ts b/packages/core/src/metadata/resource_loading.ts index 1c2aca4b2f6..c8c77f2d95b 100644 --- a/packages/core/src/metadata/resource_loading.ts +++ b/packages/core/src/metadata/resource_loading.ts @@ -7,6 +7,7 @@ */ import {Type} from '../interface/type'; + import {Component} from './directives'; @@ -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); }); @@ -104,7 +120,7 @@ export function isComponentDefPendingResolution(type: Type): 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, Component> { const old = componentResourceResolutionQueue; diff --git a/packages/core/src/render3/jit/directive.ts b/packages/core/src/render3/jit/directive.ts index 03591b3d3fb..6f82dfa0ec3 100644 --- a/packages/core/src/render3/jit/directive.ts +++ b/packages/core/src/render3/jit/directive.ts @@ -82,6 +82,9 @@ export function compileComponent(type: Type, 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')); } @@ -114,7 +117,8 @@ export function compileComponent(type: Type, 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: diff --git a/packages/core/test/acceptance/styling_spec.ts b/packages/core/test/acceptance/styling_spec.ts index 1a7dec74012..6566c99d6ea 100644 --- a/packages/core/test/acceptance/styling_spec.ts +++ b/packages/core/test/acceptance/styling_spec.ts @@ -3696,6 +3696,25 @@ describe('styling', () => { expect(logs).toEqual([]); }); + it('should support `styles` as a string', () => { + if (!isBrowser) { + return; + } + + @Component({ + template: `Hello`, + 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: `
`}) diff --git a/packages/core/test/metadata/resource_loading_spec.ts b/packages/core/test/metadata/resource_loading_spec.ts index eaec50acaf6..24bbf6f916f 100644 --- a/packages/core/test/metadata/resource_loading_spec.ts +++ b/packages/core/test/metadata/resource_loading_spec.ts @@ -49,6 +49,7 @@ Did you run and wait for 'resolveComponentResources()'?`.trim()); describe('resolution', () => { const URLS: {[url: string]: Promise} = { 'test://content': Promise.resolve('content'), + 'test://style': Promise.resolve('style'), 'test://style1': Promise.resolve('style1'), 'test://style2': Promise.resolve('style2'), }; @@ -122,6 +123,49 @@ Did you run and wait for 'resolveComponentResources()'?`.trim()); compileComponent(MyComponent2, {templateUrl: 'test://template'}); expect(isComponentResourceResolutionQueueEmpty()).toBe(false); }); + + it('should resolve styles passed in as a string', async () => { + const MyComponent: ComponentType = (class MyComponent {}) as any; + const metadata: Component = {template: '', styles: 'existing'}; + compileComponent(MyComponent, metadata); + await resolveComponentResources(testResolver); + expect(MyComponent.ɵcmp).toBeDefined(); + expect(metadata.styleUrls).toBe(undefined); + expect(metadata.styles).toEqual('existing'); + expect(resourceFetchCount).toBe(0); + }); + + it('should resolve styleUrl', async () => { + const MyComponent: ComponentType = (class MyComponent {}) as any; + const metadata: Component = {template: '', styleUrl: 'test://style'}; + compileComponent(MyComponent, metadata); + await resolveComponentResources(testResolver); + expect(MyComponent.ɵcmp).toBeDefined(); + expect(metadata.styleUrl).toBe(undefined); + expect(metadata.styles).toEqual(['style']); + expect(resourceFetchCount).toBe(1); + }); + + it('should resolve both styles passed in as a string together with styleUrl', async () => { + const MyComponent: ComponentType = (class MyComponent {}) as any; + const metadata: Component = {template: '', styleUrl: 'test://style', styles: 'existing'}; + compileComponent(MyComponent, metadata); + await resolveComponentResources(testResolver); + expect(MyComponent.ɵcmp).toBeDefined(); + expect(metadata.styleUrls).toBe(undefined); + expect(metadata.styles).toEqual(['existing', 'style']); + expect(resourceFetchCount).toBe(1); + }); + + it('should throw if both styleUrls and styleUrl are passed in', async () => { + const MyComponent: ComponentType = (class MyComponent {}) as any; + const metadata: + Component = {template: '', styleUrl: 'test://style1', styleUrls: ['test://style2']}; + compileComponent(MyComponent, metadata); + + expect(() => resolveComponentResources(testResolver)) + .toThrowError(/@Component cannot define both `styleUrl` and `styleUrls`/); + }); }); describe('fetch', () => { diff --git a/packages/core/testing/src/test_bed_compiler.ts b/packages/core/testing/src/test_bed_compiler.ts index 3cb47267e7d..339c36ec10b 100644 --- a/packages/core/testing/src/test_bed_compiler.ts +++ b/packages/core/testing/src/test_bed_compiler.ts @@ -230,7 +230,7 @@ export class TestBedCompiler { const def = (type as any)[NG_COMP_DEF]; const hasStyleUrls = (): boolean => { const metadata = this.resolvers.component.resolve(type)! as Component; - return !!metadata.styleUrls && metadata.styleUrls.length > 0; + return !!metadata.styleUrl || !!metadata.styleUrls?.length; }; const overrideStyleUrls = !!def && !ɵisComponentDefPendingResolution(type) && hasStyleUrls(); @@ -241,7 +241,8 @@ export class TestBedCompiler { // resolution). In order to avoid this, we preemptively set styleUrls to an empty array, // preserve current styles available on Component def and restore styles back once compilation // is complete. - const override = overrideStyleUrls ? {template, styles: [], styleUrls: []} : {template}; + const override = + overrideStyleUrls ? {template, styles: [], styleUrls: [], styleUrl: undefined} : {template}; this.overrideComponent(type, {set: override}); if (overrideStyleUrls && def.styles && def.styles.length > 0) {