Problem when building project

[jbx-alex]

Hi, I am using the library @wishtack/reactive-component-loader that import this library and when building my project I have some errors.

I am using Angular 7 with Meteor 1.8 with MeteorCLI -> https://github.com/Urigo/angular-meteor/tree/master/examples/MeteorCLI/all-in-one

Yjaaidi believes that it may be because this file ng-dynamic-component/dynamic/io.service is bundled as is even though it's using ES Module.

Is it possible that ng-component-dynamic exposes IoService here? https://github.com/gund/ng-dynamic-component/blob/master/src/dynamic/index.ts

link of the issue: https://github.com/wishtack/wishtack-steroids/issues/173

Thank you in advance.

[gund]

Hi, I see what's the issue.

The thing is that IoService is private internal service that was not meant to be exposed as a public API.

So that library basically accessed private API via import { IoService } from 'ng-dynamic-component/dynamic/io.service'; import which should not have worked in the first place, unless they've done some hacks to make that work...

So I think that has to be changed inside that library in order to work properly because I really do not want to expose that service as public API.

[jbx-alex]

Perfect, thanks for answering

[yjaaidi]

Hi everyone, In fact, @wishtack/reactive-component-loader is highly coupled to ng-dynamic-component as it is meant to inject & lazy load components with one single directive. I only saw three possibilities at that time:

1. Rewriting similar logic from ng-dynamic-component in @wishtack/reactive-component-loader,
2. Hacking into IoService,
3. Implementing dynamic component injection with inputs & outputs in Angular: https://github.com/angular/angular/issues/15360#issuecomment-460410260

Any other suggestions?

Great job @gund for this library by the way 😊

See ya!

[gund]

@yjaaidi thanks for the feedback.

I would like to help you to solve the problem but I'm failing to see what exactly @wishtack/reactive-component-loader is trying to do and thus what part of this library it requires to use...

Maybe if I understand better we can find more proper solution without hacking around private APIs =)

[yjaaidi]

Hi Alex,

I think that https://github.com/gund/ng-dynamic-component/issues/245 is related to this issue.

@wishtack/reactive-component-loader lazy loads components on demand which could be inserted using ngComponentOutlet but in order to pass inputs and outputs, it's using ng-dynamic-component. Cf. https://medium.com/wishtack/angular-lazy-loading-without-router-471166580c86

One of the ways of using @wishtack/reactive-component-loader is the wtLazy structural directive (which is not documented yet) and which works like this https://github.com/wishtack/wishtack-steroids/blob/4806d54c6b919e4bb03d8487936c027236a2520c/packages/reactive-component-loader/src/lib/lazy/lazy.directive.spec.ts#L19

It's clearly a wrapper of ng-dynamic-component structural directive. In order to make it work, I had to hack through, provide IoService and manually wrap the directive methods. Cf. https://github.com/wishtack/wishtack-steroids/blob/master/packages/reactive-component-loader/src/lib/lazy/lazy.directive.ts

I guess that this wtLazy structural directive hack is causing trouble and issues like: https://github.com/gund/ng-dynamic-component/issues/245 & https://github.com/wishtack/wishtack-steroids/issues/180.

The only solutions I can imagine are:

• Getting rid of wtLazy structural directive for the moment in @wishtack/reactive-component-loader.
• Exporting IoService as θIoService in ng-dynamic-component.

Do you have any other quick magical solution? 😊

Thank you!

[gund]

Hi @yjaaidi,

Thanks for your clarifications.

Now I checked the wtLazy directive and have clearer idea of how it's working.

Regarding your proposed solutions to the issue:

• Getting rid of course will remove the problem but also the feature so does not look too compelling to me =)
• Exporting IoService even with private prefix is not a solution since it's going to be public and the whole point is not expose it at all since it's just and implementation detail in current state. I cannot guarantee it's API contract and thus cannot make it public.

I think we can simply rewrite that directive in a more maintainable and clearer way.

I realized that it is manually creating 2 directives internally: DynamicDirective and NgComponentOutlet - which is an antipattern in angular and overall in systems with DI.

So what I would suggest is the following:

1. Remove manual creation of DynamicDirective, and simply extend from it: This will allow all the things it requires to be inherited automatically and then you just need to define your normal inputs and proxy them back to DynamicDirective. This can be simply done by using setters:

@Directive({
  selector: '[wtLazy]',
})
export class LazyDirective extends DynamicDirective {
@Input('wtLazy') location: ComponentLocation; // This stays as is because it's custom input
@Input() set wtLazyInputs(val: Inputs) {this.ndcDynamicInputs = val}
@Input() set wtLazyOutputs(val: Outputs) {this.ndcDynamicOutputs = val}
}

Also you can specify getters for those inputs if required. 2. Remove manual creation of NgComponentOutlet and make it to be required directive to be set along with wtLazy directive: This will first decouple your code from that directive and second promote composition of directives like so: <ng-container [ngComponentOutlet]="MyComponent" wtLazy></ng-container>. Also it will automatically work for DynamicDirective since it was designed to work alongside such directives and because your directive extends DynamicDirective it will simply inherit the same functionality.

Let me know what you think about it?

[yjaaidi]

Hi Alex,

Thank you for your detailed review. Actually, extending DynamicDirective was the first thing I experience and I don't really remember why it went wrong and end up with hacking through and coupling the implementation to DynamicDirective internals.

I'll give it a try again 😊

Thanks.

[yjaaidi]

Oh! Now I remember the limitation I met. I need to inject ReactiveComponentLoader service.

That would look something like this:

@Directive({
  selector: '[wtLazy]',
})
export class LazyDirective extends DynamicDirective {
  @Input('wtLazy') location: ComponentLocation; // This stays as is because it's custom input
  
  constructor(private _reactiveComponentLoader: ReactiveComponentLoader) {
    super(...);
  }
}

So I have to call the parents constructor and inject the services it needs... which include IoService that I have to import... but it's internal :/

[gund]

Oh I see. That's unfortunate...

Well we can go around from the other side - extend NgComponentOutlet directive.

Essentially you want your custom component resolution to be integrated with ndcDynamicInputs/Outputs so you can very well provide your extended version of NgComponentOutlet to the ndcDynamicInputs directive by registering it under COMPONENT_INJECTOR inside of your directive's providers like so:

@Directive({
  selector: '[wtLazy]',
  providers: [
    { provide: COMPONENT_INJECTOR, useExisting: forwardRef(() => LazyDirective) },
  ],
})
class LazyDirective extends NgComponentOutlet implements OnChanges {
  @Input('wtLazy') location: ComponentLocation;

  ngOnChanges(changes: SimpleChanges) {
    ... // Sync your component resolution algorithm with `NgComponentOutlet` as you did before
  }
}

And then usage will become like this:

<ng-container [wtLazy]="location" [ndcDynamicInputs]="inputs"></ng-container>

You can even go further and rename ndcDynamicInputs/Outputs to match wtLazyInputs/Outputs name by proxying it like so:

@Directive({
  selector: '[wtLazyInputs][wtLazyOutputs]',
  inputs: ['ndcDynamicInputs: wtLazyInputs', 'ndcDynamicOutputs: wtLazyOutputs'],
})
class LazyInputsDirectives extends DynamicDirective {}

NOTE: Here you do not need to inject anything at all, simply forwarding same inputs under renamed props.

And final usage should look like this:

<ng-container [wtLazy]="location" [wtLazyInputs]="inputs" [wtLazyOutputs]="outputs"></ng-container>

Which, I believe, is the same API you are having right now =)

How do you feel about it?

EDIT: If everything above feels too complicated there is last resort option - I can retrieve IoService manually from the injector instead of the constructor - in which case you will be able to keep everything as is. However I still think that current implementation of wtLazy directive is too coupled and so is fragile to changes from both NgComponentOutlet and DynamicDirective, thus my advice is to try to refactor it to something like I wrote above =)

Your call anyways)

[yjaaidi]

Hi Alex, thank you very much for your help. The idea is brilliant!!! I tried it but I encountered the following issues:

1. LazyDirective injection issue

I guess you meant:

{ provide: COMPONENT_INJECTOR, useValue: forwardRef(() => LazyDirective) }

instead of useExisting because it should provide the LazyDirective class and not the instance which is then injected manually by DynamicDirective.

Problem is that this always ends up with IoService: ComponentInjector was not set! Please call init() method! error.

First, I realized that COMPONENT_INJECTOR is injecting DynamicComponent instead of LazyDirective. I tried overriding it using the componentInjector parameter of DynamicModule.withComponents() and then I got LazyDirective in _componentInjectorType but DynamicDirective injector returns null when injecting LazyDirective.

It behaves like if skipself flag was on... but it's not. 🤔

2. COMPONENT_INJECTOR is not exposed

If this solution works, we should export COMPONENT_INJECTOR in index.ts to make it accessible to reactive-component-loader.

3. [minor] Can't extend DynamicDirective

I guess we can't extend DynamicDirective like this:

class LazyInputsDirectives extends DynamicDirective {}

because IoService would not be provided anymore and LazyInputsDirectives would have to provide it which will require to make it public too.

This is not important as I don't mind renaming ndcDynamicInputs to wtLazyInputs.

For the moment, I just removed the wtLazy directive as it's not documented anyway as it was just a hacky proof of concept implementation. If we can solve points 1 & 2, we could bring back the wtLazy directive and remove all the hacks 😊

Thanks a lot for your help!

[gund]

Hey, not sure if it's relevant but since v10.4.0 the IoService is now stable and part of public API. I'm closing this issue as it's outdated. If you want to reopen the discussion please feel free to open a new issue.