Skip to content

Commit

Permalink
Rewrite deprecation decorators (#3453)
Browse files Browse the repository at this point in the history
chore(*): Rewrite deprecation decorator for properties and methods (#2915)
  • Loading branch information
bkulov authored Jan 8, 2019
1 parent cf60f6f commit ad535e2
Show file tree
Hide file tree
Showing 5 changed files with 112 additions and 16 deletions.
24 changes: 24 additions & 0 deletions .github/CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,30 @@ 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';
...
if (isDevMode()) {
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;
}
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 @@ -484,6 +483,10 @@ export class IgxDatePickerComponent implements ControlValueAccessor, EditorProvi
*@hidden
*/
public ngOnInit(): void {
if (isDevMode()) {
console.log('\'igx-datePicker\' selector is deprecated. Use \'igx-date-picker\' selector instead.');
}

this.alert.onOpen.pipe(takeUntil(this.destroy$)).subscribe((ev) => this._focusCalendarDate());
this.alert.toggleRef.onClosed.pipe(takeUntil(this.destroy$)).subscribe((ev) => this.handleDialogCloseAction());
}
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, OperatorFunction, Subject } 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 @@ -322,6 +323,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 @@ -331,9 +334,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 @@ -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

0 comments on commit ad535e2

Please sign in to comment.