Do away with 'signals' namespace?

[jonnyreeves]

At present JS-Signals use the signals namespace to avoid global pollution. However, as with v0.7.4 of the library, the only property exported to this namespace is the Signal constructor function for creating a new Signal object. Is there a strong argument against dropping the signals namespace and just exporting the Signal constructor instead?

This would make importing Signals with RequireJS tidier, ie:

var Signal = require('vendor/signals');
var mySignal = new Signal();

[millermedeiros]

I've also questioned myself about this decision :D - at the beginning I was thinking about adding different kinds of signals to this object (AS3-signals has many diff types) but I ended up adding all the features into the same "class" (priority, addOnce, memorize) and created a diff kind of signal as a separate project.

Maybe the solution is to keep the namespace for the browser but register the signal itself for AMD and node.js... the only problem is that this change will break all AMD and node.js code that is already using signals... For now you can do like this:

var Signal = require('vendor/signals').Signal;
var mySignal = new Signal();

The namespace is more future-proof since we can add new properties to the object at will, but I will keep this issue open to see if more people agree with the change and maybe that's the change that will motivate a v1.0.0 release (since it's a breaking change).

[millermedeiros]

ahhh I just found a non-breaking solution after hitting the send button! hehe

In JavaScript Functions are first-class objects, which means we can add properties to the function, so we could change the code to be:

function Signal(){
  // ... regular code here
}

// expose constructor as a property as well
Signal.Signal = Signal;

And just expose the Signal constructor for AMD and node.js without breaking legacy code! #winning

var Signal = require('signals');
var mySignal = new Signal(); // works!
// legacy support
var signals = require('signals');
var mySignal = new signals.Signal(); // still works

This should definitely ship in the next version since it's non-breaking and will be less verbose... For the browser I think we should keep the current name signals for the namespace since it's weird to have lowercase constructors and user can just create an alias like showed in the examples. The namespace is important for the browser since user might be using the CompoundSignal as well (which requires the global signals object)

[jonnyreeves]

Sounds like a good solution to - continue to export the signals namespace in the browser whilst allowing those using AMD to require the Constructor directly :) +1

[sork]

As a user of js-signals as an AMD module, I think this is a good idea. I agree that the signals namespace is not necessary in that context.

[conradz]

@millermedeiros Is this change going to be made soon? This would really make it nicer to use.

[millermedeiros]

@conradz and @jonnyreeves sorry for the delay. This just landed on v0.8.0. Now on node.js you can do:

var Signal = require('signals');
var onAwesome = new Signal();
// ...

and on AMD:

define(['signals'], function(Signal){
   var onAwesome = new Signal();
   // ...
});

On the browser it still exposes a single global signals.

var onAwesome = new signals.Signal();

Legacy code should keep working since the Signal constructor is also exposed as a property of itself:

// this will keep working
var Signal = require('signals').Signal;