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

When both are present, the inlined styles are appended to the end of the styles
  • Loading branch information
@alexeagle
alexeagle committed Mar 10, 2018
1 parent 8594afe commit 629cd37
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 629cd37

Please sign in to comment.