Skip to content

Commit

Permalink
Rewrite deprecation decorators (#3447)
Browse files Browse the repository at this point in the history
* chore(*): Remove DeprecateClass util method (#2915)

* chore(datepicker): Replace deprecated selector (#2915)

* chore(*): Rewrite deprecation decorator for properties and methods (#2915)

* chore(*): Fix lint warnings (#2915)

* chore(*): Invoke original getter/setter (#2915)

Invoke original getter/setter for deprecated properties.

* feat(*): Improve property deprecation messages (#2915)

* chore(*): Fix lint errors (#2915)

* chore(datepicker): Revert - Replace deprecated selector (#2915)

This reverts commit a2e731b.

* chore(datepicker): Restore removed selector (#2915)

* chore(datepicker): Show deprecation message when necessary (#2915)

* chore(datepicker): Replace deprecated selector (#2915)
  • Loading branch information
bkulov authored and rkaraivanov committed Jan 9, 2019
1 parent 5ad26bf commit c3ae627
Show file tree
Hide file tree
Showing 12 changed files with 128 additions and 29 deletions.
27 changes: 27 additions & 0 deletions .github/CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,33 @@ If the bug fix or new feature development requires changes to released public AP
2. Add a `BREAKING CHANGE:` section to the commit message body or footer. See https://www.conventionalcommits.org
3. Check if the change can be migrated by `ng update` schematics and add to the project migrations. See [Update Migrations wiki](https://github.com/IgniteUI/igniteui-angular/wiki/Update-Migrations) for available functionality and instructions.

## Deprecating selectors
When deprecating selectors the following code should be placed inside `OnInit` method of the class the selector belongs to:
`
import { isDevMode } from '@angular/core';
...
constructor(..., private element: ElementRef) {}
...
if (isDevMode() && this.element.nativeElement.tagName === 'your deprecated selector in upper case') {
console.log('your deprecation message');
}
`

Write migrations.

## Deprecating methods
When a method is deprecated a few steps have to be done:
1. Add deprecation warning message by decorating the method with `@DeprecateMethod` decorator from `deprecateDecorators.ts` file.
2. Ensure that the deprecated method is no longer used in IgniteUI for Angular codebase, samples and documentation snippets.
3. Write migrations.

## Deprecating class properties
When a class property is deprecated a few steps have to be done:
1. Add deprecation warning message by decorating the property with `@DeprecateProperty` decorator from `deprecateDecorators.ts` file.
2. Ensure that the deprecated property is no longer used in IgniteUI for Angular codebase, samples and documentation snippets.
3. Write migrations.

NOTE: TypeScript disallows decorating both the get and set accessor for a single member. Instead, all decorators for the member must be applied to the first accessor specified in document order. This is because decorators apply to a Property Descriptor, which combines both the get and set accessor, not each declaration separately.

# Testing a PR
In order to test a pull request that is awaiting test, perform the following actions.
Expand Down
89 changes: 79 additions & 10 deletions projects/igniteui-angular/src/lib/core/deprecateDecorators.ts
Original file line number Diff line number Diff line change
@@ -1,27 +1,96 @@
import { isDevMode } from '@angular/core';

/**
* @hidden
*/
export function DeprecateClass(message: string): ClassDecorator {
return (constructor: any) => {
console.warn(constructor.name + ': ' + message);
export function DeprecateMethod(message: string): MethodDecorator {
let isMessageShown = false;

return function (target: any, key: string, descriptor: PropertyDescriptor) {
if (descriptor && descriptor.value) {
const originalMethod = descriptor.value;

descriptor.value = function () {
const targetName = typeof target === 'function' ? target.name : target.constructor.name;
isMessageShown = showMessage(`${targetName}.${key}: ${message}`, isMessageShown);

return originalMethod.call(this, arguments);
};

return descriptor;
}
};
}

/**
* @hidden
*/
export function DeprecateMethod(message: string): MethodDecorator {
return (constructor: any) => {
console.warn(constructor.constructor.name + ': ' + message);
export function DeprecateProperty(message: string): PropertyDecorator {
return function(target: any, key: string) {
let isMessageShown = false;
const messageToDisplay = `${target.constructor.name}.${key}: ${message}`;

// if the target already has the property defined
const originalDescriptor = Object.getOwnPropertyDescriptor(target, key);
if (originalDescriptor) {
let getter, setter;
getter = originalDescriptor.get;
setter = originalDescriptor.set;

if (getter) {
originalDescriptor.get = function() {
isMessageShown = showMessage(messageToDisplay, isMessageShown);
return getter.call(this);
};
}

if (setter) {
originalDescriptor.set = function (value) {
isMessageShown = showMessage(messageToDisplay, isMessageShown);
setter.call(this, value);
};
}

return originalDescriptor;
}

// the target doesn't contain a descriptor for that property, so create one
// use backing field to set/get the value of the property to ensure there won't be infinite recursive calls
const newKey = generateUniqueKey(target, key);
return Object.defineProperty(target, key, {
configurable: true,
enumerable: true,
set: function(value) {
isMessageShown = showMessage(messageToDisplay, isMessageShown);
this[newKey] = value;
},
get: function() {
isMessageShown = showMessage(messageToDisplay, isMessageShown);
return this[newKey];
}
});
};
}

/**
* @hidden
*/
export function DeprecateProperty(message: string): PropertyDecorator {
return (constructor: any) => {
console.warn(constructor.constructor.name + ': ' + message);
};
function generateUniqueKey(target: any, key: string): string {
let newKey = '_' + key;
while (target.hasOwnProperty(newKey)) {
newKey = '_' + newKey;
}

return newKey;
}

/**
* @hidden
*/
function showMessage(message: string, isMessageShown: boolean): boolean {
if (!isMessageShown && isDevMode()) {
console.warn(message);
}

return true;
}
4 changes: 2 additions & 2 deletions projects/igniteui-angular/src/lib/date-picker/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,14 @@ The DatePicker also supports binding through `ngModel` if two-way date-bind is n

The DatePicker input group could be retemplated.
```html
<igx-datePicker>
<igx-date-picker>
<ng-template igxDatePickerTemplate let-openDialog="openDialog" let-value="value" let-displayData="displayData">
<igx-input-group (click)="openDialog()">
<label igxLabel>Date</label>
<input igxInput [value]="displayData"/>
</igx-input-group>
</ng-template>
</igx-datePicker>
</igx-date-picker>
```

# API
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -435,14 +435,14 @@ export class IgxDatePickerNgModelComponent {

@Component({
template: `
<igx-datePicker>
<igx-date-picker>
<ng-template igxDatePickerTemplate let-displayData="displayData">
<igx-input-group>
<label igxLabel>Date</label>
<input igxInput [value]="displayData"/>
</igx-input-group>
</ng-template>
</igx-datePicker>
</igx-date-picker>
`
})
export class IgxDatePickerRetemplatedComponent {}
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ import {
HostListener,
ElementRef,
TemplateRef,
Directive
Directive,
isDevMode
} from '@angular/core';
import { ControlValueAccessor, NG_VALUE_ACCESSOR } from '@angular/forms';
import {
Expand All @@ -33,7 +34,6 @@ import { Subject } from 'rxjs';
import { takeUntil } from 'rxjs/operators';
import { IgxOverlayOutletDirective } from '../directives/toggle/toggle.directive';
import { OverlaySettings } from '../services';
import { DeprecateClass } from '../core/deprecateDecorators';
import { DateRangeDescriptor } from '../core/dates/dateRange';
import { EditorProvider } from '../core/edit-provider';

Expand Down Expand Up @@ -77,7 +77,6 @@ let NEXT_ID = 0;
styles: [':host {display: block;}'],
templateUrl: 'date-picker.component.html'
})
@DeprecateClass('\'igx-datePicker\' selector is deprecated. Use \'igx-date-picker\' selector instead.')
export class IgxDatePickerComponent implements ControlValueAccessor, EditorProvider, OnInit, OnDestroy {
/**
*An @Input property that sets the value of `id` attribute. If not provided it will be automatically generated.
Expand Down Expand Up @@ -446,7 +445,7 @@ export class IgxDatePickerComponent implements ControlValueAccessor, EditorProvi

@ViewChild(IgxInputDirective) protected input: IgxInputDirective;

constructor(private resolver: ComponentFactoryResolver) { }
constructor(private resolver: ComponentFactoryResolver, private element: ElementRef) { }

/**
*Method that sets the selected date.
Expand Down Expand Up @@ -486,6 +485,10 @@ export class IgxDatePickerComponent implements ControlValueAccessor, EditorProvi
public ngOnInit(): void {
this.alert.onOpen.pipe(takeUntil(this.destroy$)).subscribe((ev) => this._focusCalendarDate());
this.alert.toggleRef.onClosed.pipe(takeUntil(this.destroy$)).subscribe((ev) => this.handleDialogCloseAction());

if (isDevMode() && this.element.nativeElement.tagName === 'IGX-DATEPICKER') {
console.warn('IgxDatePickerComponent: \'igx-datePicker\' selector is deprecated. Use \'igx-date-picker\' selector instead.');
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { filter, takeUntil } from 'rxjs/operators';
import { Subscription, Subject, MonoTypeOperatorFunction } from 'rxjs';
import { OverlayCancelableEventArgs } from '../../services/overlay/utilities';
import { CancelableEventArgs } from '../../core/utils';
import { DeprecateProperty } from '../../core/deprecateDecorators';

@Directive({
exportAs: 'toggle',
Expand Down Expand Up @@ -323,6 +324,8 @@ export class IgxToggleActionDirective implements OnInit {
* let closesOnOutsideClick = this.toggle.closeOnOutsideClick;
* ```
*/
@Input()
@DeprecateProperty(`igxToggleAction 'closeOnOutsideClick' input is deprecated. Use 'overlaySettings' input object instead.`)
public get closeOnOutsideClick(): boolean {
return this._closeOnOutsideClick;
}
Expand All @@ -332,9 +335,7 @@ export class IgxToggleActionDirective implements OnInit {
* <div igxToggleAction [closeOnOutsideClick]="'true'"></div>
* ```
*/
@Input()
public set closeOnOutsideClick(v: boolean) {
console.warn(`igxToggleAction 'closeOnOutsideClick' input is deprecated. Use 'overlaySettings' input object instead.`);
this._closeOnOutsideClick = v;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
<igx-checkbox (change)="editValue = $event.checked" [value]="editValue" [checked]="editValue" [igxFocus]="focused" [disableRipple]="true"></igx-checkbox>
</ng-container>
<ng-container *ngIf="column.dataType === 'date'">
<igx-datePicker (onSelection)="editValue = $event" [value]="editValue" [igxFocus]="focused" [labelVisibility]="false"></igx-datePicker>
<igx-date-picker (onSelection)="editValue = $event" [value]="editValue" [igxFocus]="focused" [labelVisibility]="false"></igx-date-picker>
</ng-container>
</ng-template>
<ng-container *ngTemplateOutlet="template; context: context">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
</ng-template>

<ng-template #defaultDateUI>
<igx-datePicker tabindex="0" [(ngModel)]="value" [locale]="locale" (onClose)="datePickerClose()">
<igx-date-picker tabindex="0" [(ngModel)]="value" [locale]="locale" (onClose)="datePickerClose()">
<ng-template igxDatePickerTemplate let-openDialog="openDialog" let-displayData="displayData">
<igx-input-group type="box" [displayDensity]="'compact'" [supressInputAutofocus]="true">
<igx-prefix #inputGroupPrefix
Expand All @@ -60,7 +60,7 @@
</igx-suffix>
</igx-input-group>
</ng-template>
</igx-datePicker>
</igx-date-picker>
</ng-template>

<ng-container *ngTemplateOutlet="template; context: { $implicit: this }"></ng-container>
Expand Down
2 changes: 1 addition & 1 deletion projects/igniteui-angular/src/lib/grids/grid/cell.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ describe('IgxGrid - Cell component', () => {
await wait();

expect(cell.inEditMode).toBe(true);
const datePicker = cellDomDate.query(By.css('igx-datepicker')).componentInstance;
const datePicker = cellDomDate.query(By.css('igx-date-picker')).componentInstance;
expect(datePicker).toBeDefined();

datePicker.selectDate(selectedDate);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
<igx-checkbox (change)="editValue = $event.checked" [value]="editValue" [checked]="editValue" [disableRipple]="true"></igx-checkbox>
</ng-container>
<ng-container *ngIf="column.dataType === 'date'">
<igx-datePicker (onSelection)="editValue = $event" [value]="editValue" [labelVisibility]="false"></igx-datePicker>
<igx-date-picker (onSelection)="editValue = $event" [value]="editValue" [labelVisibility]="false"></igx-date-picker>
</ng-container>
</ng-template>
<ng-container *ngIf="!inEditMode">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import {
ViewChildren
} from '@angular/core';
import { IgxBadgeModule } from '../badge/badge.component';
import { DeprecateClass } from '../core/deprecateDecorators';
import { IgxIconModule } from '../icon/index';

export interface ISelectTabEventArgs {
Expand Down
6 changes: 3 additions & 3 deletions src/app/date-picker/date-picker.sample.html
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<h4 class="sample-title">Default Date Picker.</h4>
<p class="sample-description">Detailed description to be added.</p>
<div class="preview">
<igx-datePicker #datePicker cancelButtonLabel="cancel" todayButtonLabel="today"></igx-datePicker>
<igx-date-picker #datePicker cancelButtonLabel="cancel" todayButtonLabel="today"></igx-date-picker>
<button igxButton igxRipple type="raised" (click)="deselect()">Deselect date</button>
</div>
</article>
Expand All @@ -27,15 +27,15 @@ <h4 class="sample-title">Date Picker with passed date and custom formatter.</h4>
<h4 class="sample-title">Date Picker with retemplated input group.</h4>
<p class="sample-description">Detailed description to be added.</p>
<div class="preview">
<igx-datePicker [value]="date" [vertical]="true" [formatter]="formatter">
<igx-date-picker [value]="date" [vertical]="true" [formatter]="formatter">
<ng-template igxDatePickerTemplate let-openDialog="openDialog" let-value="value" let-displayData="displayData">
<igx-input-group (click)="openDialog()">
<label igxLabel>Date</label>
<input class="igx-date-picker__input-date" igxInput [value]="displayData" />
</igx-input-group>
<label>{{value}}</label>
</ng-template>
</igx-datePicker>
</igx-date-picker>
</div>
</article>
</section>
Expand Down

0 comments on commit c3ae627

Please sign in to comment.