Skip to content

Commit

Permalink
fix(flexbox): resolve 'renderer.setStyle()' error
Browse files Browse the repository at this point in the history 
Directives extending NgClass and NgStyle require deprecated Renderer; while
other FlexBox directives require Renderer2. Injecting both Renderers
causes intermittent `TypeError: _this._renderer.setStyle is not a function` errors.

*  Refactor the FxBaseDirective to use a StyleRenderer interface, and
*  Inject only the Renderer instance into the ClassDirective and StyleDirective constructors

Fixes #270
  • Loading branch information
@ThomasBurleson
ThomasBurleson committed May 25, 2017
1 parent f5558de commit 978a897
Show file tree
Hide file tree
Showing 6 changed files with 1,946 additions and 1,323 deletions.
86 changes: 49 additions & 37 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
"core-js": "^2.4.1",
"reflect-metadata": "^0.1.8",
"rxjs": "^5.0.1",
"systemjs": "^0.19.41",
"systemjs": "0.19.43",
"zone.js": "^0.8.4"
},
"devDependencies": {
Expand All @@ -61,81 +61,93 @@
"@angularclass/request-idle-callback": "^1.0.7",
"@angularclass/webpack-toolkit": "^1.3.3",
"@types/core-js": "^0.9.34",
"@types/glob": "^5.0.29",
"@types/gulp": "^3.8.29",
"@types/hammerjs": "~2.0.33",
"@types/jasmine": "2.5.38",
"@types/merge2": "0.0.28",
"@types/minimist": "^1.1.28",
"@types/node": "^6.0.45",
"@types/run-sequence": "0.0.27",
"@types/rx": "^2.5.33",
"@types/glob": "^5.0.30",
"@types/gulp": "^4.0.3",
"@types/hammerjs": "^2.0.34",
"@types/jasmine": "2.5.45",
"@types/merge2": "^0.3.30",
"@types/minimist": "^1.2.0",
"@types/node": "^7.0.21",
"@types/run-sequence": "^0.0.29",
"@types/rx": "2.5.33",

"@types/source-map": "^0.1.27",
"@types/uglify-js": "^2.0.27",
"@types/webpack": "^1.12.34",
"@types/webpack": "^2.2.15",
"angular2-template-loader": "^0.6.0",
"assets-webpack-plugin": "^3.4.0",
"awesome-typescript-loader": "3.0.0-beta.9",
"assets-webpack-plugin": "^3.5.1",
"awesome-typescript-loader": "^3.1.3",
"browserstacktunnel-wrapper": "^2.0.0",
"clang-format": "^1.0.45",
"concurrently": "^2.2.0",
"conventional-changelog": "^1.1.0",
"copy-webpack-plugin": "^4.0.0",
"css-loader": "^0.25.0",
"extract-text-webpack-plugin": "^1.0.1",
"file-loader": "^0.9.0",
"fs-extra": "^0.26.5",
"glob": "^6.0.4",
"copy-webpack-plugin": "^4.0.1",
"css-loader": "^0.28.2",
"extract-text-webpack-plugin": "^2.1.0",
"file-loader": "^0.11.1",
"fs-extra": "^3.0.1",
"glob": "^7.1.2",
"gulp": "^3.9.1",
"gulp-autoprefixer": "^3.1.1",
"gulp-better-rollup": "^1.0.2",
"gulp-clang-format": "^1.0.23",
"gulp-clean": "^0.3.2",
"gulp-cli": "^1.2.2",
"gulp-cli": "^1.3.0",
"gulp-conventional-changelog": "^1.1.0",
"gulp-server-livereload": "^1.8.2",
"gulp-sourcemaps": "^1.6.0",
"gulp-dom": "^0.9.17",
"gulp-flatten": "^0.3.1",
"gulp-highlight-files": "^0.0.4",
"gulp-htmlmin": "^3.0.0",
"gulp-if": "^2.0.2",
"gulp-markdown": "^1.2.0",
"gulp-rename": "^1.2.2",
"gulp-sass": "^3.1.0",
"gulp-sourcemaps": "^2.6.0",
"gulp-transform": "^2.0.0",
"gulp-server-livereload": "^1.9.2",
"gulp-transform": "^2.0.0",
"gulp-typescript": "^2.13.6",
"hammerjs": "~2.0.8",
"hammerjs": "^2.0.8",
"html-loader": "^0.4.3",
"html-webpack-plugin": "^2.22.0",
"jasmine-core": "^2.4.1",
"karma": "^1.3.0",
"jasmine-core": "^2.6.2",
"karma": "^1.7.0",
"karma-browserstack-launcher": "^1.2.0",
"karma-chrome-launcher": "^2.0.0",
"karma-chrome-launcher": "^2.1.1",
"karma-coverage": "^1.1.1",
"karma-jasmine": "^1.0.2",
"karma-firefox-launcher": "^1.0.1",
"karma-jasmine": "^1.1.0",
"karma-mocha-reporter": "^2.0.0",
"karma-remap-coverage": "^0.1.1",
"karma-sauce-launcher": "^1.1.0",
"karma-sourcemap-loader": "^0.3.7",
"karma-webpack": "1.8.1",
"madge": "^1.4.4",
"merge2": "^1.0.2",
"madge": "^1.6.0",
"merge2": "^1.0.3",
"minimist": "^1.2.0",
"opn-cli": "^3.1.0",
"prompt-sync": "^4.1.4",
"raw-loader": "^0.5.1",
"reflect-metadata": "^0.1.8",
"resolve-bin": "^0.4.0",
"rollup": "^0.34.13",
"rollup": "^0.41.6",
"rollup-plugin-includepaths": "^0.1.6",
"rollup-plugin-uglify": "^1.0.1",
"run-sequence": "^1.2.2",
"sass": "^0.5.0",
"source-map-loader": "^0.1.5",
"style-loader": "^0.13.1",
"stylelint": "^7.5.0",
"stylelint": "^7.10.1",
"travis-after-modes": "0.0.7",
"ts-helpers": "1.1.2",
"ts-node": "^0.7.3",
"tslint": "^4.2.0",
"ts-node": "^3.0.4",
"tslint": "^5.2.0",
"tslint-loader": "^3.3.0",
"typescript": "^2.1.10",
"typescript": "~2.2.1",
"url-loader": "^0.5.7",
"webpack": "2.2.0-rc.3",
"webpack": "2.6.1",
"webpack-bundle-analyzer": "^2.2.0",
"webpack-dev-server": "^2.2.0-rc.0",
"webpack-dev-server": "^2.4.5",
"webpack-livereload-plugin": "^0.9.0"
}
}
}
6 changes: 3 additions & 3 deletions src/lib/flexbox/api/base-adapter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@
* 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 {ElementRef, Renderer2} from '@angular/core';
import {ElementRef} from '@angular/core';

import {BaseFxDirective} from './base';
import {BaseFxDirective, StyleRenderer} from './base';
import {ResponsiveActivation} from './../responsive/responsive-activation';
import {MediaQuerySubscriber} from '../../media-query/media-change';
import {MediaMonitor} from '../../media-query/media-monitor';
Expand Down Expand Up @@ -48,7 +48,7 @@ export class BaseFxDirectiveAdapter extends BaseFxDirective {
constructor(protected _baseKey: string, // non-responsive @Input property name
protected _mediaMonitor: MediaMonitor,
protected _elementRef: ElementRef,
protected _renderer: Renderer2 ) {
protected _renderer: StyleRenderer ) {
super(_mediaMonitor, _elementRef, _renderer);
}

Expand Down
9 changes: 8 additions & 1 deletion src/lib/flexbox/api/base.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ import {ResponsiveActivation, KeyOptions} from '../responsive/responsive-activat
import {MediaMonitor} from '../../media-query/media-monitor';
import {MediaQuerySubscriber} from '../../media-query/media-change';

/**
* To avoid dependencies on full class Renderer2 or Renderer,
* define interface for the only method used...
*/
export interface StyleRenderer {
setStyle(el: any, style: string, value: any): void;
}
/**
* Definition of a css style. Either a property name (e.g. "flex-basis") or an object
* map of property name and value (e.g. {display: 'none', flex-order: 5}).
Expand Down Expand Up @@ -64,7 +71,7 @@ export abstract class BaseFxDirective implements OnDestroy, OnChanges {
*/
constructor(protected _mediaMonitor: MediaMonitor,
protected _elementRef: ElementRef,
protected _renderer: Renderer2) {
protected _renderer: StyleRenderer) {
this._display = this._getDisplayStyle();
}

Expand Down
22 changes: 18 additions & 4 deletions src/lib/flexbox/api/class.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
} from '@angular/core';
import {NgClass} from '@angular/common';

import {StyleRenderer} from './base';
import {BaseFxDirectiveAdapter} from './base-adapter';
import {MediaChange} from '../../media-query/media-change';
import {MediaMonitor} from '../../media-query/media-monitor';
Expand Down Expand Up @@ -110,13 +111,14 @@ export class ClassDirective extends NgClass implements DoCheck, OnChanges, OnDes
/* tslint:enable */
constructor(protected monitor: MediaMonitor,
_iterableDiffers: IterableDiffers, _keyValueDiffers: KeyValueDiffers,
_ngEl: ElementRef, _oldRenderer: Renderer, _renderer: Renderer2) {
_ngEl: ElementRef, _renderer: Renderer) {

// TODO: this should use Renderer2 as well, but NgClass hasn't switched over yet.
super(_iterableDiffers, _keyValueDiffers, _ngEl, _oldRenderer);
super(_iterableDiffers, _keyValueDiffers, _ngEl, _renderer);

this._classAdapter = new BaseFxDirectiveAdapter('class', monitor, _ngEl, _renderer);
this._ngClassAdapter = new BaseFxDirectiveAdapter('ngClass', monitor, _ngEl, _renderer);
let proxy : StyleRenderer = this.buildProxy(_renderer);
this._classAdapter = new BaseFxDirectiveAdapter('class', monitor, _ngEl, proxy);
this._ngClassAdapter = new BaseFxDirectiveAdapter('ngClass', monitor, _ngEl, proxy);
}

// ******************************************************************
Expand Down Expand Up @@ -154,6 +156,18 @@ export class ClassDirective extends NgClass implements DoCheck, OnChanges, OnDes
// Internal Methods
// ******************************************************************

/**
* Build a StyleRenderer method that proxies to the `setElementStyle()` method
* in the old, deprecated Renderer method
*/
protected buildProxy(renderer:Renderer): StyleRenderer {
return {
setStyle : (el: any, style: string, value: any) => {
renderer.setElementStyle(el, style, value);
}
} as StyleRenderer;
}

/**
* Build an mqActivation object that bridges
* mql change events to onMediaQueryChange handlers
Expand Down
22 changes: 16 additions & 6 deletions src/lib/flexbox/api/style.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@ import {
OnDestroy,
DoCheck,
Renderer,
Renderer2,
KeyValueDiffers,
SimpleChanges, OnChanges,
SecurityContext
} from '@angular/core';
import {NgStyle} from '@angular/common';

import {StyleRenderer} from './base';
import {BaseFxDirectiveAdapter} from './base-adapter';
import {BreakPointRegistry} from './../../media-query/breakpoints/break-point-registry';
import {MediaChange} from '../../media-query/media-change';
Expand Down Expand Up @@ -104,11 +104,10 @@ export class StyleDirective extends NgStyle implements DoCheck, OnChanges, OnDes
protected _sanitizer: DomSanitizer,
_differs: KeyValueDiffers,
_ngEl: ElementRef,
_oldRenderer: Renderer,
_renderer: Renderer2) {
_renderer: Renderer) {

// TODO: this should use Renderer2 when the NgStyle signature is switched over to it.
super(_differs, _ngEl, _oldRenderer);
super(_differs, _ngEl, _renderer);

// Build adapter, `cacheInput()` interceptor, and get current inline style if any
this._buildAdapter(monitor, _ngEl, _renderer);
Expand Down Expand Up @@ -183,11 +182,22 @@ export class StyleDirective extends NgStyle implements DoCheck, OnChanges, OnDes
* This adapter manages listening to mediaQuery change events and identifying
* which property value should be used for the style update
*/
protected _buildAdapter(monitor: MediaMonitor, _ngEl: ElementRef, _renderer: Renderer2) {
this._base = new BaseFxDirectiveAdapter('style', monitor, _ngEl, _renderer);
protected _buildAdapter(monitor: MediaMonitor, _ngEl: ElementRef, _renderer: Renderer) {
this._base = new BaseFxDirectiveAdapter('style', monitor, _ngEl, this.makeStyleRenderer(_renderer));
this._buildCacheInterceptor();
}

/**
* Build a StyleRenderer that proxies to the `setElementStyle()` method
* in the old, deprecated Renderer method
*/
protected makeStyleRenderer(renderer:Renderer): StyleRenderer {
return {
setStyle : (el: any, style: string, value: any) => {
renderer.setElementStyle(el, style, value);
}
} as StyleRenderer;
}

/**
* Build intercept to convert raw strings to ngStyleMap
Expand Down
Loading

0 comments on commit 978a897

Please sign in to comment.