[wip] feat(core): introduce ComponentLifecycle - #20682

This is just... so great. I could remove so much boilerplate with this.

It would be great to have an onChanges hook also.

I'm tackling view_compiler's behaviors. Currently, a component which has neither ngOnInit nor ngDoCheck won't be checked by Angular.

I think it's needed that a metadata to enable ComponentLifecycle to tell Angular the component needs update processing...

1. I think that PR demonstrate a flexibility problem on Angular. That kind of extension should be easily done without having to change internal code, like I discussed in that proposal (wouldn't be enough but it's a start).

2. Wouldn't it be better to use a single Subject with different value and filter it to create observables for each lifecycle event ?

const enum LifecycleEvent {
  OnInit,
  OnChanges ,
  ...
}

class ComponentLifecycle {
  private subject = new Subject<LifecycleEvent>();

  events = this.subject.asObservable(); 
 
  private getLifecycleEvent(eventType: LifecycleEvent){
    return this.subject.filter(v => v === eventType).mapTo(true);
  }

  // utility observable, probably better to make them lazy so we don't build them if not needed
 // a getter should do 
  onInit = getLifecycleEvent(LifecycleEvent.OnInit);

  onChanges = getLifecycleEvent(LifecycleEvent.OnChanges);
}

and then once we get OnChanges observable we can easily filter again and even get Input observables.

forInput(inputName: string) {
  return this.onChanges.pluck(inputName).filter(v => v !== undefined);
}

edit: We'll need something more complex than just number for the subject since we need to emit the changes for onChanges, but that's the idea.

@tbosch I'd like to hear how do you think this approach. Could I do this, avoiding to touch compiler codes?

let me think about this...

Wouldn't it be better to use a single Subject with different value and filter it to create observables for each lifecycle event ?

Why would it? I think the proposal should mirror existing lifecycle hooks where you get a method per hook rather than a single method that takes an argument corresponding to destroy/init etc. You can always easily combine observables with various operators and ideally each observable will have a single typed parameter (change lifecycle needs to be passed the changes, the rest are all void).

I think that PR demonstrate a flexibility problem on Angular. That kind of extension should be easily done without having to change internal code, like I discussed in that proposal (wouldn't be enough but it's a start).

That comment what kinda off topic, I did it after viewing how ComponentLifecycle was made and what I meant was :
Building a service attached to a component with the same hooks as the component should be easy to do to allow composition of components. Which is off topic, sorry for that :)

Especially considering that for this exact feature I really think we should change the internals. Basically if we have stream events, adding a lifecycle function on a component would just be sugar for 'subscribe to that event and use that function as callback'.

function subscribeToEvents(component, lifecycle) {
  for (let event of ['OnInit', 'OnChanges', etc...]) {
    const fnName = 'ng' + event;
   const eventCallback = component[fnName];

    if (typeof eventCallback === 'function') {
      const streamName = event +'$';  
      lifecycle[streamName].subscribe( eventCallback.bind(component) );
    }
  }  
}

@ohjames Well we already had this debate on a different issue but in short :

1. What we want is to observe a component throughout it's life time. During when multiple events can occurs but it still part of the same observation. 1 observation = 1 observable.
2. 1 Subject is better than N Subject, with N being the number of lifecycle events + the number of inputs. Especially with them all having the exact same lifetime because of 1) (we're just observing the same thing in the end).
3. I think ngOnChanges is really powerful because it's called once all inputs have changed during a CD round. If we were to have an observable per input we wouldn't be able to compose them in a way that would mimic ngOnChanges. If we have input1$ and input2$, input1$ emits a change then input2$ emits one as well : Was it during the same CD or part of different CD round ? In what condition do we merge those 2 emits into one ? Do we introduce a new observable emitting for each CD round ?

1. Using your own semantics, is observing the destruction and the changes the same "Observation". Not to me.
2. You state that 1 subject is better than N subjects without providing any concrete support for the betterness? Say you are only interested in the destroy event, then you have to filter out every single change event? Plus with a subject for each lifecycle event you can have an appropriate payload per observable, if you bundle them all up into one then now I have to deal with a discriminated union argument for every single observable? More hassle and less typesafe. So there's two concrete reasons why the 1-subject approach is worse rather than better, I don't see any contrary points beyond the reference to the vague semantics stated in (1).
3. I wasn't suggesting we have an observable for each input as part of this lifecycle detection. That's not a good idea at all, the life cycle observable should emit a single changes event as ngOnChanges does for this feature.

Could a single observable be used though with a simpler API that uses a consistent payload as follows:

1. First value emitted, state of all @Input values during ngOnInit().
2. Subsequent values emitted for all changes, I guess with a copy of the inputs.
3. Observable is closed at ngOnDestroy()

Then you could this.changes.take(1).subscribe(inputs => {}} for an equivalent of ngOnInit(), use this.changes.subscribe(inputs => {}) to observe all changes and this.changes.subscribe(null, null, () => { /* handle ngOnDestroy */ }); to observe the component being destroyed.

Hm I really think that taking advantage of the things that observables provide to produce a lifecycle API would be much better than taking the plain old life cycle methods and converting them to the world of observables.

For example the ngOnChanges lifecycle hook takes a SimpleChanges object to make it easy to compare the new value of an input with the previous value, however with observables we can take advantage of operators to do the same thing. So IMO it makes a lot more sense to present the changes as an object of properties against their current values than presenting an observable of SimpleChanges.

I really think that the init and destroy events could be combined into the same observable as changes also as per my last comment. Why do we need observables presenting a single event for destruction when we can just wait for another observable to terminate?

I want this feature so badly but I really think this implementation gets too much inspiration from the existing lifecycle events without taking advantage of what rxjs offers.

so I am for something like this, but I don't want to do this as one off just for init it should be holistic for all events and it also needs to fit with render3 as well as #13248 so this requires more thinking and a design doc.

it seems that this feature just part of that the rxjs deep integrated with angular.

remind me this have changes stream metioned above . I am implementing a requirement that form/layout/page inputs/outputs partically/fully controlled by data definition.

wants to see any genius idea, or real case requirements?

can't modify template free of hand, just data definition. I don't know using high level abstract downgrade to template file, or just data transform and binding data with stream.

@LinboLen This wont be usable anytime soon

@mhevery

so I am for something like this, but I don't want to do this as one off just for init it should be holistic for all events

The author changed the code to provide all events rather than just init. I don't think this is suitable though:

1. Why need a separate ngOnChanges, ngOnInit and ngOnDestroy events when you can use .take(1) and .subscribe(null, null, () => { ... }) to deduce "init" and "destroy" from a single "changes" stream.
2. Why bother passing SimpleChanges as the payload to ngOnChanges when the pairwise operator can be used instead of having to use currentValue and previousValue. Better to pass an object or map of property name against value.

So I'd really like to see a single changes stream, no init stream and no destroy stream, but I'm not sure about how to handle the other lifecycle events, maybe those really do warrant extra observables.

Hi @lacolaco! This PR has merge conflicts due to recent upstream merges.
Please help to unblock it by resolving these conflicts. Thanks!

This seems like a very cool idea. Here is an optimization question:

component_lifecycle.ts:

Why not use a single Subject() and then hook the lifecycle event to emit an event type. Then you readonly observables would simply filter on the specific event type from the same source.

Here is a suggested change:

import { filter } from 'rxjs/operators';
import { pluck } from 'rxjs/operators';

export enum LifecycleHook { onInit, onChanges, onDestroy }
export type LifecycleEvent = {  type : LifecycleHook, value : SimpleChanges | void };

export class ComponentLifecycle {
  /**
   * Used by provider.ts
   * e.g.   view.lifecycle.emit()
   */
  public emit(type: LifecycleHook,  value:SimpleChanges) {
    this.source.next({type,value,});
  }

  readonly onInit$    = this.source.pipe( ...valuesFor(LifecycleHook.onInit) );
  readonly onChanges$ = this.source.pipe( ...valuesFor(LifecycleHook.onChanges) );
  readonly onDestroy$ = this.source.pipe( ...valuesFor(LifecycleHook.onDestroy), 
     tap( () => this.source.complete() )
  ); 

  private source = new Subject<any>();
}

/**
 * Filter observables by LifecycleHook type and then pluck the value
 */
function valuesFor<any>(allowedType: string) {
  return [filter((event: LifecycleEvent) => event.type == allowedType), pluck('value')]
}

@ThomasBurleson or just provide the changes observable, take the first stream element (take(1) operator) to detect init, subscribe to the stream end (3rd param to subscribe) to detect destruction.

@lacolaco Is the PR is relevant?

Good news is that we most definitely want this feature!

The bad news is that we don't want to add it to ViewEngine and than have to reimplement in Ivy because the API will be different in Ivy. So for now I am going to close this PR and we will add this feature to Ivy.

Thank you. I'm looking forward to that!

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.}