[Bug]: NgModule is imported twice

[nzacca]

Describe the bug

When an NgModule follows the forRoot pattern (https://angular.io/guide/singleton-services#prevent-reimport-of-the-greetingmodule), the module appears to be loaded twice and produces a runtime error indicating that the module was already loaded.

TestModule is already loaded. Import it in the AppModule only
Error: TestModule is already loaded. Import it in the AppModule only
    at new TestModule (http://localhost:6006/app-test-stories.iframe.bundle.js:61:13)
    at Object.TestModule_Factory [as useFactory] (ng:///TestModule/ɵfac.js:4:10)
    at Object.factory (http://localhost:6006/vendors-node_modules_storybook_addon-docs_angular_index_js-node_modules_storybook_addon-essen-170b5a.iframe.bundle.js:43968:32)
    at R3Injector.hydrate (http://localhost:6006/vendors-node_modules_storybook_addon-docs_angular_index_js-node_modules_storybook_addon-essen-170b5a.iframe.bundle.js:43886:29)
    at R3Injector.get (http://localhost:6006/vendors-node_modules_storybook_addon-docs_angular_index_js-node_modules_storybook_addon-essen-170b5a.iframe.bundle.js:43787:23)
    at injectInjectorOnly (http://localhost:6006/vendors-node_modules_storybook_addon-docs_angular_index_js-node_modules_storybook_addon-essen-170b5a.iframe.bundle.js:36619:29)
    at ɵɵinject (http://localhost:6006/vendors-node_modules_storybook_addon-docs_angular_index_js-node_modules_storybook_addon-essen-170b5a.iframe.bundle.js:36623:59)
    at useValue (http://localhost:6006/vendors-node_modules_storybook_addon-docs_angular_index_js-node_modules_storybook_addon-essen-170b5a.iframe.bundle.js:43585:25)
    at R3Injector.resolveInjectorInitializers (http://localhost:6006/vendors-node_modules_storybook_addon-docs_angular_index_js-node_modules_storybook_addon-essen-170b5a.iframe.bundle.js:43827:9)
    at new EnvironmentNgModuleRefAdapter (http://localhost:6006/vendors-node_modules_storybook_addon-docs_angular_index_js-node_modules_storybook_addon-essen-170b5a.iframe.bundle.js:55368:14)

Note this pattern works fine in Angular 15 environment.

To Reproduce

// test.module.ts
import {
  ModuleWithProviders,
  NgModule,
  Optional,
  SkipSelf,
} from '@angular/core';

@NgModule()
export class TestModule {
  constructor(@Optional() @SkipSelf() parentModule?: TestModule) {
    if (parentModule) {
      throw new Error(
        'TestModule is already loaded. Import it in the AppModule only'
      );
    }
  }

  static forRoot(): ModuleWithProviders<TestModule> {
    return {
      ngModule: TestModule,
      providers: [],
    };
  }
}

// test.stories.ts
import { Component } from '@angular/core';
import { moduleMetadata, type Meta, type StoryFn } from '@storybook/angular';
import { TestModule } from './test.module';

@Component({
  selector: 'app-test',
  template: ``,
})
class TestComponent {}

export default {
  title: 'Module Test',
  component: TestComponent,
  decorators: [moduleMetadata({ imports: [TestModule.forRoot()] })],
} as Meta;

const Template: StoryFn<TestComponent> = (args: TestComponent) => ({
  props: args,
});

export const Test = Template.bind({});

System

Environment Info:

  System:
    OS: Windows 10 10.0.22621
    CPU: (24) x64 AMD Ryzen 9 3900X 12-Core Processor
  Binaries:
    Node: 18.14.2 - C:\dev\nodejs\node.EXE
    npm: 9.5.0 - C:\dev\nodejs\npm.CMD
  Browsers:
    Chrome: 110.0.5481.104
    Edge: Spartan (44.22621.1265.0), Chromium (110.0.1587.57)
  npmPackages:
    @storybook/addon-actions: 7.0.0-beta.55 => 7.0.0-beta.55
    @storybook/addon-essentials: 7.0.0-beta.55 => 7.0.0-beta.55
    @storybook/addon-interactions: 7.0.0-beta.55 => 7.0.0-beta.55
    @storybook/addon-links: 7.0.0-beta.55 => 7.0.0-beta.55
    @storybook/angular: 7.0.0-beta.55 => 7.0.0-beta.55
    @storybook/builder-webpack5: 7.0.0-beta.55 => 7.0.0-beta.55
    @storybook/testing-library: ^0.0.13 => 0.0.13

Additional context

Is it possible that the @SkipSelf is not being respected on the Module constructor parameter? Possibly related to this? #21243

Is it appropriate to import forRoot modules in the Story metadata? Do we need a "special" property for root imports perhaps?

[shilman]

@sheriffMoose @valentinpalkovic is this something either of you know about?

[valentinpalkovic]

Yes, working on it already.

[rezoled]

It doesn't happen only for a Module.forRoot(), it also happens for modules that are needed to be imported only once in the application like BrowserModule and BrowserAnimationsModule. Once you create a story that imports these modules (especially since BrowserAnimationsModule is needed) you get a runtime error:
Providers from the `BrowserModule` have already been loaded

Hey @valentinpalkovic, I would be happy to know of a timeline for a fix.
I upgraded to sb7 and my components won't work. I just want to know if I can wait for it or I need to downgrade to sb6.

Thanks!

[valentinpalkovic]

@rezoled the error should not occur for BrowserAnimationModule, because we handle this separately. Please provide a reproduction in a separate issue, because this is unrelated to this one.

[valentinpalkovic]

The timeline is to work on this bug this or next week.

[rezoled]

What about this error, is it the same bug?
NG04007: The Router was provided more than once. This can happen if 'forRoot' is used outside of the root injector. Lazy loaded modules should use RouterModule.forChild() instead

[valentinpalkovic]

That's related to the same error mentioned in this issue

[rezoled]

I think I understand the issue with BrowserAnimationsModule, when I import it like this:
imports: [BrowserAnimationsModule]
I get not errors, however when I specify configurations:

imports[
	BrowserAnimationsModule.withConfig({
		disableAnimations: isDev,
	}),
]

It throws Providers from the `BrowserModule` have already been loaded

[rezoled]

Another error I get is:

Type AccountNameComponent is part of the declarations of 2 modules: AccountNameModule and AccountNameModule! Please consider moving AccountNameComponent to a higher module that imports AccountNameModule and AccountNameModule. You can also create a new NgModule that exports and includes AccountNameComponent then import that NgModule in AccountNameModule and AccountNameModule

It seems that Angular tries to generate the same module twice for the same context.

[valentinpalkovic]

@rezoled

You can fix the browser animation error by applying the following code:

{
  ...
  imports:[isDev ? NoopAnimationsModule : BrowserAnimationsModule]
}

[valentinpalkovic]

Another error I get is:

Type AccountNameComponent is part of the declarations of 2 modules: AccountNameModule and AccountNameModule! Please consider moving AccountNameComponent to a higher module that imports AccountNameModule and AccountNameModule. You can also create a new NgModule that exports and includes AccountNameComponent then import that NgModule in AccountNameModule and AccountNameModule

It seems that Angular tries to generate the same module twice for the same context.

@rezoled This is another error and does not relate to this Github Issue. Please open a new issue with a reproduction.

[shilman]

Great Caesar's ghost!! I just released https://github.com/storybookjs/storybook/releases/tag/v7.0.0-rc.11 containing PR #21770 that references this issue. Upgrade today to the @next NPM tag to try it out!

npx sb@next upgrade --prerelease

[jinder]

@shilman we have upgraded to RC11 and we still see this error:

core.mjs:8405 ERROR Error: NG04007: The Router was provided more than once. This can happen if 'forRoot' is used outside of the root injector. Lazy loaded modules should use RouterModule.forChild() instead.
at Object.provideForRootGuard [as useFactory] (router.mjs:7009:1)
at Object.factory (core.mjs:8107:1)
at R3Injector.hydrate (core.mjs:8020:1)
at R3Injector.get (core.mjs:7908:1)
at injectInjectorOnly (core.mjs:633:1)
at Module.ɵɵinject (core.mjs:637:1)
at Object.RouterModule_Factory [as useFactory] (router.mjs:6962:1)
at Object.factory (core.mjs:8107:1)
at R3Injector.hydrate (core.mjs:8020:1)
at R3Injector.get (core.mjs:7908:1)

This is migrating an existing moduleMetadata that used to work in version 6.

[valentinpalkovic]

@jinder Please follow the migration guide: https://github.com/storybookjs/storybook/blob/v7.0.0-rc.11/MIGRATION.md#angular-application-providers-and-modulewithproviders

[jinder]

@valentinpalkovic thanks - this now works! We were getting some strange errors because a child module was importing BrowserAnimationsModule directly.

[valentinpalkovic]

@jinder i think BrowserAnimationsModule should always be imported on the root level and should never be imported by a child module if I am not wrong. Please open a new issue, if you still have problems

[jinder]

@valentinpalkovic I think you're right, however Angular doesn't error or warn you. It's only apparent when using Storybook and the errors were a bit confusing. It all works now though, thank you!

[rezoled]

I'm still getting duplicate module error:

Type AvatarComponent is part of the declarations of 2 modules: AvatarModule and StorybookComponentModule! Please consider moving AvatarComponent to a higher module that imports AvatarModule and StorybookComponentModule. You can also create a new NgModule that exports and includes AvatarComponent then import that NgModule in AvatarModule and StorybookComponentModule.

The test:

export default {
	component: AvatarComponent,
	decorators: [
		applicationConfig({
			providers: [importProvidersFrom(AvatarModule)],
		}),
	],
} as Meta;

avatar.module:

import { NgModule } from '@angular/core';
import { AvatarComponent } from './avatar.component';

@NgModule({
	declarations: [AvatarComponent],
	exports: [AvatarComponent],
})
export class AvatarModule {}

I tried running ``npx sb@next upgrade --prerelease``` but I got error while running migrations.
However I did update the pacakges:

		"@storybook/addon-essentials": "7.0.0",
		"@storybook/core-server": "7.0.0",
		"@storybook/angular": "7.0.0",

Is it something I'm doing wrong? Could it be because of the migration error?

[valentinpalkovic]

@rezoled please open a new issue and mention me, because this issue was about the forRoot pattern (and if possible, please provide a reproduction). I have already a solution in mind for your problem. I will take a look on Monday.

[rezoled]

No problem, opening a new issue. Meanwhile I checked and the problem was not from the broken migration