Infrastructure for per-transport configuration

[Xon]

Rather than generate complex configuration, use the existing emitter infrustructure to ask how a transport should be configured.

Primary goal is to be able to configure long-polling with custom headers, but support shared master/slave transport roles.

Demo example of using (note pollDelay isn't included in this PR);

sub.on('transportSetup', function (subscriber, transport){
    if (subscriber === 'longpoll') {
        transport.headers['xfToken'] = XF.config.csrf;
        transport.pollDelay = 10*1000;
    }
});

[slact]

I quite like this approach. However, this would mean that I will have to maintain API consistency for all transport objects, which up to now have been internal and free to be refactored.

What I would prefer is to expose the underlying transport object (WebSocket/EventSource/XMLHTTPRequest, and an object of options that the 'transportSetup' callback can mutate. This way, the transport wrappers can stay internal, but their options as well as the native browser object can be exposed to the user.

[Xon]

@slact so my thoughts are; move the 'transportSetup' to after the native object is created but before it is used. This means when polling via XMLHTTPRequest it would be called once per request. This has it's trade-offs.

This then gives a place to defined an interface rather than exposing the raw transport object.

[Xon]

After looking through how to integrate the above, by the time the XMLHTTPRequest is created it is too late to set additional headers via the nanoajax library.

Perhaps an additional per-polling configuration callback?

[slact]

Can you think of any use case for XMLHTTPRequest on transportSetup other than setting headers? I think a callback signature of something like function(opt, nativeTransportObject) would be enough, where opt.headers would represent headers to set for every request.

This mechanism would need to deal with the inability to set headers for WebSocket and EventSource objects, and probably error out in those cases right after transportSetup. The main use that I can see in exposing the underlying native transport object is for users to be able to use the WebSocket object bidirectionally. The rest should be handled by settings fields in the opt object.

[Xon]

My use-case is being able to change the polling frequency, and being able to set a custom header for the XMLHTTPRequest to use.

Perhaps splitting it into two events, transportSetup(opt) and transportNativeCreated(nativeTransportObject) would be the solution. transportSetup(opt) (predominately around XMLHTTPRequest) could setup headers and polling frequency. While transportNativeCreated(nativeTransportObject) allows the WebSocket/EventSource to be extracted.

[slact]

Makes sense to me. Two events seems like the way to go.

[Xon]

Does it make much sense to pass the name/subscriber into these callbacks?

[slact]

Yes, because different native transports accept different options, and more transport negotiation may be added in the future.

[Xon]

@slact recent force-pushed commits should follow the previous discussed structure. I've then rebuilt my polling delay bits on-top of it to test & use it

[slact]

Great stuff! We're almost there. Just two more things:

I think the callbacks function signatures should be

sub.on("transportSetup", function(opt, subscriberName) {
  // opt is a hash/object - not all transports support all options equally. Only longpoll supports arbitrary headers
  // subscriberName is a string
});

sub.on("transportNativeCreated", function(nativeTransportObject, subscriberName) {
  // nativeTransportObject is the native transport object and depends on the subscriber type
  // subscriberName is a string
});

-- because the primary parameter of use is the transport and the options object, the the name may or may not be used by the user.

The other thing is I'm not sure about exposing a callback for the __slave subscriber at all -- There's nothing configurable about it. I think it should generate no transportNativeCreated callback, since there is no native object being created here. I don't know if transportSetup should be called for __slave either -- what are your thoughts?

[Xon]

I agree it doesn't make any sense for a __slave transportNativeCreated callback, but there should be some notification that a master has been demoted to slave if you are going to expose the long-lived native transport options.

Another aspect is the cross-tab notification system, it would be handy to be notified that a tab is a 'slave' to drive other UI logic.

I'm also leaning to a transportTeardown callback would also make sense for completeness purposes.

[Xon]

@slact any feedback?

I have an additional PR I plan to base on top of this work; https://github.com/Xon/nchan.js/commit/41b6fe81e3b8eb1d96747fa2f7d011a00d0bc919

[slact]

Nice work. We'll figure out how to best expose shared master/slave subscribers later.