Skip to content

Commit

Permalink
fix(compiler-cli): enableResourceInlining handles both styles and sty…
Browse files Browse the repository at this point in the history 
…leUrls (#22688)

When both are present, the inlined styles are appended to the end of the styles

PR Close #22688
  • Loading branch information
@alexeagle @kara
alexeagle authored and kara committed Mar 12, 2018
1 parent 123efba commit 40315be
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 57 deletions.
7 changes: 3 additions & 4 deletions packages/bazel/src/ng_module.bzl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,7 @@ def _ngc_tsconfig(ctx, files, srcs, **kwargs):

return dict(tsc_wrapped_tsconfig(ctx, files, srcs, **kwargs), **{
"angularCompilerOptions": {
# Always assume that resources can be loaded statically at build time
# TODO(alexeagle): if someone has a legitimate use case for dynamic
# template loading, maybe we need to make this configurable.
"enableResourceInlining": True,
"enableResourceInlining": ctx.attr.inline_resources,
"generateCodeForLibraries": False,
"allowEmptyCodegenFiles": True,
"enableSummariesForJit": True,
Expand Down Expand Up @@ -346,6 +343,8 @@ NG_MODULE_ATTRIBUTES = {

"type_check": attr.bool(default = True),

"inline_resources": attr.bool(default = True),

"no_i18n": attr.bool(default = False),

"compiler": attr.label(
Expand Down
108 changes: 61 additions & 47 deletions packages/compiler-cli/src/transformers/inline_resources.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,16 @@ export class InlineResourcesMetadataTransformer implements MetadataTransformer {
arg['template'] = loader.get(arg['templateUrl']);
delete arg.templateUrl;
}
if (arg['styleUrls']) {
const styleUrls = arg['styleUrls'];
if (Array.isArray(styleUrls)) {
arg['styles'] = styleUrls.map(styleUrl => loader.get(styleUrl));
delete arg.styleUrls;
}

const styles = arg['styles'] || [];
const styleUrls = arg['styleUrls'] || [];
if (!Array.isArray(styles)) throw new Error('styles should be an array');
if (!Array.isArray(styleUrls)) throw new Error('styleUrls should be an array');

styles.push(...styleUrls.map(styleUrl => loader.get(styleUrl)));
if (styles.length > 0) {
arg['styles'] = styles;
delete arg.styleUrls;
}

return arg;
Expand Down Expand Up @@ -262,49 +266,59 @@ function updateComponentProperties(
// argument
return args;
}
const newArgument = ts.updateObjectLiteral(
componentArg, ts.visitNodes(componentArg.properties, (node: ts.ObjectLiteralElementLike) => {
if (!ts.isPropertyAssignment(node)) {
// Error: unsupported
return node;
}

if (ts.isComputedPropertyName(node.name)) {
// computed names are not supported
return node;
const newProperties: ts.ObjectLiteralElementLike[] = [];
const newStyleExprs: ts.Expression[] = [];
componentArg.properties.forEach(prop => {
if (!ts.isPropertyAssignment(prop) || ts.isComputedPropertyName(prop.name)) {
newProperties.push(prop);
return;
}

switch (prop.name.text) {
case 'styles':
if (!ts.isArrayLiteralExpression(prop.initializer)) {
throw new Error('styles takes an array argument');
}
newStyleExprs.push(...prop.initializer.elements);
break;

const name = node.name.text;
switch (name) {
case 'styleUrls':
if (!ts.isArrayLiteralExpression(node.initializer)) {
// Error: unsupported
return node;
}
const styleUrls = node.initializer.elements;

return ts.updatePropertyAssignment(
node, ts.createIdentifier('styles'),
ts.createArrayLiteral(ts.visitNodes(styleUrls, (expr: ts.Expression) => {
if (ts.isStringLiteral(expr)) {
const styles = loader.get(expr.text);
return ts.createLiteral(styles);
}
return expr;
})));


case 'templateUrl':
if (ts.isStringLiteral(node.initializer)) {
const template = loader.get(node.initializer.text);
return ts.updatePropertyAssignment(
node, ts.createIdentifier('template'), ts.createLiteral(template));
}
return node;

default:
return node;
case 'styleUrls':
if (!ts.isArrayLiteralExpression(prop.initializer)) {
throw new Error('styleUrls takes an array argument');
}
newStyleExprs.push(...prop.initializer.elements.map((expr: ts.Expression) => {
if (!ts.isStringLiteral(expr) && !ts.isNoSubstitutionTemplateLiteral(expr)) {
throw new Error(
'Can only accept string literal arguments to styleUrls. ' + PRECONDITIONS_TEXT);
}
const styles = loader.get(expr.text);
return ts.createLiteral(styles);
}));
break;

case 'templateUrl':
if (!ts.isStringLiteral(prop.initializer) &&
!ts.isNoSubstitutionTemplateLiteral(prop.initializer)) {
throw new Error(
'Can only accept a string literal argument to templateUrl. ' + PRECONDITIONS_TEXT);
}
}));
return ts.createNodeArray<ts.Expression>([newArgument]);
const template = loader.get(prop.initializer.text);
newProperties.push(ts.updatePropertyAssignment(
prop, ts.createIdentifier('template'), ts.createLiteral(template)));
break;

default:
newProperties.push(prop);
}
});

// Add the non-inline styles
if (newStyleExprs.length > 0) {
const newStyles = ts.createPropertyAssignment(
ts.createIdentifier('styles'), ts.createArrayLiteral(newStyleExprs));
newProperties.push(newStyles);
}

return ts.createNodeArray([ts.updateObjectLiteral(componentArg, newProperties)]);
}
29 changes: 23 additions & 6 deletions packages/compiler-cli/test/transformers/inline_resources_spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,18 @@ describe('inline resources transformer', () => {
const actual = convert(`import {Component} from '@angular/core';
@Component({
templateUrl: './thing.html',
otherProp: 3,
}) export class Foo {}`);
otherProp: 3,
}) export class Foo {}`);
expect(actual).not.toContain('templateUrl:');
expect(actual.replace(/\s+/g, ' '))
.toContain(
'Foo = __decorate([ core_1.Component({ template: "Some template", otherProp: 3, }) ], Foo)');
'Foo = __decorate([ core_1.Component({ template: "Some template", otherProp: 3 }) ], Foo)');
});
it('should allow different quotes', () => {
const actual = convert(`import {Component} from '@angular/core';
@Component({"templateUrl": \`./thing.html\`}) export class Foo {}`);
expect(actual).not.toContain('templateUrl:');
expect(actual).toContain('{ template: "Some template" }');
});
it('should replace styleUrls', () => {
const actual = convert(`import {Component} from '@angular/core';
Expand All @@ -58,11 +64,21 @@ describe('inline resources transformer', () => {
expect(actual).not.toContain('styleUrls:');
expect(actual).toContain('styles: [".some_style {}", ".some_other_style {}"]');
});
it('should preserve existing styles', () => {
const actual = convert(`import {Component} from '@angular/core';
@Component({
styles: ['h1 { color: blue }'],
styleUrls: ['./thing1.css'],
})
export class Foo {}`);
expect(actual).not.toContain('styleUrls:');
expect(actual).toContain(`styles: ['h1 { color: blue }', ".some_style {}"]`);
});
it('should handle empty styleUrls', () => {
const actual = convert(`import {Component} from '@angular/core';
@Component({styleUrls: []}) export class Foo {}`);
@Component({styleUrls: [], styles: []}) export class Foo {}`);
expect(actual).not.toContain('styleUrls:');
expect(actual).toContain('styles: []');
expect(actual).not.toContain('styles:');
});
});
describe('annotation input', () => {
Expand Down Expand Up @@ -115,6 +131,7 @@ describe('metadata transformer', () => {
@Component({
templateUrl: './thing.html',
styleUrls: ['./thing1.css', './thing2.css'],
styles: ['h1 { color: red }'],
})
export class Foo {}
`;
Expand All @@ -135,7 +152,7 @@ describe('metadata transformer', () => {
expect(JSON.stringify(classData)).toContain('"template":"Some template"');
expect(JSON.stringify(classData)).not.toContain('styleUrls');
expect(JSON.stringify(classData))
.toContain('"styles":[".some_style {}",".some_other_style {}"]');
.toContain('"styles":["h1 { color: red }",".some_style {}",".some_other_style {}"]');
}
}
});
Expand Down

0 comments on commit 40315be

Please sign in to comment.