feat(events): add basic async events support

[raymondfeng]

This PR introduces basic observer pattern support for LoopBack 4 so that we can start to use eventing to communicate between different artifacts. For example, we can notify subscribers of start/stop events. The context itself can generate bind/unbind events too.

This is a rewrite of https://github.com/strongloop/loopback-datasource-juggler/blob/master/lib/observer.js.

Checklist

• npm test passes on your machine
• New tests added or existing tests modified to cover all changes
• Code conforms with the style guide
• API Documentation in code was updated
• Documentation in /docs/site was updated
• Affected artifact templates in packages/cli were updated
• Affected example projects in examples/* were updated

[bajtos]

Let's discuss this proposal at high level first.

(1) I would like to follow the scenario-driven approach here and start by defining high-level acceptance tests to drive the design of the new APIs. This pull request seems to go in the opposite direction, starting with low-level building blocks that may or may not fit together once we start building an end-to-end solution.

Few examples from your PR description:

• notify subscribers of start/stop events
• the context itself can generate bind/unbind events too

I would also like to see an example showing how to leverage this new API for model/repositories (think of Operation Hooks in LB 3.x) and REST API (think of remoting hooks in LB 3.x), assuming these two scenarios are valid for our new LB4 architecture (I am not entirely sure).

For example, if we decide to support repository-level events, then it's not clear to me how the currently proposed design will allow LB4 app developers to register observers for repository events. The problem I see is that each Repository instance is a short-living instance freshly created for each incoming request. Another problem to consider - how to allow inheritance of observer registration, i.e. allow developers to register a single observer to listen on events on all repositories (DefaultCrudRepository subclasses).

(2) Have you considered using an existing implementation of Observer API, instead of building (and maintaining) our own version? RxJS seems to be a highly popular alternative these days.

There is also a TC39 Proposal for adding Observable API to JavaScript language, it would be great if we can align with that proposal. See https://github.com/tc39/proposal-observable and packages mentioned by that proposal: zen-observable and any-observable.

If we decide to keep building our own:

(2) Is it a good idea to bundle Observer API into Context module, instead of creating a new npm package? My concern is that we are forcing consumers of Observer API to depend on full Context module, even if the consumers do not need Context (DI/IoC).

For example, models and entities can implement Observable interface, but should not need Context at all.

This can become a problem when there are multiple semver-major versions of Context, thus different LB components/extensions end up with different instances of Context packages in their dependencies, despite the fact that they all depend on the same version of Observer API. I guess this will not affect runtime behavior, but it does unnecessarily increase dependency size and installation time.

I am proposing to introduce a new package, e.g. @loopback/observable or @loopback/observer.

(3) In your proposal, you require all observers to be object instances. IMO, that's an anti-pattern in JavaScript, where event observers have been implemented as (anonymous) functions from the early days of the language. I am ok to support observers as objects if you like, but only as long as users can define observers as functions too.

[raymondfeng]

1. The PR was triggered by the a few use cases:

• allow various components to listen on start/stop events
• allow extension points to be notified when extensions are added/removed/updated

I agree that we should continue to build an inventory of use cases that fit into the observer pattern.

2. I was trying to implement it using RxJS and realized it might be over-engineered and complicated for the use cases I have so far.

• RxJS pushes a series of events to subscribed observers - next/next/..., complete/error.
• I haven't figured out how to tell RxJS to wait for observers to resolve/reject so that we can make sure hooks/listeners are completed before moving to the next step.

I'm definitely open to have RxJS part of the LoopBack 4 programming model if necessary.

3. I'm open to have @loopback/observer module.

[bajtos]

The PR was triggered by the a few use cases:

• allow various components to listen on start/stop events
• allow extension points to be notified when extensions are added/removed/updated

Makes sense. I am proposing to include the implementation of the first point in this pull request and use acceptance-level tests for this feature to drive the internal design.

For example, I am not sure if the current interface + class based design can support our needs, a mixin-based approach may work better. It's hard to tell until we start the actual implementation of listeners for start/stop events.

I was trying to implement it using RxJS and realized it might be over-engineered and complicated for the use cases I have so far.

• RxJS pushes a series of events to subscribed observers - next/next/..., complete/error.
• I haven't figured out how to tell RxJS to wait for observers to resolve/reject so that we can make sure hooks/listeners are completed before moving to the next step.

I see. The requirement to wait for observers to finish is critical for us. AFAICT, the Observable proposal for TC39 does not allow to wait for the consumers to finish either.

My take-away: We need a different programming model from what Observables provide.

I think it's important to find a different name than Observer, to make it clear that our programming model is different from Observables & RxJS.

How about using "async event emitter"? IMO, this concept describes exactly what we need: a publisher wants to trigger an event and wait until all subscribers finished processing this event.

Surprisingly enough, there are very little existing implementations of this pattern on npmjs.org. The package emittery seems to be the most popular option, it's maintained by respected @sindresorhus. Personally, I'd prefer to use this module instead of inventing our own. My only reservation is that emittery is still in 0.x version, but I hope that can be easily fixed if we ask for v1.0.

Thoughts?

[raymondfeng]

@bajtos Great feedback.

I refactored it to be @loopback/events with potential expansion for other events related patterns such as RxJS. The code is more aligned with emittery now.

The emittery module provides similar functions for now. But there are a few things that I don't like:

1. A global registry - see https://github.com/sindresorhus/emittery/blob/master/index.js#L3
2. The lack of support for wildcard matching which we need for LB3 and intend to optimize with trie based matching.

[sindresorhus]

A global registry - see sindresorhus/emittery:index.js@master#L3

I'm curious why you don't like that?

[raymondfeng]

@sindresorhus Thank you for chiming in.

In your code, the maps are module-scoped constants. I think the intention is to hide such implementation details via closure functions. But it becomes difficult to test in isolation or replace with a new way to manage subscriptions.

That's why I use
https://github.com/strongloop/loopback-next/blob/observer/packages/events/src/events.ts#L177 and make it a dependency of EventSource.

[bajtos]

I see a lot of new low-level APIs added by this proposal and honestly I cannot comprehend what is your intended use for many of those APIs. Let's follow our scenario-driven approach please.

Re-posting an older comment again:

The PR was triggered by the a few use cases:

• allow various components to listen on start/stop events
• allow extension points to be notified when extensions are added/removed/updated

Makes sense. I am proposing to include the implementation of the first point in this pull request and use acceptance-level tests for this feature to drive the internal design.

For example, I am not sure if the current interface + class based design can support our needs, a mixin-based approach may work better. It's hard to tell until we start the actual implementation of listeners for start/stop events.

[bajtos]

I think most listeners won't be interested in the eventType value. Can we switch the order of the arguments to allow consumers of this API to ignore eventType?

export type ListenerFunction = (
  event: Event,
  eventType: string
) => Promise<void>;

A test to verify this works:

registry.subscribe(source, 'starting', data => {/* do something with data */});

Also, have you considered how to enable type checking for events? I believe TypeScript has a way how to express that certain eventType values are linked to certain event data formats.

For example, I can imagine using the same trick we do for BindingKeys.

// a simplified mock-up
subscribe<T>(source, eventType: EventType<T>, listenerFunction: EventListener<T>) {
  // ...
}

[bajtos]

I think we should support subscribeOnce too.

[raymondfeng]

Added once.

[bajtos]

Personally, I'd prefer to use eventName instead of eventType. In my mind, a "type" usually refer to concepts like Number, String, Product (i.e. a constructor function). OTOH, "name" is almost always a string.

[raymondfeng]

Added EventName<T>

[raymondfeng]

@bajtos PTAL

[bajtos]

I am proposing to put this pull request on hold until #1928 is finished & landed first.

[bajtos]

Related: #2291

[emonddr]

@raymondfeng , @bajtos mentioned:

I am proposing to put this pull request on hold until #1928 is finished & landed first.

If #1928 lands shortly, can this one land shortly afterwards, or will it require some (major/minor) rework?

[rgupta2]

@bajtos @raymondfeng Not sure if this is relevant to ask here but my question has roots in this old open PR. With 393 issues and 49 PRs where one of the oldest open PRs goes back as far as Nov 2018 doesn't give me confidence to kickstart our organization's migration from Loopback 3 to 4. There's also just 215 question on Stackoverflow out of which 159 remain unanswered. You seem to be the two most active contributors to this project. Could you help me get some perspective here? Should we delay migration from loopback 3 to 4? Should we write our new projects in loopback 4? i have been trying to use lb4 and community support seems hard to get

[w20k]

Hello everyone,
Out of curiosity, will this PR be moved to an 'upcoming' milestone (this year or next?) or will be left in the backlog for a while?

In advance, thank you for response! 😉

[achrinza]

Hi @w20k, is there a use-case that would benefit from this pull request? This would be helpful in understanding the needs and if existing solutions may be better-suited.

[raymondfeng]

The PR is very out of date. I'm closing it.