feat(context): add strongly typed on and once methods

[bajtos]

Declare on and once overload methods to describe event parameters in a strongly typed way:

context.on('bind' | 'unbind', ContextEventListener)
context.once('bind' | 'unbind', ContextEventListener)
binding.on('changed', BindingEventListener)
binding.once('changed', BindingEventListener)
view.on('refresh', listener);
view.once('refresh', listener);
// and so on

The change preserves the generic variant provided by EventEmitter too and thus is fully backwards compatible.

Checklist

👉 Read and sign the CLA (Contributor License Agreement) 👈

• 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

👉 Check out how to submit a PR 👈

[bajtos]

What’s the impact on tsdocs?

It looks like our tsdoc processor is not merging class + interface declarations correctly 😠 In http://127.0.0.1:4001/doc/en/lb4/apidocs.context.html, I see Binding listed under Classes and Interfaces sections. Both entries point to the same URL http://127.0.0.1:4001/doc/en/lb4/apidocs.context.binding.html where only the interface members are described.

I tried to swap the order in which the class & interface is defined, but that did not help.

I reported the problem to api-extractor, see microsoft/rushstack#1921

[bajtos]

I pushed a new version, can you @raymondfeng PTAL again please?

The impact of the api-extractor bug looks pretty major to me, I think we will have to wait with this pull request until a fix is available. WDYT?

[raymondfeng]

The impact of the api-extractor bug looks pretty major to me, I think we will have to wait with this pull request until a fix is available. WDYT?

I agree. We should wait.

[bajtos]

I found a workaround that preserves the current API docs for class members and excludes members that are added via the interface, see 06abcdf.

@raymondfeng LGTY? Are you good to proceed to landing this PR (after I squash commits)?

Screenshot from the rendered docs:

[raymondfeng]

@bajtos Why don't we add the following the Context class itself? The following is working for me.

export class Context {
  // ...
  on(eventName: 'bind' | 'unbind', listener: ContextEventListener): this;
  on(event: string | symbol, listener: (...args: any[]) => void): this;
  on(event: string | symbol, listener: (...args: any[]) => void): this {
    return super.on(event, listener);
  }
}

[bajtos]

Why don't we add the following the Context class itself? The following is working for me.

It didn't work for me, let me try again.

[bajtos]

Nice, the proposed solution works 👍

I find it a bit suboptimal that we have to implement on/once methods calling super.on/super.once, but I guess it's the cost we have to accept now.

Here is a screenshot of API docs rendered for Binding class:

@raymondfeng LGTY now?