Event type guards

[steve-taylor]

The following properties could be used to narrow down event types:

• hasValue
• isNext
• isInitial
• isError
• isEnd

Rather than this:

myStream$.subscribe(event => {
    if (event.hasValue) {
        const {prop1, prop2} = (event as Value<{prop1: string, prop2: boolean}>).value
    } else if (event.isError) {
        const {error} = (event as Error)
    }
})

it would be nice to do this instead:

myStream$.subscribe(event => {
    if (event.hasValue) {
        const {prop1, prop2} = event.value
    } else if (event.isError) {
        const {error} = event
    }
})

Here's some working skeleton code (only implements the distinguishing features of various event types):

interface INextEvent<V> {
    readonly hasValue: true
    readonly isNext: true
    readonly isInitial: false
    readonly isError: false
    readonly isEnd: false
    readonly value: V
}

interface IInitialEvent<V> {
    readonly hasValue: true
    readonly isNext: false
    readonly isInitial: true
    readonly isError: false
    readonly isEnd: false
    readonly value: V
}

interface IErrorEvent {
    readonly hasValue: false
    readonly isNext: false
    readonly isInitial: false
    readonly isError: true
    readonly isEnd: false
    readonly error: unknown
}

interface IEndEvent {
    readonly hasValue: false
    readonly isNext: false
    readonly isInitial: false
    readonly isError: false
    readonly isEnd: true
}

type IEvent<V> = INextEvent<V> | IInitialEvent<V> | IErrorEvent | IEndEvent

abstract class AbstractEvent<
    HasValue extends boolean,
    IsNext extends boolean,
    IsInitial extends boolean,
    IsError extends boolean,
    IsEnd extends boolean
> {
    constructor(
        public readonly hasValue: HasValue,
        public readonly isNext: IsNext,
        public readonly isInitial: IsInitial,
        public readonly isError: IsError,
        public readonly isEnd: IsEnd
    ) {}
}

class NextEvent<V> extends AbstractEvent<true, true, false, false, false> implements INextEvent<V> {
    constructor(public readonly value: V) {
        super(true, true, false, false, false)
    }
}

class InitialEvent<V> extends AbstractEvent<true, false, true, false, false> implements IInitialEvent<V> {
    constructor(public readonly value: V) {
        super(true, false, true, false, false)
    }
}

class ErrorEvent extends AbstractEvent<false, false, false, true, false> implements IErrorEvent {
    constructor(public readonly error: unknown) {
        super(false, false, false, true, false)
    }
}

class EndEvent extends AbstractEvent<false, false, false, false, true> implements IEndEvent {
    constructor() {
        super(false, false, false, false, true)
    }
}

The key is to declare events as being of type IEvent<V>. Although not all event types have a value, the nice part about this is that when either hasValue, isNext or isInitial are true, value doesn't need to be cast to V.

My own Bacon alternative (work in progress) doesn't use event classes at all. Events are created by factories and there's no need for weird prefixed type names such as IEvent.

[raimohanska]

Hi Steve,

There are actually type-narrowing functions available:

https://baconjs.github.io/api3/globals.html#hasvalue
https://baconjs.github.io/api3/globals.html#isend
https://baconjs.github.io/api3/globals.html#iserror

[steve-taylor]

Nice. I didn't know about these and will definitely use them.

Ideally, they wouldn't be needed. I'm considering making a PR to support type narrowing in the event objects themselves, but wondering if it's possible without breaking changes. Thoughts?

[raimohanska]

Well I wouldn't necessarily duplicate the functionality that already exists, especially when it may introduce breaking changes. Maybe hiding the fields altogether and improving documentation might be a better route?

[steve-taylor]

The type narrowing functions feel like a compromise. (Don't get me wrong — I'll use them.) They're not attached to event objects, so it's not OOP, and they have to be imported.

event.hasValue feels a lot nicer than hasValue(event).

I'm happy to take matters into my own hands, being mindful of not making breaking changes.

[raimohanska]

Okay, break a leg :)

[steve-taylor]

Looks like it can't be done without messy breaking changes. Specifically, the Event<V> class will need five more generic types, which itself is a breaking change as it's part of the exposed API.

[steve-taylor]

Reopening as a possible v4 enhancement. It could be done in a clean way by replacing the event classes with interfaces and factories.