Finalize API and release 1.0.0

[Hypercubed]

I would like to finalize the API and release a 1.0.0 version. The current mini-signals API matches js-signals:

var signal = new MiniSignal();

signal.add(callbackFunc, callbackContext);
signal.dispatch();
signal.remove(callbackFunc);  // or signal.remove(callbackFunc, callbackContext);

My proposal for the first 1.x release is to return the binding object with a detach method:

var signal = new MiniSignal();
var handle = signal.add(callbackFunc, callbackContext);
signal.dispatch();

handle.detach();

I'm looking for feedback.

[Hypercubed]

Also considering signal.once for 1.0.0.

[samuelmesq]

+1 for signal.once

[Hypercubed]

Some version of the second API is the direction I am going to go. This makes detaching a listener O(1). I have merged the new API into master to help the discussion. Still not published to NPM and still room to discuss. Alternative APIs could be:

var signal = new MiniSignal();
var handleDetach = signal.add(callbackFunc, callbackContext);
signal.dispatch();

handleDetach();

Similar to angular's scope.$on. Or perhaps:

var signal = new MiniSignal();
var handle = signal.add(callbackFunc, callbackContext);
signal.dispatch();

signal.detach(handle);

Which can still be O(1).

[Hypercubed]

@samuelmesq .once is now on master. Thanks.

[Hypercubed]

Other choice to make before 1.0.0 is to finalize the names of all the methods:

signal.add could be .addListener, .on, subscribe, forEach.
signal.once could be .addOnce, .then.
signal.dispatch could be .emit, trigger, .next.
signalHandle.detach could be .off.

Other ideas?

[samuelmesq]

it's a matter of taste, i prefer short names like add, once, emit and off. i use these in one little event-emitter i wrote to start my project. (planning replace it, was how i found yours).

definitely add over on, because i think make more sense in reading. in my case i use string for events name, so: ee.on("thisevent", thishandler);

but signals dont have a event name, is kind of weird read that way: mySignal.on(thishandler);. Add a handler is better.

about detach could be also remove, both are ok.

like i said matter of taste.

Good Luck, have fun!

[robotmayo]

I think detach should be remove as remove is often used as the reverse of add

[Hypercubed]

I'm leaning towards:

var signal = new MiniSignal();
var handle = signal.add(callbackFunc, callbackContext);
signal.once(callbackFunc, callbackContext);
signal.emit();

signal.remove(handle);  // or handle.detach();

Is it good to have both methods to detach signal.remove(handle) and handle.detach()?

Also, still debating if signal.add should return a function or an object. If it returns the binding object then remove can have dual purpose of removing by handle (an O(1) operation) or callback ( O(N) ).

[samuelmesq]

what the purpose of handler? i see possibilities, but not practical use cases.

[Hypercubed]

Currently the signal.remove(fn, ctx) method removes all listeners that match the callback and (optionally) ctx. This operation is O(N). I think this method should be discouraged or removed entirely.

signal.add currently returns the listener "handle" with a handler.detach method. This detaches the exact listener that was added. This is a O(1) operation (given the fact that I use a doubly linked lists for handler nodes).

Alternatively signal.add can simply return the detach function.

The advantage of returning a handler object vs. an unsubscribe function is that we can implement both handle.detach() and signal.remove(handle), both O(1). I'm also considering if it is ever useful for a user to modify the callback or the context after it has been added to a signal... or maybe something more crazy like handle.emitFromHere() or handle.pause().

[samuelmesq]

@Hypercubed i get it, return the handle object is cool.

[shergin]

I like naming in the first post, add and detach. detach should not be remove because this is called on different kind of object. Also I like handle concept, because both of complexity (constant time) and clarity.

[Hypercubed]

Thank you @samuelmesq and @shergin. I think remove will be removed (no pun intended) in the 1.0x. I'm leaving it in for the moment until I can do a major bump.

[r4j4h]

The distinction between returning an object containing a reference to the function or directly returning a reference to the function is so difficult!

I really, really want to side with the object because of your point in your last paragraph in this post that we get two implementations for detaching signals.

But then I kept asking myself why do we want to have two ways of doing it? What are the benefits or how they differ?

In one we need the handle reference, so we have to hold on to that when we add our signal.

In the other we need the signal and the handle's references. We likely already had access to the signal still, and so we are in the same position as the first with no real benefit other than explicitly re-stating the signal this handler is from.

But if handlers are outputs from adding functions to signals, that distinction seems useless - there is no chance of a handler being used by multiple functions OR multiple signals as they are already one to one with the function attachments to the signal. The original js-signals implementation, while slower, does not have this distinction through the deprecated functionality: there is the chance that a function reference is added to the same signal multiple times -- in this case the ability to pass a function and a context can aid in distinguishing between these re-used adds.

So that makes me think that it should just stay as simple as possible and just return the detach function directly.

The other uses of such a returned object would totally change my thoughts - e.g. signalHandler.pause assuming it pauses only that handler while allowing others to continue. I'm not sure if this is where you plan to take mini-signals though so I am erring on the lean side of things.

Hope this is helpful and not just me rambling, because now that I've typed it all I fele like I'm just rambling.

[Hypercubed]

@r4j4h Thank you very much for you input. I feel similar. I really want to return the handler object because it seems to have potential (e.g. .pause). But right now it's just an extra JS object that gets created and needs to be garbage collected later.

jhusain/observable-spec spec suggests that "subscribe must return a subscription object.". But for some reason zenparsing/es-observable returns an unsubscribe function.

One more thing I've been thinking of, that might have some impact on this decision, is the .listeners method. Currently, this returns an array of listener callback functions. I don't see the use of that, especially after .remove(fn, ctx) is deprecated. If anything it should return an array of handle objects or an array of unsubscribe functions... which is weird.

[Hypercubed]

Another thought... returning the signalHandler would allow:

var signal = new MiniSignal();
var handle = signal.add(callbackFunc, callbackContext);
signal.addAfter(handle, callbackFunc, callbackContext);
signal.addBefore(handle, callbackFunc, callbackContext);

or perhaps:

var signal = new MiniSignal();
var handle = signal.add(callbackFunc);
handle.addAfter(callbackFunc, callbackContext);
handle.addBefore(callbackFunc, callbackContext);

I've been considering if and how to implement priority. This would allow priority and still be O(1) on add. I'll probably add an .addFirst method at least.

[r4j4h]

It's interesting to see other libraries are already inconsistent in this regard. I like the ideas in observable-spec and at a glance I feel returning the handler object would conform to that which would be nice.

I think you are right, .listeners could be deprecated. Especially if an unsubscribe function is all that is returned.

But I really like that addAfter/addBefore concept! Using the handlers for priority control is clever, but it makes me wonder then if getting a list of handle objects might be useful. Maybe .listeners would work in that case? Maybe .handlers would be a better semantic name? Either way, I can see it being useful to have a way to check the flow of a signal by seeing the order of all the handlers.

[divmgl]

Purely semantic and minor, but maybe we should rename add to listen for clarity. Or at least provide a prototype.listen = prototype.add. I'd be willing to help with the development. Do we have a roadmap?

[Hypercubed]

@divmgl Thank you for your input. add vs listen may be a difficult choice.

I don't have a roadmap. This project started out as a minimal js-signals implementation to address v8 deoptimization (millermedeiros/js-signals#67). When I switched to linked lists I realized it was easy to implement some of @millermedeiros' v2.0 notes (millermedeiros/js-signals#60).

Here are my thoughts:

• more testing including perhaps some real world application tests.
• add tests for memory leaks.
• more documentation since the API deviates significantly from js-signals v1.0.
• my current thought is to not implement memoization, curried parameters, or auto binding for dispatch (add some easy way to bind Signal.dispatch() millermedeiros/js-signals#47).
• the last two (curried callbacks and binding) should happen in userland. This should be clearly documented, including examples.
• overall I would like to commit to speed, especially for dispatch, and not implement features that slow down.
• once we decide on the v1.0 API I'd like to follow semver... i.e. [breaking].[feature].[bugfix]

[Hypercubed]

I'd also like to keep the option of merging into js-signals open.

[Hypercubed]

For curried parameters I'd prefer signal.add(fn.bind(ctx, 'testing', 'testing'));. This becomes more reasonable when we deprecate .remove as there is no API to remove a listener by function.

That makes me wonder is there any difference between signal.add(fn, ctx) and signal.add(fn.bind(ctx));?

[millermedeiros]

I also like the idea of removing the context argument on add.. Nowadays
all the important environments supports Function#bind and arrow functions
are also becoming available and widely used.

Need to remind that js-signals started back in 2010 and v1.0 was released
on 2012... It was different times.

I really like the idea of keeping the API/features as simple as possible.

Also as a background story.. I had a branch that used linked lists (and was
theoretically faster) but I decided to use arrays since on my projects it
was never the bottleneck (most cases I have 1-2 listeners per event type)
and implementation was way more complex.
On Sep 18, 2015 22:10, "Jayson Harshbarger" [email protected]
wrote:

For curried parameters I'd prefer signal.add(fn.bind(ctx, 'testing',
'testing'));. This becomes more reasonable when we deprecate .remove as
there is no API to remove a listener by function.

That makes me wonder is there any difference between signal.add(fn, ctx)
and signal.add(fn.bind(ctx));?

—
Reply to this email directly or view it on GitHub
#1 (comment)
.

[Hypercubed]

Hi @millermedeiros, really great to have your input. Different times indeed.... in 2010 my primary language was FORTRAN. Fell free to interject as much as you would like. I'd also like to keep this API simple (after all I called it mini-). If later someone wants to make a more robust version they can add back someone of these features (e.g. auto rebind).

In version 0.0.1 I was using some of the optimizations techniques from EventEmitter 3... then I realized that they basically implementing a very dumb (the algorithm, not the people) linked list. It was effectively a linked list for n < 2, then created an array. I tried just an LL and the difference was 2x in the benchmarks, real world applications will vary of course.

[Hypercubed]

I think at some point I'll create a wiki(s) page covering the API difference between mini-signals (pending v1.0) and js-signals. So far I have:

• listeners will be deprecated, use handlers.
• dispatch (or emit) is not auto-rebound, use Function#bind.
• add (or listen) and once doesn't accept a thisArg, use Function#bind.
• add doesn't accept a priority, use addFirst, addBefore and addAfter.
• add returns a SignalBinding instance.
• no default parameters, use Function#bind.
• remove will be deprecated, use detach.
• memorize not supported
• Stop/Halt Propagation not supported (maybe added later)

anything I'm missing?

[Hypercubed]

Hello all, thank you for your help. 1.0.0-beta is now on github in the 1.0.0 branch and on npm (npm install [email protected]).

[Hypercubed]

Hello all, thank you all again for your feedback. I have published v1.0.0 (actually v1.0.1 because my NPM foo is bad). This implements all the new API features we discussed and removes deprecated js-signals compatible API. Releasing v1.0.0 means that I have committed to the API and won't make breaking changes unless absolutely necessary. I'll try to follow semver ([breaking].[feature].[bugfix]) from here on out.

The API details and a comparison of mini-signals to js-signals on the wiki.

[divmgl]

@Hypercubed I love your library. I'm currently using it in a project for an abstract router. As soon as I'm done with the implementation I'm going to make it public. In the meantime, I will add you as a contributor. Seriously, great work.

[Hypercubed]

@divmgl, thank you. Maybe the first usage in the wild! I see you are using mini-signals in ES6 through jspm. This is something I struggled with. I would have liked for JSPM users to be able to use the ES6 modules directly, but this would have meant, in some edge cases, ackward syntax like require('mini-signals').default. After a few back and forths with @guybedford here the best I could do was expose the CJS to JSPM. Open an issue if you think this should be rethought.

[Hypercubed]

I'm thinking of following ericelliott's advice and dumping the class syntax for MiniSignal in favor of a factor. Thoughts?

Pro factory: No new keyword (but backwards compat), more composable.
Con factory: No instanceOf (breaking change), more verbose code.

Thoughts?

[divmgl]

How would the syntax change?

[Hypercubed]

Instead of:

var MiniSignal = require('mini-signals');
var mySignal = new MiniSignal();

You would use:

var miniSignalFactory = require('mini-signals');
var mySignal = miniSignalFactory();

(of course MiniSignal -> miniSignalFactory is optional).

Here is some relevant links: https://medium.com/@_ericelliott/why-hate-on-class-inheritance-4c1067db3f03#6bc5

[divmgl]

I'd say let's not over-complicate things. I like the regular signal syntax. It gives us the option of creating a factory implementation if we choose rather than having the factory pattern forced on us.

[Hypercubed]

I'll leave it as is for now since it is a breaking change for anyone relying on instanceof but something to think about. Signals are designed to be attached to objects, what is the use case for composing or inheriting from a signal?

[divmgl]

Hey @Hypercubed I'm going to open a separate issue for the factory implementation

[divmgl]

Congrats on 1.0, this is a really solid library

[Hypercubed]

Thanks, feel free to keep discussing... I just wanted to see 100% on the v1.0.0 milestone!