Skip to content
This repository has been archived by the owner on Oct 7, 2020. It is now read-only.

Commit

Permalink
fix: Directives inheriting from components for Ivy template type chec…
Browse files Browse the repository at this point in the history
…ks (#2017)

As described in the comments on #1982, Ivy does not appreciate it when directives inherit from components. I found three places where this happens:

* `MdcRippleDirective` extends `MdcRippleComponent`
* `MdcSelectIcon` extends  `MdcIcon`
* `MdcTextFieldIcon` extends `MdcIcon`
I apologize in advance if any of these attempts are not done correctly (I'm well out of my comfort zone here), as well as for any needless change caused by code formatting. Perhaps a more opinionated `.editorconfig` would help.

The last two cases are similar and simpler: as each is apparently designed to augment an `<mdc-icon>` element, I figured that simply eliminating the inheritance could be performed without negative impact.

Ripple was more challenging. As the component has effectively no template, I decided to try to implement both with a single directive, as is described in the comments inside the commit. Initially, I tried defining a type alias for `MdcRippleComponent`, in order to be able to fool any existing code (including the tests). This was unsuccessful, as apparently an actual JS object must exist. Therefore, I had to make a couple of small changes to the tests as well, namely replacing instances of `MdcRippleComponent` with `MdcRippleDirective` and changing the way that the test grabs the TS controller object from `componentInstance` to `injector.get()`. The meat of the tests themselves are unmodified, and hopefully that is acceptable.

I was able to at least build and run an app using Angular 9.0.0-rc0 out of the box using this branch, and hope that it is at least of some use to others, if only as a stepping stone towards being done more skillfully by somebody more intimately familiar with Angular internals than I.

Thank you for this effort, much appreciated @rapropos !!
  • Loading branch information
rapropos authored and trimox committed Nov 5, 2019
1 parent 1882912 commit 932eac6
Show file tree
Hide file tree
Showing 6 changed files with 110 additions and 115 deletions.
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@
"url": "https://github.com/trimox/angular-mdc-web.git"
},
"license": "MIT",
"version": "3.2.1",
"version": "4.0.0-SNAPSHOT",
"engines": {
"node": ">= 9.11.1"
},
"requiredAngularVersion": "^8.0.0 || ^9.0.0-0",
"requiredAngularVersion": "^8.0.0 || ^9.0.0-rc.0",
"scripts": {
"build": "gulp web:build",
"build:release": "gulp web:build-release",
Expand Down
5 changes: 2 additions & 3 deletions packages/ripple/ripple-module.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
import {NgModule} from '@angular/core';

import {
MdcRippleComponent,
MdcRippleDirective
} from './ripple';

@NgModule({
exports: [MdcRippleComponent, MdcRippleDirective],
declarations: [MdcRippleComponent, MdcRippleDirective],
exports: [MdcRippleDirective],
declarations: [MdcRippleDirective],
})
export class MdcRippleModule { }
201 changes: 100 additions & 101 deletions packages/ripple/ripple.ts
Original file line number Diff line number Diff line change
@@ -1,112 +1,111 @@
import {
AfterViewInit,
ChangeDetectionStrategy,
Component,
Directive,
ElementRef,
Input,
OnDestroy
} from '@angular/core';
import {AfterViewInit, Directive, ElementRef, Input, OnDestroy} from '@angular/core';
import {coerceBooleanProperty} from '@angular/cdk/coercion';

import {MdcRipple, MDCRippleCapableSurface} from './ripple.service';

@Component({
selector: 'mdc-ripple, [mdc-ripple]',
template: '<ng-content></ng-content>',
providers: [MdcRipple],
changeDetection: ChangeDetectionStrategy.OnPush
// rapropos 2019.11.02
// Ivy doesn't allow directives to inherit from components, so this merges
// what used to be the two. see the constructor for more information

@Directive({
selector: 'mdc-ripple, [mdc-ripple], [mdcRipple]',
providers: [MdcRipple],
})
export class MdcRippleComponent implements AfterViewInit, OnDestroy, MDCRippleCapableSurface {
_root!: Element;

get ripple(): MdcRipple {
return this._ripple;
}

@Input()
get attachTo(): any {
return this._attachTo;
}
set attachTo(element: any) {
if (this._attachTo) {
this._attachTo.classList.remove('mdc-ripple-surface');
export class MdcRippleDirective implements AfterViewInit, OnDestroy, MDCRippleCapableSurface {
_root!: Element;

get ripple(): MdcRipple {
return this._ripple;
}
this._attachTo = element;
if (this._attachTo) {
this._attachTo.classList.add('mdc-ripple-surface');

@Input()
get attachTo(): any {
return this._attachTo;
}
}
private _attachTo: any;

@Input()
get primary(): boolean {
return this._primary;
}
set primary(value: boolean) {
this._primary = coerceBooleanProperty(value);
this._primary ? this.attachTo.classList.add('mdc-ripple-surface--primary')
: this.attachTo.classList.remove('mdc-ripple-surface--primary');
}
private _primary: boolean = false;

@Input()
get secondary(): boolean {
return this._secondary;
}
set secondary(value: boolean) {
this._secondary = coerceBooleanProperty(value);
this._secondary ? this.attachTo.classList.add('mdc-ripple-surface--accent')
: this.attachTo.classList.remove('mdc-ripple-surface--accent');
}
private _secondary: boolean = false;

@Input()
get disabled(): boolean {
return this._disabled;
}
set disabled(value: boolean) {
this._disabled = coerceBooleanProperty(value);
}
private _disabled: boolean = false;

@Input()
get unbounded(): boolean {
return this._unbounded;
}
set unbounded(value: boolean) {
this._unbounded = coerceBooleanProperty(value);
}
protected _unbounded: boolean = false;

constructor(
private _ripple: MdcRipple,
public elementRef: ElementRef<HTMLElement>) {
this._root = this.elementRef.nativeElement;
}

ngAfterViewInit(): void {
this._ripple = new MdcRipple(this.elementRef);
this._ripple.init();
}

ngOnDestroy(): void {
this.ripple.destroy();
}
}

@Directive({
selector: '[mdcRipple]',
providers: [MdcRipple]
})
export class MdcRippleDirective extends MdcRippleComponent {
constructor(
_ripple: MdcRipple,
elementRef: ElementRef) {
set attachTo(element: any) {
if (this._attachTo) {
this._attachTo.classList.remove('mdc-ripple-surface');
}
this._attachTo = element;
if (this._attachTo) {
this._attachTo.classList.add('mdc-ripple-surface');
}
}

private _attachTo: any;

@Input()
get primary(): boolean {
return this._primary;
}

set primary(value: boolean) {
this._primary = coerceBooleanProperty(value);
this._primary ? this.attachTo.classList.add('mdc-ripple-surface--primary')
: this.attachTo.classList.remove('mdc-ripple-surface--primary');
}

private _primary: boolean = false;

@Input()
get secondary(): boolean {
return this._secondary;
}

set secondary(value: boolean) {
this._secondary = coerceBooleanProperty(value);
this._secondary ? this.attachTo.classList.add('mdc-ripple-surface--accent')
: this.attachTo.classList.remove('mdc-ripple-surface--accent');
}

super(_ripple, elementRef);
private _secondary: boolean = false;

this._unbounded = true;
this.elementRef.nativeElement.setAttribute('data-mdc-ripple-is-unbounded', '');
}
@Input()
get disabled(): boolean {
return this._disabled;
}

set disabled(value: boolean) {
this._disabled = coerceBooleanProperty(value);
}

private _disabled: boolean = false;

@Input()
get unbounded(): boolean {
return this._unbounded;
}

set unbounded(value: boolean) {
this._unbounded = coerceBooleanProperty(value);
}

protected _unbounded: boolean = false;

constructor(
private _ripple: MdcRipple,
public elementRef: ElementRef<HTMLElement>) {
this._root = this.elementRef.nativeElement;

// rapropos 2019.11.02
// the difference between the old component and directive was boundedness
// and the presence of the 'data-mdc-ripple-is-unbounded' attribute. we
// recreate that here by checking our tag name. if it is `MDC-RIPPLE',
// then we are acting as the component did. otherwise, we act as the directive
if (this._root.tagName !== 'MDC-RIPPLE') {
this.unbounded = true;
this._root.setAttribute('data-mdc-ripple-is-unbounded', '');
}
}

ngAfterViewInit(): void {
this._ripple = new MdcRipple(this.elementRef);
this._ripple.init();
}

ngOnDestroy(): void {
this.ripple.destroy();
}
}

3 changes: 1 addition & 2 deletions packages/select/select-icon.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import {Directive} from '@angular/core';
import {MdcIcon} from '@angular-mdc/web/icon';

@Directive({
selector: '[mdcSelectIcon]',
exportAs: 'mdcSelectIcon',
host: { 'class': 'mdc-select__icon' }
})
export class MdcSelectIcon extends MdcIcon { }
export class MdcSelectIcon { }
3 changes: 1 addition & 2 deletions packages/textfield/text-field-icon.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,13 @@ import {
Input
} from '@angular/core';
import {coerceBooleanProperty} from '@angular/cdk/coercion';
import {MdcIcon} from '@angular-mdc/web/icon';

@Directive({
selector: '[mdcTextFieldIcon]',
exportAs: 'mdcTextFieldIcon',
host: { 'class': 'mdc-text-field__icon' }
})
export class MdcTextFieldIcon extends MdcIcon {
export class MdcTextFieldIcon {
@Input()
get leading(): boolean { return this._leading; }
set leading(value: boolean) {
Expand Down
9 changes: 4 additions & 5 deletions test/ripple/ripple.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import {dispatchMouseEvent} from '../testing/dispatch-events';

import {
MdcRippleModule,
MdcRippleComponent,
MdcRippleDirective
} from '@angular-mdc/web';

Expand All @@ -32,16 +31,16 @@ describe('MdcRippleComponent', () => {
describe('basic behaviors', () => {
let testDebugElement: DebugElement;
let testNativeElement: HTMLElement;
let testInstance: MdcRippleComponent;
let testInstance: MdcRippleDirective;
let testComponent: SimpleTest;

beforeEach(() => {
fixture = TestBed.createComponent(SimpleTest);
fixture.detectChanges();

testDebugElement = fixture.debugElement.query(By.directive(MdcRippleComponent));
testDebugElement = fixture.debugElement.query(By.directive(MdcRippleDirective));
testNativeElement = testDebugElement.nativeElement;
testInstance = testDebugElement.componentInstance;
testInstance = testDebugElement.injector.get(MdcRippleDirective);
testComponent = fixture.debugElement.componentInstance;
});

Expand Down Expand Up @@ -137,7 +136,7 @@ describe('MdcRippleComponent', () => {

testDebugElement = fixture.debugElement.query(By.directive(MdcRippleDirective));
testNativeElement = testDebugElement.nativeElement;
testInstance = testDebugElement.componentInstance;
testInstance = testDebugElement.injector.get(MdcRippleDirective);
testComponent = fixture.debugElement.componentInstance;
});

Expand Down

0 comments on commit 932eac6

Please sign in to comment.