Skip to content
This repository was archived by the owner on Sep 29, 2020. It is now read-only.

Added typings from @types, typings should now ship with the package. #129

Merged
merged 3 commits into from
Jul 14, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
116 changes: 116 additions & 0 deletions index.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
// Type definitions for core-decorators.js 0.10
// Project: https://github.com/jayphelps/core-decorators.js
// Definitions by: Qubo <https://github.com/tkqubo>
// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped
// TypeScript Version: 2.1

export interface ClassDecorator {
Copy link
Collaborator

@BurtHarris BurtHarris Jul 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typescript comes with library file lib.es5.d.ts, which contains *Decorator definitions similar to these. Is there some reason to duplicate them here?

The typescript provided definitions read as follows.

declare type ClassDecorator = <TFunction extends Function>(target: TFunction) => TFunction | void;
declare type PropertyDecorator = (target: Object, propertyKey: string | symbol) => void;
declare type MethodDecorator = <T>(target: Object, propertyKey: string | symbol, descriptor: TypedPropertyDescriptor<T>) => TypedPropertyDescriptor<T> | void;
declare type ParameterDecorator = (target: Object, propertyKey: string | symbol, parameterIndex: number) => void;

Are these the definitions that will change due to changes made in advancing the proposal to stage 2? It seems like if we leverage the TypeScript provided definitions, then we'll get updates when TypeScript changes to adopt to the evolving standard. This might be an advantage, even if it breaks code when the change happens.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup I agree with that, no point in doubling the number of type rules if they are provided to us already. Similarly the Console is defined already for us in lib.dom.d.ts so I've gone and removed that extraneous definition as well.

<TFunction extends Function>(target: TFunction): TFunction | void;
}

export interface ParameterDecorator {
(target: object, propertyKey: string | symbol, parameterIndex: number): void;
}

export interface PropertyDecorator {
(target: object, propertyKey: string | symbol): void;
}

export interface MethodDecorator {
<T>(target: object, propertyKey: string | symbol, descriptor: TypedPropertyDescriptor<T>): TypedPropertyDescriptor<T> | void;
}

export interface PropertyOrMethodDecorator extends MethodDecorator, PropertyDecorator {
(target: object, propertyKey: string | symbol): void;
}

export interface Deprecate extends MethodDecorator {
(message?: string, option?: DeprecateOption): MethodDecorator;
}

export interface DeprecateOption {
url: string;
}

export interface ThrottleOptions {
/** allows to trigger function on the leading. */
leading?: boolean;
/** allows to trigger function on the trailing edge of the wait interval. */
trailing?: boolean;
}

export interface Console {
log(message?: any, ...optionalParams: any[]): void;
time(timerName?: string): void;
timeEnd(timerName?: string): void;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think ThrottleOptions can go now that we've removed throttle.


/**
* Forces invocations of this function to always have this refer to the class instance,
* even if the function is passed around or would otherwise lose its this context. e.g. var fn = context.method;
*/
export const autobind: Function;
/**
* Marks a property or method as not being writable.
*/
export const readonly: PropertyOrMethodDecorator;
/**
* Checks that the marked method indeed overrides a function with the same signature somewhere on the prototype chain.
*/
export const override: MethodDecorator;
/**
* Calls console.warn() with a deprecation message. Provide a custom message to override the default one. You can also provide an options hash with a url, for further reading.
*/
export const deprecate: Deprecate;
/**
* Calls console.warn() with a deprecation message. Provide a custom message to override the default one. You can also provide an options hash with a url, for further reading.
*/
export const deprecated: Deprecate;
/**
* Creates a new debounced function which will be invoked after wait milliseconds since the time it was invoked. Default timeout is 300 ms.
*/
export function debounce(wait: number): MethodDecorator;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As debounce, throttle and mixin are deprecated for this package, do we really want to include them in the TypeScript declarations?

At a minimum, the JSdoc @deprecated annotation should probably be added to their doc comments.

@jayphelps do you have an opinion on this choice?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I agree we should omit the type signatures for them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My only small complaint with not including the type declaration for these methods would be that someone that ignores the fact these methods are deprecated are going to think there is something wrong with their typings file since the compiler is going to tell them there are no definitions available.

Not a major issue though. Happy to leave these definitions out.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guess my thought was I want to discourage their use, providing type signatures seems to encourage them, even if unintentionally. Let's not include them pls. Thanks! 🤘

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider them history 🔥

/**
* Creates a new throttled function which will be invoked in every wait milliseconds. Default timeout is 300 ms.
*/
export function throttle(wait: number, options?: ThrottleOptions): MethodDecorator;
/**
* Suppresses any JavaScript console.warn() call while the decorated function is called. (i.e. on the stack)
*/
export const suppressWarnings: MethodDecorator;
/**
* Marks a property or method as not being enumerable.
*/
export const nonenumerable: PropertyOrMethodDecorator;
/**
* Marks a property or method as not being writable.
*/
export const nonconfigurable: PropertyOrMethodDecorator;
/**
* Initial implementation included, likely slow. WIP.
*/
export const memoize: MethodDecorator;
/**
* Immediately applies the provided function and arguments to the method, allowing you to wrap methods with arbitrary helpers like those provided by lodash.
* The first argument is the function to apply, all further arguments will be passed to that decorating function.
*/
export function decorate(func: Function, ...args: any[]): MethodDecorator;
/**
* Prevents a property initializer from running until the decorated property is actually looked up.
* Useful to prevent excess allocations that might otherwise not be used, but be careful not to over-optimize things.
*/
export const lazyInitialize: PropertyDecorator;
/**
* Mixes in all property descriptors from the provided Plain Old JavaScript Objects (aka POJOs) as arguments.
* Mixins are applied in the order they are passed, but do not override descriptors already on the class, including those inherited traditionally.
*/
export function mixin(...mixins: any[]): ClassDecorator;
/**
* Mixes in all property descriptors from the provided Plain Old JavaScript Objects (aka POJOs) as arguments.
* Mixins are applied in the order they are passed, but do not override descriptors already on the class, including those inherited traditionally.
*/
export function mixins(...mixins: any[]): ClassDecorator;
/**
* Uses console.time and console.timeEnd to provide function timings with a unique label whose default prefix is ClassName.method. Supply a first argument to override the prefix:
*/
export function time(label: string, console?: Console): MethodDecorator;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: It's usually good to end a file with a newline.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies tslint usually auto-formats my work for me I need to have a word with it.

4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,14 @@
"main": "lib/core-decorators.js",
"module": "es/core-decorators.js",
"jsnext:main": "es/core-decorators.js",
"typings": "./index.d.ts",
"files": [
"es",
"lib",
"src",
"README.md",
"LICENSE"
"LICENSE",
"index.d.ts"
],
"scripts": {
"prepublish": "npm run build",
Expand Down