ngneat / until-destroy

🦊 RxJS operator that unsubscribe from observables on destroy
https://netbasal.com/
MIT License
1.74k stars 100 forks source link

ERROR Error: ASSERTION ERROR: token must be defined [Expected=> null != undefined <=Actual] #232

Closed dancornilov closed 1 year ago

dancornilov commented 1 year ago

Hey,

I would like to extend my heartfelt appreciation for your remarkable contribution to the open-source community. Your package has become an integral part of many of my projects, and its impact on their success cannot be overstated. However, I have recently encountered a minor yet critical bug that poses a significant challenge, particularly in large-scale projects.

This issue manifests when using your package within an Angular component that extends the ControlValueAccessor.

ERROR Error: ASSERTION ERROR: token must be defined [Expected=> null != undefined <=Actual]

image

To facilitate a thorough understanding of the problem, I have prepared a default Angular project that can be used to reproduce the issue. You can access it by following this link: https://github.com/dancornilov/va-until-destroy-bug

Thank you for your time and consideration.

arturovt commented 1 year ago

Hey, thanks for the repro.

Note we already had similar issues before.

TLDR:

// minute-counter.component.ts
providers: [ValueAccessorBase.getProviderConfig(forwardRef(() => MinuteCounterComponent))]

// value-accessor-base.class.ts
static getProviderConfig(useExisting: any): Provider {
  return {
    provide: NG_VALUE_ACCESSOR,
    useExisting: useExisting,
    multi: true
  };
}

Your current implementation contains the following code:

providers: [ValueAccessorBase.getProviderConfig(MinuteCounterComponent)]

But you're using forwardRef inside the getProviderConfig which isn't correct because the class reference should be captured before. The TS compiler generates a bit different code when the @UntilDestroy decorator is used on the class.

When there is no @UntilDestroy() decorator:

class MinuteCounterComponent extends ValueAccessorBase { ... }

MinuteCounterComponent.ɵcmp = ɵɵdefineComponent({
  type: MinuteCounterComponent,
  features: [
    ɵɵProvidersFeature([
      ValueAccessorBase.getProviderConfig(MinuteCounterComponent),
    ]),
    ɵɵInheritDefinitionFeature,
  ],
});

In the example above the MinuteCounterComponent is already "available" to be referenced and used below when the getProviderConfig is called, so forwardRef wouldn't even be required.

When the decorator is used:

var _class;
var MinuteCounterComponent_1;

let MinuteCounterComponent = (MinuteCounterComponent_1 =
  ((_class = class MinuteCounterComponent extends ValueAccessorBase {}),
  (_class.ɵcmp = ɵɵdefineComponent({
    type: _class,
    features: [
      ɵɵProvidersFeature([
        ValueAccessorBase.getProviderConfig(MinuteCounterComponent_1),
      ]),
      ɵɵInheritDefinitionFeature,
    ],
  })),
  _class));

MinuteCounterComponent = MinuteCounterComponent_1 = __decorate(
  [(0, UntilDestroy)()],
  MinuteCounterComponent
);

The MinuteCounterComponent_1, which is passed to getProviderConfig, equals undefined at the moment the function is being called. Because it's set later after the expression is evaluated completely. At the end the providers array has useExisting: forwardRef(() => undefined).

dancornilov commented 1 year ago

Thanks for quick and descriptive answer, your insights have shed light on the situation.

I wonder if there is a possibility to enhance the error message by incorporating additional validation or implementing supplementary logic. This improvement would greatly benefit developers encountering similar scenarios.

Interestingly, I have a project with a similar implementation of ControlValueAccessor & @UntilDestroy on Angular 14, which functions without encountering any errors. It is worth noting that this error only started to manifest in Angular 15+.

arturovt commented 1 year ago

Because you're targetting es2022.

arturovt commented 1 year ago

I wonder if there is a possibility to enhance the error message by incorporating additional validation or implementing supplementary logic.

We can't because the error belongs to vague use-cases. This may happen in any implementation (e.g. ReactJS) using TypeScript and decorators.

dancornilov commented 1 year ago

Thanks again for your work 🙏. (I have no other comments)