Skip to content

Commit

Permalink
refactor(ivy): break apart stylingMap into styleMap and classMap inst…
Browse files Browse the repository at this point in the history 
…ructions (#30293)

This patch breaks up the existing `elementStylingMap` into
`elementClassMap` and `elementStyleMap` instructions. It also breaks
apart `hostStlyingMap` into `hostClassMap` and `hostStyleMap`
instructions. This change allows for better tree-shaking and reduces
the complexity of the styling algorithm code for `[style]` and `[class]`
bindings.

PR Close #30293
  • Loading branch information
@matsko @kara
matsko authored and kara committed May 7, 2019
1 parent 98a38ec commit be8fbac
Show file tree
Hide file tree
Showing 18 changed files with 415 additions and 321 deletions.
4 changes: 2 additions & 2 deletions packages/common/src/directives/ng_class.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/
import {Directive, DoCheck, Input, ɵRenderFlags, ɵɵdefineDirective, ɵɵelementHostStyling, ɵɵelementHostStylingApply, ɵɵelementHostStylingMap} from '@angular/core';
import {Directive, DoCheck, Input, ɵRenderFlags, ɵɵdefineDirective, ɵɵelementHostClassMap, ɵɵelementHostStyling, ɵɵelementHostStylingApply} from '@angular/core';

import {NgClassImpl, NgClassImplProvider} from './ng_class_impl';

Expand Down Expand Up @@ -38,7 +38,7 @@ export const ngClassDirectiveDef__POST_R3__ = ɵɵdefineDirective({
ɵɵelementHostStyling();
}
if (rf & ɵRenderFlags.Update) {
ɵɵelementHostStylingMap(ctx.getValue());
ɵɵelementHostClassMap(ctx.getValue());
ɵɵelementHostStylingApply();
}
}
Expand Down
4 changes: 2 additions & 2 deletions packages/common/src/directives/ng_style.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/
import {Directive, DoCheck, Input, ɵRenderFlags, ɵɵdefineDirective, ɵɵelementHostStyling, ɵɵelementHostStylingApply, ɵɵelementHostStylingMap} from '@angular/core';
import {Directive, DoCheck, Input, ɵRenderFlags, ɵɵdefineDirective, ɵɵelementHostStyleMap, ɵɵelementHostStyling, ɵɵelementHostStylingApply} from '@angular/core';

import {NgStyleImpl, NgStyleImplProvider} from './ng_style_impl';

Expand Down Expand Up @@ -38,7 +38,7 @@ export const ngStyleDirectiveDef__POST_R3__ = ɵɵdefineDirective({
ɵɵelementHostStyling();
}
if (rf & ɵRenderFlags.Update) {
ɵɵelementHostStylingMap(null, ctx.getValue());
ɵɵelementHostStyleMap(ctx.getValue());
ɵɵelementHostStylingApply();
}
}
Expand Down
44 changes: 26 additions & 18 deletions packages/compiler-cli/test/compliance/r3_view_compiler_styling_spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ describe('compiler compliance: styling', () => {
$r3$.ɵɵelementEnd();
}
if (rf & 2) {
$r3$.ɵɵelementStylingMap(0, null, $ctx$.myStyleExp);
$r3$.ɵɵelementStyleMap(0, $ctx$.myStyleExp);
$r3$.ɵɵelementStylingApply(0);
}
}
Expand Down Expand Up @@ -454,7 +454,7 @@ describe('compiler compliance: styling', () => {
$r3$.ɵɵelementEnd();
}
if (rf & 2) {
$r3$.ɵɵelementStylingMap(0, $r3$.ɵɵinterpolation1("foo foo-", $ctx$.fooId, ""));
$r3$.ɵɵelementClassMap(0, $r3$.ɵɵinterpolation1("foo foo-", $ctx$.fooId, ""));
$r3$.ɵɵelementStylingApply(0);
}
}
Expand All @@ -468,7 +468,7 @@ describe('compiler compliance: styling', () => {
$r3$.ɵɵelementEnd();
}
if (rf & 2) {
$r3$.ɵɵelementStylingMap(0, $r3$.ɵɵinterpolation2("foo foo-", $ctx$.fooId, "-", $ctx$.fooUsername, ""));
$r3$.ɵɵelementClassMap(0, $r3$.ɵɵinterpolation2("foo foo-", $ctx$.fooId, "-", $ctx$.fooUsername, ""));
$r3$.ɵɵelementStylingApply(0);
}
}
Expand All @@ -482,7 +482,7 @@ describe('compiler compliance: styling', () => {
$r3$.ɵɵelementEnd();
}
if (rf & 2) {
$r3$.ɵɵelementStylingMap(0, $ctx$.exp);
$r3$.ɵɵelementClassMap(0, $ctx$.exp);
$r3$.ɵɵelementStylingApply(0);
}
}
Expand Down Expand Up @@ -538,7 +538,7 @@ describe('compiler compliance: styling', () => {
$r3$.ɵɵelementEnd();
}
if (rf & 2) {
$r3$.ɵɵelementStylingMap(0, null, $ctx$.myStyleExp);
$r3$.ɵɵelementStyleMap(0, $ctx$.myStyleExp);
$r3$.ɵɵelementStyleProp(0, 0, $ctx$.myWidth);
$r3$.ɵɵelementStyleProp(0, 1, $ctx$.myHeight);
$r3$.ɵɵelementStylingApply(0);
Expand Down Expand Up @@ -704,7 +704,7 @@ describe('compiler compliance: styling', () => {
$r3$.ɵɵelementEnd();
}
if (rf & 2) {
$r3$.ɵɵelementStylingMap(0,$ctx$.myClassExp);
$r3$.ɵɵelementClassMap(0,$ctx$.myClassExp);
$r3$.ɵɵelementStylingApply(0);
}
}
Expand Down Expand Up @@ -760,7 +760,7 @@ describe('compiler compliance: styling', () => {
$r3$.ɵɵelementEnd();
}
if (rf & 2) {
$r3$.ɵɵelementStylingMap(0, $ctx$.myClassExp);
$r3$.ɵɵelementClassMap(0, $ctx$.myClassExp);
$r3$.ɵɵelementClassProp(0, 0, $ctx$.yesToApple);
$r3$.ɵɵelementClassProp(0, 1, $ctx$.yesToOrange);
$r3$.ɵɵelementStylingApply(0);
Expand Down Expand Up @@ -882,7 +882,8 @@ describe('compiler compliance: styling', () => {
$r3$.ɵɵelementEnd();
}
if (rf & 2) {
$r3$.ɵɵelementStylingMap(0, $ctx$.myClassExp, $ctx$.myStyleExp);
$r3$.ɵɵelementStyleMap(0, $ctx$.myStyleExp);
$r3$.ɵɵelementClassMap(0, $ctx$.myClassExp);
$r3$.ɵɵelementStylingApply(0);
}
}
Expand Down Expand Up @@ -919,12 +920,13 @@ describe('compiler compliance: styling', () => {
if (rf & 1) {
$r3$.ɵɵelementStart(0, "div");
$r3$.ɵɵelementStyling(null, null, $r3$.ɵɵdefaultStyleSanitizer);
$r3$.ɵɵpipe(1, "classPipe");
$r3$.ɵɵpipe(2, "stylePipe");
$r3$.ɵɵpipe(1, "stylePipe");
$r3$.ɵɵpipe(2, "classPipe");
$r3$.ɵɵelementEnd();
}
if (rf & 2) {
$r3$.ɵɵelementStylingMap(0, $r3$.ɵɵpipeBind1(1, 0, $ctx$.myClassExp), $r3$.ɵɵpipeBind1(2, 2, $ctx$.myStyleExp));
$r3$.ɵɵelementStyleMap(0, $r3$.ɵɵpipeBind1(1, 0, $ctx$.myStyleExp));
$r3$.ɵɵelementClassMap(0, $r3$.ɵɵpipeBind1(2, 2, $ctx$.myClassExp));
$r3$.ɵɵelementStylingApply(0);
}
}
Expand Down Expand Up @@ -980,7 +982,8 @@ describe('compiler compliance: styling', () => {
$r3$.ɵɵelementEnd();
}
if (rf & 2) {
$r3$.ɵɵelementStylingMap(0, $e2_styling$, $r3$.ɵɵpipeBind2(1, 1, $ctx$.myStyleExp, 1000));
$r3$.ɵɵelementStyleMap(0, $r3$.ɵɵpipeBind2(1, 1, $ctx$.myStyleExp, 1000));
$r3$.ɵɵelementClassMap(0, $e2_styling$);
$r3$.ɵɵelementStyleProp(0, 0, $r3$.ɵɵpipeBind2(2, 4, $ctx$.barExp, 3000));
$r3$.ɵɵelementStyleProp(0, 1, $r3$.ɵɵpipeBind2(3, 7, $ctx$.bazExp, 4000));
$r3$.ɵɵelementClassProp(0, 0, $r3$.ɵɵpipeBind2(4, 10, $ctx$.fooExp, 2000));
Expand Down Expand Up @@ -1042,7 +1045,8 @@ describe('compiler compliance: styling', () => {
$r3$.ɵɵelementHostStyling($e0_classBindings$, $e0_styleBindings$, $r3$.ɵɵdefaultStyleSanitizer);
}
if (rf & 2) {
$r3$.ɵɵelementHostStylingMap(ctx.myClass, ctx.myStyle);
$r3$.ɵɵelementHostStyleMap(ctx.myStyle);
$r3$.ɵɵelementHostClassMap(ctx.myClass);
$r3$.ɵɵelementHostStyleProp(0, ctx.myColorProp);
$r3$.ɵɵelementHostClassProp(0, ctx.myFooClass);
$r3$.ɵɵelementHostStylingApply();
Expand Down Expand Up @@ -1102,7 +1106,8 @@ describe('compiler compliance: styling', () => {
$r3$.ɵɵelementHostStyling(_c0, _c1, $r3$.ɵɵdefaultStyleSanitizer);
}
if (rf & 2) {
$r3$.ɵɵelementHostStylingMap(ctx.myClasses, ctx.myStyle);
$r3$.ɵɵelementHostStyleMap(ctx.myStyle);
$r3$.ɵɵelementHostClassMap(ctx.myClasses);
$r3$.ɵɵelementHostStyleProp(0, ctx.myHeightProp, "pt");
$r3$.ɵɵelementHostStyleProp(1, ctx.myWidthProp);
$r3$.ɵɵelementHostClassProp(0, ctx.myBarClass);
Expand Down Expand Up @@ -1166,7 +1171,8 @@ describe('compiler compliance: styling', () => {
$r3$.ɵɵelementEnd();
}
if (rf & 2) {
$r3$.ɵɵelementStylingMap(0, ctx.myClassExp, ctx.myStyleExp);
$r3$.ɵɵelementStyleMap(0, ctx.myStyleExp);
$r3$.ɵɵelementClassMap(0, ctx.myClassExp);
$r3$.ɵɵelementStyleProp(0, 0, ctx.myHeightExp, null, true);
$r3$.ɵɵelementClassProp(0, 0, ctx.myBarClassExp, true);
$r3$.ɵɵelementStylingApply(0);
Expand All @@ -1183,7 +1189,8 @@ describe('compiler compliance: styling', () => {
$r3$.ɵɵelementHostStyling(_c0, _c1, $r3$.ɵɵdefaultStyleSanitizer);
}
if (rf & 2) {
$r3$.ɵɵelementHostStylingMap(ctx.myClassExp, ctx.myStyleExp);
$r3$.ɵɵelementHostStyleMap(ctx.myStyleExp);
$r3$.ɵɵelementHostClassMap(ctx.myClassExp);
$r3$.ɵɵelementHostStyleProp(0, ctx.myWidthExp, null, true);
$r3$.ɵɵelementHostClassProp(0, ctx.myFooClassExp, true);
$r3$.ɵɵelementHostStylingApply();
Expand Down Expand Up @@ -1251,7 +1258,7 @@ describe('compiler compliance: styling', () => {
$r3$.ɵɵelementHostStyling();
}
if (rf & 2) {
$r3$.ɵɵelementHostStylingMap(ctx.myClassMap);
$r3$.ɵɵelementHostClassMap(ctx.myClassMap);
$r3$.ɵɵelementHostStylingApply();
}
}
Expand Down Expand Up @@ -1335,7 +1342,8 @@ describe('compiler compliance: styling', () => {
if (rf & 2) {
$r3$.ɵɵproperty("id", ctx.id, null, true);
$r3$.ɵɵproperty("title", ctx.title, null, true);
$r3$.ɵɵelementHostStylingMap(ctx.myClass, ctx.myStyle);
$r3$.ɵɵelementHostStyleMap(ctx.myStyle);
$r3$.ɵɵelementHostClassMap(ctx.myClass);
$r3$.ɵɵelementHostStylingApply();
}
}
Expand Down
11 changes: 8 additions & 3 deletions packages/compiler/src/render3/r3_identifiers.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,9 @@ export class Identifiers {

static elementStyling: o.ExternalReference = {name: 'ɵɵelementStyling', moduleName: CORE};

static elementStylingMap: o.ExternalReference = {name: 'ɵɵelementStylingMap', moduleName: CORE};
static elementStyleMap: o.ExternalReference = {name: 'ɵɵelementStyleMap', moduleName: CORE};

static elementClassMap: o.ExternalReference = {name: 'ɵɵelementClassMap', moduleName: CORE};

static elementStyleProp: o.ExternalReference = {name: 'ɵɵelementStyleProp', moduleName: CORE};

Expand All @@ -62,8 +64,11 @@ export class Identifiers {

static elementHostStyling: o.ExternalReference = {name: 'ɵɵelementHostStyling', moduleName: CORE};

static elementHostStylingMap:
o.ExternalReference = {name: 'ɵɵelementHostStylingMap', moduleName: CORE};
static elementHostStyleMap:
o.ExternalReference = {name: 'ɵɵelementHostStyleMap', moduleName: CORE};

static elementHostClassMap:
o.ExternalReference = {name: 'ɵɵelementHostClassMap', moduleName: CORE};

static elementHostStyleProp:
o.ExternalReference = {name: 'ɵɵelementHostStyleProp', moduleName: CORE};
Expand Down
121 changes: 60 additions & 61 deletions packages/compiler/src/render3/view/styling_builder.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@ interface BoundStylingEntry {
* elementStyling(...)
* }
* if (updateMode) {
* elementStylingMap(...)
* elementStyleMap(...)
* elementClassMap(...)
* elementStyleProp(...)
* elementClassProp(...)
* elementStylingApp(...)
Expand Down Expand Up @@ -339,71 +340,65 @@ export class StylingBuilder {
}

/**
* Builds an instruction with all the expressions and parameters for `elementStylingMap`.
* Builds an instruction with all the expressions and parameters for `elementClassMap`.
*
* The instruction data will contain all expressions for `elementStylingMap` to function
* which include the `[style]` and `[class]` expression params (if they exist) as well as
* the sanitizer and directive reference expression.
* The instruction data will contain all expressions for `elementClassMap` to function
* which includes the `[class]` expression params.
*/
buildElementStylingMapInstruction(valueConverter: ValueConverter): Instruction|null {
if (this._classMapInput || this._styleMapInput) {
const stylingInput = this._classMapInput ! || this._styleMapInput !;
let totalBindingSlotsRequired = 0;

// these values must be outside of the update block so that they can
// be evaluted (the AST visit call) during creation time so that any
// pipes can be picked up in time before the template is built
const mapBasedClassValue =
this._classMapInput ? this._classMapInput.value.visit(valueConverter) : null;
if (mapBasedClassValue instanceof Interpolation) {
totalBindingSlotsRequired += mapBasedClassValue.expressions.length;
}

const mapBasedStyleValue =
this._styleMapInput ? this._styleMapInput.value.visit(valueConverter) : null;
if (mapBasedStyleValue instanceof Interpolation) {
totalBindingSlotsRequired += mapBasedStyleValue.expressions.length;
}
buildElementClassMapInstruction(valueConverter: ValueConverter): Instruction|null {
if (this._classMapInput) {
return this._buildMapBasedInstruction(valueConverter, true, this._classMapInput);
}
return null;
}

const isHostBinding = this._directiveExpr;
const reference = isHostBinding ? R3.elementHostStylingMap : R3.elementStylingMap;
/**
* Builds an instruction with all the expressions and parameters for `elementStyleMap`.
*
* The instruction data will contain all expressions for `elementStyleMap` to function
* which includes the `[style]` expression params.
*/
buildElementStyleMapInstruction(valueConverter: ValueConverter): Instruction|null {
if (this._styleMapInput) {
return this._buildMapBasedInstruction(valueConverter, false, this._styleMapInput);
}
return null;
}

return {
sourceSpan: stylingInput.sourceSpan,
reference,
allocateBindingSlots: totalBindingSlotsRequired,
buildParams: (convertFn: (value: any) => o.Expression) => {
// HOST:
// min params => elementHostStylingMap(classMap)
// max params => elementHostStylingMap(classMap, styleMap)
// Template:
// min params => elementStylingMap(elmIndex, classMap)
// max params => elementStylingMap(elmIndex, classMap, styleMap)
private _buildMapBasedInstruction(
valueConverter: ValueConverter, isClassBased: boolean, stylingInput: BoundStylingEntry) {
let totalBindingSlotsRequired = 0;

const params: o.Expression[] = [];
if (!isHostBinding) {
params.push(this._elementIndexExpr);
}
// these values must be outside of the update block so that they can
// be evaluated (the AST visit call) during creation time so that any
// pipes can be picked up in time before the template is built
const mapValue = stylingInput.value.visit(valueConverter);
if (mapValue instanceof Interpolation) {
totalBindingSlotsRequired += mapValue.expressions.length;
}

let expectedNumberOfArgs = 0;
if (mapBasedStyleValue) {
expectedNumberOfArgs = 2;
} else if (mapBasedClassValue) {
// index and class = 2
expectedNumberOfArgs = 1;
}
const isHostBinding = this._directiveExpr;
let reference: o.ExternalReference;
if (isClassBased) {
reference = isHostBinding ? R3.elementHostClassMap : R3.elementClassMap;
} else {
reference = isHostBinding ? R3.elementHostStyleMap : R3.elementStyleMap;
}

addParam(
params, mapBasedClassValue, mapBasedClassValue ? convertFn(mapBasedClassValue) : null,
1, expectedNumberOfArgs);
addParam(
params, mapBasedStyleValue, mapBasedStyleValue ? convertFn(mapBasedStyleValue) : null,
2, expectedNumberOfArgs);
return params;
return {
sourceSpan: stylingInput.sourceSpan,
reference,
allocateBindingSlots: totalBindingSlotsRequired,
buildParams: (convertFn: (value: any) => o.Expression) => {
const params: o.Expression[] = [];
if (!isHostBinding) {
params.push(this._elementIndexExpr);
}
};
}
return null;

params.push(convertFn(mapValue));
return params;
}
};
}

private _buildSingleInputs(
Expand Down Expand Up @@ -498,9 +493,13 @@ export class StylingBuilder {
buildUpdateLevelInstructions(valueConverter: ValueConverter) {
const instructions: Instruction[] = [];
if (this.hasBindings) {
const mapInstruction = this.buildElementStylingMapInstruction(valueConverter);
if (mapInstruction) {
instructions.push(mapInstruction);
const styleMapInstruction = this.buildElementStyleMapInstruction(valueConverter);
if (styleMapInstruction) {
instructions.push(styleMapInstruction);
}
const classMapInstruction = this.buildElementClassMapInstruction(valueConverter);
if (classMapInstruction) {
instructions.push(classMapInstruction);
}
instructions.push(...this._buildStyleInputs(valueConverter));
instructions.push(...this._buildClassInputs(valueConverter));
Expand Down
4 changes: 2 additions & 2 deletions packages/compiler/src/render3/view/template.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -686,8 +686,8 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver

// the code here will collect all update-level styling instructions and add them to the
// update block of the template function AOT code. Instructions like `elementStyleProp`,
// `elementStylingMap`, `elementClassProp` and `elementStylingApply` are all generated
// and assign in the code below.
// `elementStyleMap`, `elementClassMap`, `elementClassProp` and `elementStylingApply`
// are all generated and assigned in the code below.
stylingBuilder.buildUpdateLevelInstructions(this._valueConverter).forEach(instruction => {
this._bindingSlots += instruction.allocateBindingSlots;
this.processStylingInstruction(implicit, instruction, false);
Expand Down
6 changes: 4 additions & 2 deletions packages/core/src/core_render3_private_export.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,14 +111,16 @@ export {
ɵɵelementContainerStart,
ɵɵelementContainerEnd,
ɵɵelementStyling,
ɵɵelementStylingMap,
ɵɵelementStyleMap,
ɵɵelementClassMap,
ɵɵelementStyleProp,
ɵɵelementStylingApply,
ɵɵelementClassProp,

ɵɵelementHostAttrs,
ɵɵelementHostClassMap,
ɵɵelementHostStyleMap,
ɵɵelementHostStyling,
ɵɵelementHostStylingMap,
ɵɵelementHostStyleProp,
ɵɵelementHostClassProp,
ɵɵelementHostStylingApply,
Expand Down

0 comments on commit be8fbac

Please sign in to comment.