Add `bindMethods()` method and a `mixin()` decorator (TS) #37

stroncium · 2019-03-06T14:56:26Z

Fixes #30

stroncium · 2019-03-06T14:59:08Z

/**
 * Bind a `methodNames` or all `Emittery` methods if `methodNames` not defined into `target` object.
 */
bindMethods(target: Object, methodNames?: string[]): void;

/**
 * Returns a decorator which mixins `Emittery` as property `emitteryPropertyName` and `methodNames` or all `Emittery` methods if `methodNames` not defined into target class.
 */
static mixin(emitteryPropertyName: string, methodNames?: string[]): Function;

Solves #30

sindresorhus · 2019-06-12T02:43:54Z

Emittery.d.ts

@@ -76,6 +76,16 @@ declare class Emittery {
 	 * specified.
 	 */
 	listenerCount(eventName?: string): number;
+
+	/**
+	 * Bind a `methodNames` or all `Emittery` methods if `methodNames` not defined into `target` object.


Suggested change

* Bind a `methodNames` or all `Emittery` methods if `methodNames` not defined into `target` object.

* Bind the given `methodNames`, or all `Emittery` methods if `methodNames` is not defined, into the `target` object.

Can you include a usage example.

https://github.com/sindresorhus/typescript-definition-style-guide#documentation

Emittery.d.ts

sindresorhus · 2019-06-12T02:46:06Z

Emittery.d.ts

+	/**
+	 * Returns a decorator which mixins `Emittery` as property `emitteryPropertyName` and `methodNames` or all `Emittery` methods if `methodNames` not defined into target class.
+	 */
+	static mixin(emitteryPropertyName: string, methodNames?: string[]): Function;


Can you move this to the top? I prefer having static properties first.

Suggested change

static mixin(emitteryPropertyName: string, methodNames?: string[]): Function;

static mixin(emitteryPropertyName: string, methodNames?: readonly string[]): Function;

Is there any way to make the type more specific than Function?

It is not supposed to be more specific than function, I guess. There shouldn't be any type checks on decorators value. It is pretty hard to misuse and it's not supposed to be used directly in TS anyway.

Also, as far as I understand it will require to type function as all possible numbers of arguments of constructor. And do we really need this complication for absurd level check?

You're right. Probably not worth it.

sindresorhus · 2019-06-12T02:48:05Z

Emittery.d.ts

+	bindMethods(target: Object, methodNames?: string[]): void;
+
+	/**
+	 * Returns a decorator which mixins `Emittery` as property `emitteryPropertyName` and `methodNames` or all `Emittery` methods if `methodNames` not defined into target class.


Suggested change

* Returns a decorator which mixins `Emittery` as property `emitteryPropertyName` and `methodNames` or all `Emittery` methods if `methodNames` not defined into target class.

* Returns a [decorator](https://github.com/tc39/proposal-decorators) which mixes in `Emittery` as a property named by `emitteryPropertyName` and `methodNames`, or all `Emittery` methods if `methodNames` is not defined, into target class.

It does not conform to this proposal. (Hint: it is a proposal, it is not implemented anywhere yet.)

index.js

sindresorhus · 2019-06-12T02:52:12Z

index.js

+		}
+	}
+
+	static mixin(emitteryPropertyName, methodNames) {


Is this following the latest decorator spec? https://github.com/tc39/proposal-decorators

It is not a spec, it is a proposal. There is no implementation which can handle decorators implemented in such a way yet. I followed the typescript implementation of decorators.

Alright. But it needs to be clearly documented in the type and readme that it follows the TypeScript implementation.

I described it only for typescript part of readme so far(as using it from JS is weird anyway atm).

test/_run.js

index.js

sindresorhus · 2019-06-12T02:55:45Z

You also need to document them in the readme. Should use the same text and examples as the TS definition, to make it easy to keep them in sync in the future.

stroncium · 2019-06-14T06:42:16Z

merged master in

Andykmcc · 2019-09-12T22:21:07Z

@sindresorhus can you cut a new release and push it to npm? github master source and docs include this feature but they haven't been pushed to NPM.

sindresorhus · 2019-09-17T14:28:46Z

Done: https://github.com/sindresorhus/emittery/releases/tag/v0.5.0

glasser · 2020-01-24T19:57:51Z

It seems to me that this feature is not really TypeScript-specific, right? Should its docs in the readme be in its own ## instead of nested under the TypeScript section?

sindresorhus changed the title ~~added bindMethods and mixin, solves #30~~ Add bindMethods() method and a mixin() decorator Jun 12, 2019

sindresorhus requested changes Jun 12, 2019

View reviewed changes

sindresorhus reviewed Jun 12, 2019

View reviewed changes

index.js Outdated Show resolved Hide resolved

bindMethods and mixin

6b8cf8d

stroncium force-pushed the bind-methods branch from 9ace582 to 6b8cf8d Compare June 14, 2019 06:21

Yanis Benson added 2 commits June 14, 2019 09:25

ws

d99e954

refactoring

32c4c57

Yanis Benson and others added 6 commits June 15, 2019 05:25

error style

efd6565

linter, readonly

e644f73

Update index.d.ts

c5bfad5

Update index.d.ts

2c38b76

Update index.js

3ca82e7

Update readme.md

02b4d7b

sindresorhus changed the title ~~Add bindMethods() method and a mixin() decorator~~ Add bindMethods() method and a mixin() decorator (TS) Jun 15, 2019

sindresorhus merged commit 96a2cb8 into sindresorhus:master Jun 15, 2019

snyk-bot mentioned this pull request Dec 4, 2019

[Snyk] Upgrade emittery from 0.4.1 to 0.5.1 dyna-dot/testcafe#6

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `bindMethods()` method and a `mixin()` decorator (TS) #37

Add `bindMethods()` method and a `mixin()` decorator (TS) #37

stroncium commented Mar 6, 2019 •

edited by sindresorhus

stroncium commented Mar 6, 2019

sindresorhus Jun 12, 2019

sindresorhus Jun 12, 2019

sindresorhus Jun 12, 2019

sindresorhus Jun 12, 2019

stroncium Jun 14, 2019 •

edited

sindresorhus Jun 14, 2019

sindresorhus Jun 12, 2019

stroncium Jun 14, 2019

sindresorhus Jun 12, 2019

stroncium Jun 14, 2019

sindresorhus Jun 14, 2019

stroncium Jun 15, 2019

sindresorhus commented Jun 12, 2019 •

edited

stroncium commented Jun 14, 2019 •

edited

Andykmcc commented Sep 12, 2019

sindresorhus commented Sep 17, 2019

glasser commented Jan 24, 2020

	* Bind a `methodNames` or all `Emittery` methods if `methodNames` not defined into `target` object.
	* Bind the given `methodNames`, or all `Emittery` methods if `methodNames` is not defined, into the `target` object.

	static mixin(emitteryPropertyName: string, methodNames?: string[]): Function;
	static mixin(emitteryPropertyName: string, methodNames?: readonly string[]): Function;

	* Returns a decorator which mixins `Emittery` as property `emitteryPropertyName` and `methodNames` or all `Emittery` methods if `methodNames` not defined into target class.
	* Returns a [decorator](https://github.com/tc39/proposal-decorators) which mixes in `Emittery` as a property named by `emitteryPropertyName` and `methodNames`, or all `Emittery` methods if `methodNames` is not defined, into target class.

Add bindMethods() method and a mixin() decorator (TS) #37

Add bindMethods() method and a mixin() decorator (TS) #37

Conversation

stroncium commented Mar 6, 2019 • edited by sindresorhus

stroncium commented Mar 6, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stroncium Jun 14, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sindresorhus commented Jun 12, 2019 • edited

stroncium commented Jun 14, 2019 • edited

Andykmcc commented Sep 12, 2019

sindresorhus commented Sep 17, 2019

glasser commented Jan 24, 2020

Add `bindMethods()` method and a `mixin()` decorator (TS) #37

Add `bindMethods()` method and a `mixin()` decorator (TS) #37

stroncium commented Mar 6, 2019 •

edited by sindresorhus

stroncium Jun 14, 2019 •

edited

sindresorhus commented Jun 12, 2019 •

edited

stroncium commented Jun 14, 2019 •

edited