[WIP]: optional registration

[jamestalmage]

This is an idea I had for optional registration. Basically I see two potential scenarios for libraries that have "optional" support.

1. All functionality of the library is supported without Observables, but the presence of a registered implementation will cause it to be enhanced some way. For this, the libraries docs would say: "If there is an Observable implementation registered, returnValue.observable will contain an Observable event stream for XXX."
2. Some methods exported by the library work without Observable support, some do not. In this case, the libraries documentation would say "This method throws an error if no Observable implementation is registered".

Usage:

const registration : OptionalRegistration = require('any-observable/optional');

interface OptionalRegistration {
  implementation: string;
  Observable: Function | null;
  registrationFound: boolean;
  Throwing: Function
}

The only new / interesting idea here is the Throwing function. If an implementation is found, it will just be the Observable constructor (i.e. Throwing === Observable). If not, it will be a function that throws a good error message when called.

This would help with 2 from above. Instead of libraries needing to incorporate logic checking if optional returned a registration or not, and generating their own errors. They can just import the Throwing constructor:

var Observable = require('any-observable/optional').Throwing;

module.exports.needsObservable = function () {
  return new Observable(...); // throws with a helpful message if no implementation found.
}

I have also included logic to allow the error message to reference which library requires Observable support:

// If registration does not exist, thrown errors will reference 'my-lib';
var Observable = require('any-observable/optional')('my-lib').Throwing;

There's a part of me that thinks this just isn't worth it. That optional should just return Observable or null. Especially since that's where I think the any-promise equivalent is headed.

cc: @kevinbeaty

Note: This adds a few tests that need to land regardless. They use proxyquire to test the auto-load behavior when rxjs is not available. And one for global.Observable.

[kevinbeaty]

This is a cool idea.

Another thought:

require('any-observable/optional');  // => Throwing
require('any-observable/implementation'); // => string | null

That is optional would return your Throwing class and implementation would be modified to never throw, and registrationFound could be removed since it is the same as if(implementation).

There's a part of me that thinks this just isn't worth it. That optional should just return Observable or null.

Yeah, me too.

Especially since that's where I think the any-promise equivalent is headed.

Not necessarily. It would be nice to keep feature parity between the two unless there is a strong reason to deviate. I left the optional PR open for your feedback

[jamestalmage]

I think that plan is pretty good.

It gets rid of the message customization, but I think that was of dubious value anyways.

My only concern is that it's unfriendly for usage scenario 1 above. If I want to do an if condition, then I must do two imports? One to get the Observable implementation, once to get the string or null value.

Also, wouldn't not throwing technically be a breaking change for any-promise? We haven't cut a release with the any-promise based code yet (otherwise it would be for us as well).

Perhaps just add a property to Throwing:

var Observable = require('any-observable/optional'); // => Throwing

if (Observable.isThrowing) {
  console.log('not adding observable feature to XXX');
}

Your suggestion has the advantage of being immune to namespace collisions on that property.

[kevinbeaty]

If I want to do an if condition, then I must do two imports?

Yeah, that's a bummer. I'm regretting exporting implementation (I should have just exported registration)

Also, wouldn't not throwing technically be a breaking change for any-promise?

Hmm. Technically, maybe. Although it was never documented that it throws and could be considered a bug. Practically, I think this is very rarely used and would be extremely surprised if someone relied on this behavior. I would probably be willing to write this off as a bugfix.

Although it might be better just to silently deprecate and come up with something better, i.e. remove implementation from the documentation but leave code with comment until next major version.

One thing that is bugging me is the name of optional. It immediately makes me think of option types. I don't think we need to get too fancy, but maybe something similar to Java's Optional (even though many think it's "broken").

module.exports = {
   isPresent() // -or- nullable implementation
   get() // => Observable throws if !isPresent()  (or !implementation)
}

Case 1:

{isPresent, get} = require('any-observable/optional')

if(isPresent()){
   get();
}

Case 2:

{get} = require('any-observable/optional')

get()  // throws generic error

get could potentially accept a function that throws a custom exception. (Maybe a better name is getOrThrow.

getOrThrow();
getOrThrow(() => throw new Error('my custom error'))
// or maybe
getOrThrow('my custom error') // same as previous, throws Error with custom message

Alternatively we could just export registration where both are nullable

var {implementation, Observable} = require('any-observable/registration')
if(implementation){
   new Observable(...)
}

// -- or even just --
if(Observable){
}

That handles Case 1 and the concern with multiple imports. Case 2 is sugar that can be handled either using your Throwable concept or something like getOrThrow.

I'm currently thinking it would be good to export registration and remove implementation from the documentation (you wouldn't have to add implementation to any-observable).

I'm not sure if the sugar for Case 2 is worth it yet.

[sindresorhus]

Closing as this went stale. Happy to reopen when James gets back into open source again.