TakeUntilDestroy not working as expected

[sebitsi]

• Your ABP Framework version. 3.0.3
• Your User Interface Angular

I have noticed that TakeUntilDestroy is not unsubscribe from observable.

Simple example:

I have created observable from ngxs state and local variable that stores state into it

  @Select(ItemDataSource.viewOf<ItemDto>())
  public view$: Observable<IView<ItemDto>>;

  public focusedItem: ItemDto = undefined;

Than in OnInit hook i subscribe on this observable like this:

  ngOnInit(): void { 
    this.view$.pipe(takeUntilDestroy(this), tap(x => console.log("selected", x)))
      .subscribe((v: IView<ItemDto>) => this.focusedItem = (!!v && !!v.selected && !!v.selected.items) ? <ItemDto>(v.selected.items) : undefined);
  }

Nothing special really.

All this code is in component managed by router (router-outlet).
When i navigate to another route and than back takeUntilDestroy should unsubscribe from view$.
But it is not.

If i repeate navigate to another route and than back for 4 times and than produce state change i got 4 messages

There should be ony one message !

Is there any limitations using takeUntilDestroy or i'm missing something ?

[armanozak]

Hi @sebitsi,

Thank you. Yes, apparently, it doesn't work anymore.

We learned that Ivy collects lifecycle hook methods before initialization and does not call the method on the component instance. Therefore, modifying/patching of ngOnDestroy has to happen before component initialization. I made a quick demo for us to see the problem.

In case of absolute need, you may use this library. It has a decorator which patches the component constructor before an instance is created. However, be warned: Custom decorators are by default not tree-shakable. We have not tested this common knowledge against the library though.

We will clear all uses of takeUntilDestroy in built-in packages.

[sebitsi]

No problem. I like to help if i can.

We will clear all uses of takeUntilDestroy in built-in packages.

What exactly this mean ?
Will you replace takeUntilDestroy with classic takeUntil ?

Can you share you approach ? Thanks...

[armanozak]

On components and directives, we will most probably use a service for subscriptions provided and injected at directive-level. On services, depending on the case, takeUntil or old-school unsubscribe.

[sebitsi]

Is service you mentioned already exists ?

[armanozak]

It soon will. 🙂

#4838

[sebitsi]

@armanozak Great work !

I took a peek into documentation file.

I like idea how you solved this problem. On the other hand i don't like method names.

For example:

this.subscription.subscribe(this.count$, console.log);

Technically this method return subscription and from this point of view name "subcribe" is ok.
But from user point of view we basically register our observable (this.count$) to subscription service.

So, for me, method names like: register, unRegister, attach, detach, etc. are more descriptive.

Of course this is my personal opinion. And as i said i took only a peek. :)

Regards,

[armanozak]

Hi @sebitsi,

I think you took an early peek, because we felt exactly and changed those names before the PR was merged. 🙂

Well, we did not use register or attach, but we did not use subscribe or unsubscribe either. The method names are addOne, removeOne, closeOne, closeAll, and reset.

I hope this works for you, too.

I wish you a cheerful day.

[sebitsi]

@armanozak

Of course this changes work for me to.
It will not make mess with observable subscribe.

Regards.