Finalizing output() and outputFromObservable() APIs

[devversion]

See individual commits.

———

output()

This commit exposes the new output() API with numerous benefits:

• Symmetrical API to input(), model() etc.
• Fixed types for EventEmitter.emit— current emit method of
  EventEmitter is broken and accepts undefined via emit(value?: T)
• Removal of RxJS specific concepts from outputs. error channels,
  completion channels etc. We now have a simple consistent
  interface.
• Automatic clean-up of subscribers upon directive/component destory-
  when subscribed programmatically.

import {output} from '@angular/core';

@Directive({..})
export class MyDir {
  nameChange = output<string>();     // OutputEmitterRef<string>
  onClick = output();                // OutputEmitterRef<void>

  doSomething() {
    this.onClick.emit();
  }
}

Note: RxJS custom observable cases will be handled in future commits via
explicit helpers from the interop.

outputFromObservable()

import {outputFromObservable} from '@angular/core/rxjs-interop';

 @Directive({..})
 export class MyDir {
   nameChange$ = new BehavorSubject<string>();
   nameChange = outputFromObservable(this.nameChange$); // OutputRef<string>
}

outputToObservable

import {outputToObservable} from '@angular/core/rxjs-interop';


outputToObservable(myInstance.someOutput)
  .pipe(take(1))
  .subscribe(...)

[markostanimirovic]

A great addition to the rxjs-interop plugin! 🚀 However, function names seem too verbose to me. Consider the following naming:

• outputFromObservable => rxOutput
• outputToObservable => add an additional signature to the existing toObservable function that accepts OutputRef as input argument

-import { outputFromObservable } from '@angular/core/rxjs-interop';
+import { rxOutput } from '@angular/core/rxjs-interop';

@Directive({..})
export class MyDir {
  nameChange$ = new Subject<string>();
- nameChange = outputFromObservable(this.nameChange$);
+ nameChange = rxOutput(this.nameChange$);
}

EDIT: Another suggestion - outputFromObservable(obs$) => toOutput(obs$)

[alxhub]

Burn it all 🔥

[alxhub]

Why not inject(DestroyRef, {optional: true})?

Or are you worried about EventEmitters created outside of DI contexts? I would recommend avoiding the try/catch and checking getInjectImplementation() || getCurrentInjector() (see impl of assertInInjectionContext.

Also, have we checked in g3 whether making EventEmitter depend on injection context is that breaking?

[devversion]

It's because EventEmitter may be constructed outside of an injection context, yes.

Started a TGP to check how breaking it would be.

[devversion]

Results: 40k failures, unfeasible. Most issues arise from QueryList lazily creating an EventEmitter (?!) lazily for .changes. This can happen outside of injection contexts and affects a lot of targets

[crisbeto]

Do we need to do something with the error/complete channels here?

[devversion]

Good point. I think we should just document that users are responsible for handling errors beforehand. If they aren't, the error will be simply handled by RxJS and will be reported unhandled to window.onerror (see RxJS reportUnhandledError.ts)

[twittwer]

I really like the suggestion from @markostanimirovic to use the same naming pattern as the signal interop with toOutput & toObervable.

And if the extension of toObervable is a problem maybe fromOutput.

[pkozlowski-opensource]

LGTM. As discussed in person I'm not sure about the completion semantics of the output as Observable and - more importantly - EventEmitter involvement in it. I would lean towards dropping the completion logic but at the same time I don't feel like I understand all the consequences here.

Reviewed-for: public-api
Reviewed-for: fw-core

[brandonroberts]

I agree with toOutput to be consistent with the other helper functions. This is the other direction, but I recall Ben Lesh providing some feedback against using the fromThing convention for naming.

Also from an import perspective, you might hit output much more frequently when trying to get to outputFromObservable when using autocomplete because of the naming similarity. toOutput being similar to the others is more intentional when trying to use it.

[pkozlowski-opensource]

This PR was merged into the repository by commit d4154f9.

[TomieAi]

.required when? I love some vscode yelling I forgot something.

[Flo0806]

.required when? I love some vscode yelling I forgot something.

The new output is the replacement for EventEmitter(). And it would be not a good idea to have a required option. Think: You don't need the click event on every button. But if you must use it, it would just annoy the hell out of you.

In the end: a input is different - here it makes sense to use required if needed, but not on a output/EventEmitter.

Greetings, Flo

[blogcraft]

Why output() does not use signals like the new input()?

[JoostK]

Why output() does not use signals like the new input()?

It represents an event, i.e. an action at some point in time, rather than state, i.e. stored data. Signals correspond with state storage, not events.

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}