Skip to content

Commit

Permalink
feat(compiler-cli): inline resources when generating class metadata c…
Browse files Browse the repository at this point in the history
…alls (#43178)

Previously with View Engine output, the `enableResourceInlining` option
could be set to inline external templates and styles (also for the
resulting `.metadata.json` files). We want to do the same for the Ivy
compilation pipeline (regardless of the compilation mode). The full
compilation definitions, and partial declarations currently already
inline resources in a way that no external requests need to be made.

Although there is one exception currently. These are the calls for
setting class metadata (for testbed overrides). This commit updates
the set class metadata calls (for both partial and full compilation)
to always inline resources. This means that libraries do not need
to start shipping external styles/templates just for the
`setClassMetadata` calls.

Note: Only doing this for partial compilation has been considered, but
it seems like it would be simpler implementation-wise to do this for
full compilation as well. Given the external resources are already
inlined (through their `ecmp` definitions), it seems acceptable (or
even more aligned) to do the same for the set class metadata calls.

PR Close #43178
  • Loading branch information
devversion authored and TeriGlover committed Sep 22, 2021
1 parent a707d5b commit 234b04d
Show file tree
Hide file tree
Showing 8 changed files with 111 additions and 61 deletions.
52 changes: 51 additions & 1 deletion packages/compiler-cli/src/ngtsc/annotations/src/component.ts
Expand Up @@ -504,7 +504,8 @@ export class ComponentDecoratorHandler implements
},
typeCheckMeta: extractDirectiveTypeCheckMeta(node, inputs, this.reflector),
classMetadata: extractClassMetadata(
node, this.reflector, this.isCore, this.annotateForClosureCompiler),
node, this.reflector, this.isCore, this.annotateForClosureCompiler,
dec => this._transformDecoratorToInlineResources(dec, component, styles, template)),
template,
providersRequiringFactory,
viewProvidersRequiringFactory,
Expand Down Expand Up @@ -920,6 +921,55 @@ export class ComponentDecoratorHandler implements
return compileResults(fac, def, classMetadata, 'ɵcmp');
}

/**
* Transforms the given decorator to inline external resources. i.e. if the decorator
* resolves to `@Component`, the `templateUrl` and `styleUrls` metadata fields will be
* transformed to their semantically-equivalent inline variants.
*
* This method is used for serializing decorators into the class metadata. The emitted
* class metadata should not refer to external resources as this would be inconsistent
* with the component definitions/declarations which already inline external resources.
*
* Additionally, the references to external resources would require libraries to ship
* external resources exclusively for the class metadata.
*/
private _transformDecoratorToInlineResources(
dec: Decorator, component: Map<string, ts.Expression>, styles: string[],
template: ParsedTemplateWithSource): Decorator {
if (dec.name !== 'Component') {
return dec;
}

// 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')) {
return dec;
}

const metadata = new Map(component);

// Set the `template` property if the `templateUrl` property is set.
if (metadata.has('templateUrl')) {
metadata.delete('templateUrl');
metadata.set('template', ts.createStringLiteral(template.content));
}

// Set the `styles` property if the `styleUrls` property is set.
if (metadata.has('styleUrls')) {
metadata.delete('styleUrls');
metadata.set('styles', ts.createArrayLiteral(styles.map(s => ts.createStringLiteral(s))));
}

// Convert the metadata to TypeScript AST object literal element nodes.
const newMetadataFields: ts.ObjectLiteralElementLike[] = [];
for (const [name, value] of metadata.entries()) {
newMetadataFields.push(ts.createPropertyAssignment(name, value));
}

// Return the original decorator with the overridden metadata argument.
return {...dec, args: [ts.createObjectLiteral(newMetadataFields)]};
}

private _resolveLiteral(decorator: Decorator): ts.ObjectLiteralExpression {
if (this.literalCache.has(decorator)) {
return this.literalCache.get(decorator)!;
Expand Down
11 changes: 7 additions & 4 deletions packages/compiler-cli/src/ngtsc/annotations/src/metadata.ts
Expand Up @@ -23,7 +23,8 @@ import {valueReferenceToExpression, wrapFunctionExpressionsInParens} from './uti
*/
export function extractClassMetadata(
clazz: DeclarationNode, reflection: ReflectionHost, isCore: boolean,
annotateForClosureCompiler?: boolean): R3ClassMetadata|null {
annotateForClosureCompiler?: boolean,
angularDecoratorTransform: (dec: Decorator) => Decorator = dec => dec): R3ClassMetadata|null {
if (!reflection.isClass(clazz)) {
return null;
}
Expand All @@ -37,7 +38,9 @@ export function extractClassMetadata(
}
const ngClassDecorators =
classDecorators.filter(dec => isAngularDecorator(dec, isCore))
.map(decorator => decoratorToMetadata(decorator, annotateForClosureCompiler))
.map(
decorator => decoratorToMetadata(
angularDecoratorTransform(decorator), annotateForClosureCompiler))
// Since the `setClassMetadata` call is intended to be emitted after the class
// declaration, we have to strip references to the existing identifiers or
// TypeScript might generate invalid code when it emits to JS. In particular
Expand Down Expand Up @@ -156,8 +159,8 @@ function isAngularDecorator(decorator: Decorator, isCore: boolean): boolean {

/**
* Recursively recreates all of the `Identifier` descendant nodes with a particular name inside
* of an AST node, thus removing any references to them. Useful if a particular node has to be t
* aken from one place any emitted to another one exactly as it has been written.
* of an AST node, thus removing any references to them. Useful if a particular node has to be
* taken from one place any emitted to another one exactly as it has been written.
*/
function removeIdentifierReferences<T extends ts.Node>(node: T, name: string): T {
const result = ts.transform(
Expand Down
Expand Up @@ -27,7 +27,7 @@ var __decorate = (this && this.__decorate) || function (decorators, target, key,
else for (var i = decorators.length - 1; i >= 0; i--) if (d = decorators[i]) r = (c < 3 ? d(r) : c > 3 ? d(target, key, r) : d(target, key)) || r;
return c > 3 && r && Object.defineProperty(target, key, r), r;
};
import { Injectable } from '@angular/core';
import { Component, Injectable } from '@angular/core';
import { CustomClassDecorator } from './custom';
import * as i0 from "@angular/core";
export class BasicInjectable {
Expand Down Expand Up @@ -55,6 +55,14 @@ CustomInjectable = __decorate([
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: CustomInjectable, decorators: [{
type: Injectable
}] });
export class ComponentWithExternalResource {
}
ComponentWithExternalResource.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: ComponentWithExternalResource, deps: [], target: i0.ɵɵFactoryTarget.Component });
ComponentWithExternalResource.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", type: ComponentWithExternalResource, selector: "test-cmp", ngImport: i0, template: "<span>Test template</span>\n" });
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: ComponentWithExternalResource, decorators: [{
type: Component,
args: [{ selector: 'test-cmp', template: "<span>Test template</span>\n" }]
}] });

/****************************************************************************************************
* PARTIAL FILE: class_decorators.d.ts
Expand All @@ -68,6 +76,10 @@ export declare class RootInjectable {
static ɵfac: i0.ɵɵFactoryDeclaration<RootInjectable, never>;
static ɵprov: i0.ɵɵInjectableDeclaration<RootInjectable>;
}
export declare class ComponentWithExternalResource {
static ɵfac: i0.ɵɵFactoryDeclaration<ComponentWithExternalResource, never>;
static ɵcmp: i0.ɵɵComponentDeclaration<ComponentWithExternalResource, "test-cmp", never, {}, {}, never, never>;
}

/****************************************************************************************************
* PARTIAL FILE: custom.js
Expand Down
Expand Up @@ -29,3 +29,17 @@ CustomInjectable = __decorate([
type: Injectable
}], null, null);
})();


ComponentWithExternalResource.ɵfac = ;
ComponentWithExternalResource.ɵcmp = ;
(function () {
(typeof ngDevMode === "undefined" || ngDevMode) && $i0$.ɵsetClassMetadata(ComponentWithExternalResource, [{
type: Component,
args: [{
selector: 'test-cmp',
template: "<span>Test template</span>\n"
}]
}], null, null);
})();
@@ -1,4 +1,5 @@
import {Injectable} from '@angular/core';
import {Component, Injectable} from '@angular/core';

import {CustomClassDecorator} from './custom';

@Injectable()
Expand All @@ -13,3 +14,10 @@ export class RootInjectable {
@CustomClassDecorator()
class CustomInjectable {
}

@Component({
selector: 'test-cmp',
templateUrl: 'test_cmp_template.html',
})
export class ComponentWithExternalResource {
}
@@ -0,0 +1 @@
<span>Test template</span>
Expand Up @@ -135,12 +135,7 @@ MyComponent.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.
MyComponent.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", type: MyComponent, selector: "my-component", ngImport: i0, template: "<!--\n NOTE: This template has escaped `\\r\\n` line-endings markers that will be converted to real `\\r\\n` line-ending chars when loaded from the test file-system.\n This conversion happens in the monkeyPatchReadFile() function, which changes `fs.readFile()`.\n-->\n<div title=\"abc\r\ndef\" i18n-title i18n>\r\n Some Message\r\n {\r\n value,\r\n select,\r\n =0 {\r\n zero\r\n }\r\n }</div>" });
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyComponent, decorators: [{
type: Component,
args: [{
selector: 'my-component',
// NOTE: The template has escaped `\r\n` line-endings markers that will be converted to real
// `\r\n` line-ending chars when loaded from the test file-system.
templateUrl: 'template.html'
}]
args: [{ selector: 'my-component', template: "<!--\n NOTE: This template has escaped `\\r\\n` line-endings markers that will be converted to real `\\r\\n` line-ending chars when loaded from the test file-system.\n This conversion happens in the monkeyPatchReadFile() function, which changes `fs.readFile()`.\n-->\n<div title=\"abc\r\ndef\" i18n-title i18n>\r\n Some Message\r\n {\r\n value,\r\n select,\r\n =0 {\r\n zero\r\n }\r\n }</div>" }]
}] });
export class MyModule {
}
Expand Down Expand Up @@ -178,12 +173,7 @@ MyComponent.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.
MyComponent.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", type: MyComponent, selector: "my-component", ngImport: i0, template: "<!--\n NOTE: This template has escaped `\\r\\n` line-endings markers that will be converted to real `\\r\\n` line-ending chars when loaded from the test file-system.\n This conversion happens in the monkeyPatchReadFile() function, which changes `fs.readFile()`.\n-->\n<div title=\"abc\r\ndef\" i18n-title i18n>\r\n Some Message\r\n {\r\n value,\r\n select,\r\n =0 {\r\n zero\r\n }\r\n }</div>" });
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyComponent, decorators: [{
type: Component,
args: [{
selector: 'my-component',
// NOTE: The template has escaped `\r\n` line-endings markers that will be converted to real
// `\r\n` line-ending chars when loaded from the test file-system.
templateUrl: 'template.html'
}]
args: [{ selector: 'my-component', template: "<!--\n NOTE: This template has escaped `\\r\\n` line-endings markers that will be converted to real `\\r\\n` line-ending chars when loaded from the test file-system.\n This conversion happens in the monkeyPatchReadFile() function, which changes `fs.readFile()`.\n-->\n<div title=\"abc\r\ndef\" i18n-title i18n>\r\n Some Message\r\n {\r\n value,\r\n select,\r\n =0 {\r\n zero\r\n }\r\n }</div>" }]
}] });
export class MyModule {
}
Expand Down Expand Up @@ -347,12 +337,7 @@ MyComponent.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.
MyComponent.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", type: MyComponent, selector: "my-component", ngImport: i0, template: "<!--\n NOTE: This template has escaped `\\r\\n` line-endings markers that will be converted to real `\\r\\n` line-ending chars when loaded from the test file-system.\n This conversion happens in the monkeyPatchReadFile() function, which changes `fs.readFile()`.\n-->\n<div title=\"abc\r\ndef\" i18n-title i18n>\r\n Some Message\r\n {\r\n value,\r\n select,\r\n =0 {\r\n zero\r\n }\r\n }</div>" });
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyComponent, decorators: [{
type: Component,
args: [{
selector: 'my-component',
// NOTE: The template has escaped `\r\n` line-endings markers that will be converted to real
// `\r\n` line-ending chars when loaded from the test file-system.
templateUrl: 'template.html'
}]
args: [{ selector: 'my-component', template: "<!--\n NOTE: This template has escaped `\\r\\n` line-endings markers that will be converted to real `\\r\\n` line-ending chars when loaded from the test file-system.\n This conversion happens in the monkeyPatchReadFile() function, which changes `fs.readFile()`.\n-->\n<div title=\"abc\r\ndef\" i18n-title i18n>\r\n Some Message\r\n {\r\n value,\r\n select,\r\n =0 {\r\n zero\r\n }\r\n }</div>" }]
}] });
export class MyModule {
}
Expand Down Expand Up @@ -390,12 +375,7 @@ MyComponent.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.
MyComponent.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", type: MyComponent, selector: "my-component", ngImport: i0, template: "<!--\n NOTE: This template has escaped `\\r\\n` line-endings markers that will be converted to real `\\r\\n` line-ending chars when loaded from the test file-system.\n This conversion happens in the monkeyPatchReadFile() function, which changes `fs.readFile()`.\n-->\n<div title=\"abc\r\ndef\" i18n-title i18n>\r\n Some Message\r\n {\r\n value,\r\n select,\r\n =0 {\r\n zero\r\n }\r\n }</div>" });
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyComponent, decorators: [{
type: Component,
args: [{
selector: 'my-component',
// NOTE: The template has escaped `\r\n` line-endings markers that will be converted to real
// `\r\n` line-ending chars when loaded from the test file-system.
templateUrl: 'template.html'
}]
args: [{ selector: 'my-component', template: "<!--\n NOTE: This template has escaped `\\r\\n` line-endings markers that will be converted to real `\\r\\n` line-ending chars when loaded from the test file-system.\n This conversion happens in the monkeyPatchReadFile() function, which changes `fs.readFile()`.\n-->\n<div title=\"abc\r\ndef\" i18n-title i18n>\r\n Some Message\r\n {\r\n value,\r\n select,\r\n =0 {\r\n zero\r\n }\r\n }</div>" }]
}] });
export class MyModule {
}
Expand Down

0 comments on commit 234b04d

Please sign in to comment.