fixup! feat(core): add memoization to style generation

Author: CaerusKaru

src/lib/core/base/base.ts

@@ -112,16 +112,15 @@ export abstract class BaseDirective implements OnDestroy, OnChanges {
   /** Add styles to the element using predefined style builder */
   protected addStyles(input: string, parent?: Object) {
     let styleDefinition: StyleDefinition;
-    if (this._styleCache.has(input)) {
+    if (this._styleBuilder!.shouldCache && this._styleCache.has(input)) {
       styleDefinition = this._styleCache.get(input)!;
       this._applyStyleToElement(styleDefinition);
     } else {
-      const {shouldCache, styles} = this._styleBuilder!.buildStyles(input, parent);
-      this._applyStyleToElement(styles);
-      if (shouldCache) {
-        this._styleCache.set(input, styles);
+      styleDefinition = this._styleBuilder!.buildStyles(input, parent);
+      this._applyStyleToElement(styleDefinition);
+      if (this._styleBuilder!.shouldCache) {
+        this._styleCache.set(input, styleDefinition);
       }
-      styleDefinition = styles;
     }
     this._styleBuilder!.sideEffect(input, styleDefinition, parent);
   }

src/lib/core/style-builder/style-builder.ts

@@ -5,19 +5,22 @@
  * 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 {Injectable} from '@angular/core';
 import {StyleDefinition} from '../style-utils/style-utils';
 
-@Injectable()
+/** A class that encapsulates CSS style generation for common directives */
 export abstract class StyleBuilder {
-  abstract buildStyles(input: string, parent?: Object): StyleBuilderOutput;
 
+  /** Whether to cache the generated output styles */
+  shouldCache = true;
+
+  /** Build the styles given an input string and configuration object from a host */
+  abstract buildStyles(input: string, parent?: Object): StyleDefinition;
+
+  /**
+   * Run a side effect computation given the input string and the computed styles
+   * from the build task and the host configuration object
+   * NOTE: This should be a no-op unless an algorithm is provided in a subclass
+   */
   sideEffect(_input: string, _styles: StyleDefinition, _parent?: Object) {
-    // This should be a no-op unless an algorithm is provided in a subclass
   }
 }
-
-export interface StyleBuilderOutput {
-  styles: StyleDefinition;
-  shouldCache: boolean;
-}

src/lib/flex/flex-align/flex-align.ts

@@ -20,17 +20,14 @@ import {
   MediaChange,
   MediaMonitor,
   StyleBuilder,
-  StyleBuilderOutput,
+  StyleDefinition,
   StyleUtils,
 } from '@angular/flex-layout/core';
 
 @Injectable({providedIn: 'root'})
 export class FlexAlignStyleBuilder extends StyleBuilder {
-  constructor() {
-    super();
-  }
-  buildStyles(input: string): StyleBuilderOutput {
-    const styles: {[key: string]: string | number} = {};
+  buildStyles(input: string) {
+    const styles: StyleDefinition = {};
 
     // Cross-axis
     switch (input) {
@@ -45,7 +42,7 @@ export class FlexAlignStyleBuilder extends StyleBuilder {
         break;
     }
 
-    return {styles, shouldCache: true};
+    return styles;
   }
 }
 
@@ -128,4 +125,8 @@ export class FlexAlignDirective extends BaseDirective implements OnInit, OnChang
 
     this.addStyles(value && (value + '') || '');
   }
+
+  protected _styleCache = flexAlignCache;
 }
+
+const flexAlignCache: Map<string, StyleDefinition> = new Map();

src/lib/flex/flex-fill/flex-fill.ts

@@ -10,7 +10,7 @@ import {
   BaseDirective,
   MediaMonitor,
   StyleBuilder,
-  StyleBuilderOutput,
+  StyleDefinition,
   StyleUtils,
 } from '@angular/flex-layout/core';
 
@@ -24,11 +24,8 @@ const FLEX_FILL_CSS = {
 
 @Injectable({providedIn: 'root'})
 export class FlexFillStyleBuilder extends StyleBuilder {
-  constructor() {
-    super();
-  }
-  buildStyles(_input: string): StyleBuilderOutput {
-    return {styles: FLEX_FILL_CSS, shouldCache: true};
+  buildStyles(_input: string) {
+    return FLEX_FILL_CSS;
   }
 }
 
@@ -50,4 +47,8 @@ export class FlexFillDirective extends BaseDirective {
     super(monitor, elRef, styleUtils, styleBuilder);
     this.addStyles('');
   }
+
+  protected _styleCache = flexFillCache;
 }
+
+const flexFillCache: Map<string, StyleDefinition> = new Map();

src/lib/flex/flex-offset/flex-offset.spec.ts

@@ -224,11 +224,8 @@ describe('flex-offset directive', () => {
 
 @Injectable({providedIn: FlexModule})
 export class MockFlexOffsetStyleBuilder extends StyleBuilder {
-  constructor() {
-    super();
-  }
   buildStyles(_input: string) {
-    return {styles: {'margin-top': '10px'}, shouldCache: false};
+    return {'margin-top': '10px'};
   }
 }
 

src/lib/flex/flex-offset/flex-offset.ts

@@ -23,7 +23,7 @@ import {
   MediaChange,
   MediaMonitor,
   StyleBuilder,
-  StyleBuilderOutput,
+  StyleDefinition,
   StyleUtils,
 } from '@angular/flex-layout/core';
 import {Subscription} from 'rxjs';
@@ -38,10 +38,7 @@ export interface FlexOffsetParent {
 
 @Injectable({providedIn: 'root'})
 export class FlexOffsetStyleBuilder extends StyleBuilder {
-  constructor() {
-    super();
-  }
-  buildStyles(offset: string, parent: FlexOffsetParent): StyleBuilderOutput {
+  buildStyles(offset: string, parent: FlexOffsetParent) {
     const isPercent = String(offset).indexOf('%') > -1;
     const isPx = String(offset).indexOf('px') > -1;
     if (!isPx && !isPercent && !isNaN(+offset)) {
@@ -51,7 +48,7 @@ export class FlexOffsetStyleBuilder extends StyleBuilder {
     const styles = isFlowHorizontal(parent.layout) ? {[horizontalLayoutKey]: `${offset}`} :
       {'margin-top': `${offset}`};
 
-    return {styles, shouldCache: true};
+    return styles;
   }
 }
 
@@ -189,6 +186,20 @@ export class FlexOffsetDirective extends BaseDirective implements OnInit, OnChan
     // The flex-direction of this element's flex container. Defaults to 'row'.
     const layout = this._getFlexFlowDirection(this.parentElement, true);
     const isRtl = this._directionality.value === 'rtl';
+    if (layout === 'row' && isRtl) {
+      this._styleCache = flexOffsetCacheRowRtl;
+    } else if (layout === 'row' && !isRtl) {
+      this._styleCache = flexOffsetCacheRowLtr;
+    } else if (layout === 'column' && isRtl) {
+      this._styleCache = flexOffsetCacheColumnRtl;
+    } else if (layout === 'column' && !isRtl) {
+      this._styleCache = flexOffsetCacheColumnLtr;
+    }
     this.addStyles((value && (value + '') || ''), {layout, isRtl});
   }
 }
+
+const flexOffsetCacheRowRtl: Map<string, StyleDefinition> = new Map();
+const flexOffsetCacheColumnRtl: Map<string, StyleDefinition> = new Map();
+const flexOffsetCacheRowLtr: Map<string, StyleDefinition> = new Map();
+const flexOffsetCacheColumnLtr: Map<string, StyleDefinition> = new Map();

src/lib/flex/flex-order/flex-order.ts

@@ -20,19 +20,16 @@ import {
   MediaChange,
   MediaMonitor,
   StyleBuilder,
-  StyleBuilderOutput,
+  StyleDefinition,
   StyleUtils,
 } from '@angular/flex-layout/core';
 
 @Injectable({providedIn: 'root'})
 export class FlexOrderStyleBuilder extends StyleBuilder {
-  constructor() {
-    super();
-  }
-  buildStyles(value: string): StyleBuilderOutput {
+  buildStyles(value: string) {
     const val = parseInt(value, 10);
     const styles = {order: isNaN(val) ? 0 : val};
-    return {styles, shouldCache: true};
+    return styles;
   }
 }
 
@@ -112,4 +109,8 @@ export class FlexOrderDirective extends BaseDirective implements OnInit, OnChang
 
     this.addStyles(value || '');
   }
+
+  protected _styleCache = flexOrderCache;
 }
+
+const flexOrderCache: Map<string, StyleDefinition> = new Map();

src/lib/flex/flex/flex.spec.ts

@@ -858,6 +858,7 @@ describe('flex directive', () => {
   });
 
   describe('with column basis zero disabled', () => {
+    let styleBuilder: FlexStyleBuilder;
     beforeEach(() => {
       jasmine.addMatchers(customMatchers);
 
@@ -875,16 +876,22 @@ describe('flex directive', () => {
       });
     });
 
-    it('should set flex basis to auto', async(() => {
+    it('should set flex basis to auto', () => {
       componentWithTemplate(`
         <div fxLayout='column'>
           <div fxFlex></div>
         </div>
       `);
-      fixture.detectChanges();
-      let element = queryFor(fixture, '[fxFlex]')[0];
-      expectEl(element).toHaveStyle({'flex': '1 1 auto'}, styler);
-    }));
+      inject([FlexStyleBuilder], (_styleBuilder: FlexStyleBuilder) => {
+        styleBuilder = _styleBuilder;
+
+        // Reset the cache because the layout config is only set at startup
+        styleBuilder.shouldCache = false;
+        fixture.detectChanges();
+        let element = queryFor(fixture, '[fxFlex]')[0];
+        expectEl(element).toHaveStyle({'flex': '1 1 auto'}, styler);
+      });
+    });
   });
 
   describe('with custom builder', () => {
@@ -927,11 +934,8 @@ describe('flex directive', () => {
 
 @Injectable({providedIn: FlexModule})
 export class MockFlexStyleBuilder extends StyleBuilder {
-  constructor() {
-    super();
-  }
   buildStyles(_input: string) {
-    return {styles: {'flex': '1 1 30%'}, shouldCache: false};
+    return {'flex': '1 1 30%'};
   }
 }
 

src/lib/flex/flex/flex.ts

@@ -27,7 +27,7 @@ import {
   StyleUtils,
   validateBasis,
   StyleBuilder,
-  StyleBuilderOutput,
+  StyleDefinition,
 } from '@angular/flex-layout/core';
 import {Subscription} from 'rxjs';
 
@@ -41,19 +41,16 @@ export type FlexBasisAlias = 'grow' | 'initial' | 'auto' | 'none' | 'nogrow' | '
 interface FlexBuilderParent {
   direction: string;
   hasWrap: boolean;
-  useColumnBasisZero: boolean | undefined;
 }
 
 @Injectable({providedIn: 'root'})
 export class FlexStyleBuilder extends StyleBuilder {
-  constructor() {
+  constructor(@Inject(LAYOUT_CONFIG) protected layoutConfig: LayoutConfigOptions) {
     super();
   }
-  buildStyles(input: string, parent: FlexBuilderParent): StyleBuilderOutput {
-    let grow: string | number;
-    let shrink: string | number;
-    let basis: string | number;
-    [grow, shrink, basis] = input.split('_');
+  buildStyles(input: string, parent: FlexBuilderParent) {
+    let [grow, shrink, ...basisParts]: (string|number)[] = input.split(' ');
+    let basis = basisParts.join(' ');
 
     // The flex-direction of this element's flex container. Defaults to 'row'.
     const direction = (parent.direction.indexOf('column') > -1) ? 'column' : 'row';
@@ -100,7 +97,7 @@ export class FlexStyleBuilder extends StyleBuilder {
     };
     switch (basis || '') {
       case '':
-        const useColumnBasisZero = parent.useColumnBasisZero !== false;
+        const useColumnBasisZero = this.layoutConfig.useColumnBasisZero !== false;
         basis = direction === 'row' ? '0%' : (useColumnBasisZero ? '0.000000001px' : 'auto');
         break;
       case 'initial':   // default
@@ -195,7 +192,7 @@ export class FlexStyleBuilder extends StyleBuilder {
       }
     }
 
-    return {styles: extendObject(css, {'box-sizing': 'border-box'}), shouldCache: true};
+    return extendObject(css, {'box-sizing': 'border-box'}) as StyleDefinition;
   }
 }
 
@@ -313,12 +310,25 @@ export class FlexDirective extends BaseDirective implements OnInit, OnChanges, O
       flexBasis = this._mqActivation.activatedInput;
     }
 
-    let basis = String(flexBasis).replace(';', '');
-    let parts = validateBasis(basis, this._queryInput('grow'), this._queryInput('shrink'));
+    const basis = String(flexBasis).replace(';', '');
+    const parts = validateBasis(basis, this._queryInput('grow'), this._queryInput('shrink'));
     const addFlexToParent = this.layoutConfig.addFlexToParent !== false;
     const direction = this._getFlexFlowDirection(this.parentElement, addFlexToParent);
     const hasWrap = this._layout && this._layout.wrap;
-    const useColumnBasisZero = this.layoutConfig.useColumnBasisZero;
-    this.addStyles(parts.join('_'), {direction, hasWrap, useColumnBasisZero});
+    if (direction === 'row' && hasWrap) {
+      this._styleCache = flexRowWrapCache;
+    } else if (direction === 'row' && !hasWrap) {
+      this._styleCache = flexRowCache;
+    } else if (direction === 'column' && hasWrap) {
+      this._styleCache = flexColumnWrapCache;
+    } else if (direction === 'column' && !hasWrap) {
+      this._styleCache = flexColumnCache;
+    }
+    this.addStyles(parts.join(' '), {direction, hasWrap});
   }
 }
+
+const flexRowCache: Map<string, StyleDefinition> = new Map();
+const flexColumnCache: Map<string, StyleDefinition> = new Map();
+const flexRowWrapCache: Map<string, StyleDefinition> = new Map();
+const flexColumnWrapCache: Map<string, StyleDefinition> = new Map();

src/lib/flex/layout-align/layout-align.spec.ts

@@ -453,11 +453,9 @@ describe('layout-align directive', () => {
 
 @Injectable({providedIn: FlexModule})
 export class MockLayoutAlignStyleBuilder extends StyleBuilder {
-  constructor() {
-    super();
-  }
+  shouldCache = false;
   buildStyles(_input: string) {
-    return {styles: {'justify-content': 'flex-end'}, shouldCache: false};
+    return {'justify-content': 'flex-end'};
   }
 }