ObservableLike's subscribe() return type should have unsubscribe()

[darcyparker]

See https://github.com/sindresorhus/type-fest/blob/main/source/observable-like.d.ts#L13

Currently the subscribe() returns void and not the subscription. Various implementations have different interfaces for the subscription... but all seem to include unsubscribe().

I can submit a pull request to fix this, but wanted to check/verify there wasn't a good reason for returning void.

An example change that points out my uncertainty/doubts about the SubscribeResult.

/**
@remarks

Some implementations define additional properties that are not standard.

So, `Subscription` is defined as a type rather than interface to ensure the implementation's subscribe's return type is assignable
*/
type Subscription = {
	closed?: boolean; //Many implementations define `closed`
	unsubscribe(): void;
};

/**
@remarks
I am not sure about `Unsubscribe` because it is not in ts39 proposal.
It is defined in `zen-observable`'s types, and I can see it as plausible return type. So maybe its worth defining.  But it would mean the consumer of the `SubscribeResult` would need to check the type of the `SubscribeResult` at run time.
However, if `Unsubscribe` is an uncommon `SubscribeResult`, the extra type checking at runtime is painful...
@see https://github.com/DefinitelyTyped/DefinitelyTyped/blob/7aded252f5659f98dad25d2a1b3d04f8e324ab8c/types/zen-observable/index.d.ts#L34-L39
*/
type Unsubscribe = () => void;

type SubscribeResult =
	Subscription | //As per tc39 proposal and most implementations I am aware of
	Unsubscribe | //See remarks above
	void; //plausible, but it is not in tc39 proposal, and like `Unsubscribe` case above, it means more type checking at runtime.

/**
Matches a value that is like an [Observable](https://github.com/tc39/proposal-observable).

@see https://github.com/tc39/proposal-observable
@category Basic
*/
export interface ObservableLike<ValueType = unknown> {
	subscribe(observer: (value: ValueType) => void): SubscribeResult;
	[Symbol.observable](): ObservableLike<ValueType>;
}

Some references:

• https://github.com/tc39/proposal-observable#example-observing-keyboard-events (last line has subscription.unsubscribe())
• https://github.com/tc39/proposal-observable/blob/master/src/Observable.js#L129-L130
• https://github.com/benlesh/symbol-observable#making-an-object-observable: notice subscribe() returns an object with unsubscribe()
• https://github.com/staltz/xstream/blob/6c22580c1d84d69773ee4b0905df44ad464955b3/src/index.ts#L79-L85
• https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/zen-observable/index.d.ts#L37

[sindresorhus]

I prefer to follow the TC39 proposal. So a unsubscribe() and closed, but not type Unsubscribe = () => void;.

[sindresorhus]

closed?: boolean; should not be optional?

[darcyparker]

@sindresorhus - Thanks. I like unsubscribe() and closed() too. I suggested making it optional because some implementations don't include it... But I prefer to follow the TC39 proposal, so will include it. This will be a flag to others that they don't implement the TC39 proposal.

[sindresorhus]

Which implementations do not provide closed?

[darcyparker]

https://github.com/staltz/xstream/blob/6c22580c1d84d69773ee4b0905df44ad464955b3/src/index.ts#L79-L85

[sindresorhus]

Ok, let's keep the closed optional for now, with a comment on why it's optional. People that know it exists can just use !.

I would encourage you to open an issue on implementations without closed to add it for compatibility.

[darcyparker]

WRT 'closed' not existing on xstream's subscription implementation: staltz/xstream#326