Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NgxPopperModule.forRoot should be only used with AppModule #117

Open
steve3d opened this issue Aug 6, 2019 · 7 comments
Open

NgxPopperModule.forRoot should be only used with AppModule #117

steve3d opened this issue Aug 6, 2019 · 7 comments
Labels

Comments

@steve3d
Copy link
Contributor

steve3d commented Aug 6, 2019

to my understanding, the module.forRoot should be only used in bootstrapped module, and any options passed by forRoot should be inherit into any other module.

but now ngx-popper need to use forRoot on every module and repeatly set the options. the options didn't inherit from the bootstrapped module.

So I don't know if this is the design or something wrong?

@steve3d
Copy link
Contributor Author

steve3d commented Aug 6, 2019

just checked the source, you are using the provider to set the default options, and provider is module indepentend, so by your design this is the correct result.

you might need to change from plain string provide: 'popperDefaults' to


export const POPPER_DEFAULTS = new InjectionToken<PopperContentOptions>('popperDefaults');

public static forRoot(popperBaseOptions: PopperContentOptions = {}): ModuleWithProviders {
  return {ngModule: NgxPopperModule, providers: [{provide: POPPER_DEFAULTS, useValue: popperBaseOptions}]};
}

this will make forRoot actually forRoot.

@steve3d
Copy link
Contributor Author

steve3d commented Aug 7, 2019

no, the problem is not the InjectionToken, because in module you write:

  providers: [
    {
      provide: 'popperDefaults', useValue: {}
    }]

this will make any module import use a empty option, to use a default option, we need to use forRoot in every module import to set the options, and also because of these lines, you all options can not be inherited into children modules of AppModule.

@MrFrankel
Copy link
Owner

MrFrankel commented Aug 14, 2019

@steve3d , thanks for bringing this up, sorry for the late reply, ill look into your PR

@MrFrankel MrFrankel added the bug label Aug 14, 2019
@steve3d
Copy link
Contributor Author

steve3d commented Aug 15, 2019

when can we expect the new version?

@tonysamperi
Copy link

I just stumbled across this, when upgrading a library of mine which uses NgxPopper.
I was previously using NgxPopper 6.0.7 and everything was fine.
But now with v 7.0.0 I got an error saying NullInjectorError: No provider for popperDefaults! which I had to solve by adding the forRoot to a secondary module (luckily it was only 1).

@tonysamperi
Copy link

@steve3d if you're interested I'm about to fork this project because I noticed that MrFrankel has not been answering lately, I need this and I don't want the extra forRoot and I need to make everything work with Angular 9 for a project I'm working on...

@tonysamperi
Copy link

@steve3d just released ngx-popperjs@8.0.3 with some improvements, like this and upgrade to @popperjs/core 2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants