[Bug]: Missing ng* parameters for elements injected with ng-content (Angular)

[markusnissl]

Describe the bug

I am overriding the template in the render method using Angular as follow:

render: (badge) => ({
        template: `
        <ds-badge variant="${badge.variant}" size="${badge.size}" label="${badge.label}">
            <ds-counter slot="start" type="${badge.icon}">10</ds-counter>
        </ds-badge>
        `
    })

where the ds-badge component contains following HTML

<ng-content select="[slot=start]"/>
<span class="badge-text">{{label}}</span>
<ng-content select="[slot=end]"/>

This results in docs being rendered as the following HTML:

<ds-badge variant="primary" size="medium" label="LABEL" _nghost-ng-c3862264782="" ng-reflect-variant="primary" ng-reflect-size="medium" ng-reflect-label="LABEL" class="medium primary shared">
  <ds-counter slot="start" type="primary">10</ds-counter>
  <span _ngcontent-ng-c3862264782="" class="badge-text">LABEL</span>
</ds-badge>

As soon as I change a property in the controls, it gets updated to (which is correct)

<ds-badge variant="primary" size="medium" label="LABE" _nghost-ng-c3862264782="" ng-reflect-variant="primary" ng-reflect-size="medium" ng-reflect-label="LABE" class="medium primary shared">
  <ds-counter slot="start" type="primary" _nghost-ng-c2123879280="" ng-reflect-type="primary" class="medium primary shared">10</ds-counter>
  <span _ngcontent-ng-c3862264782="" class="badge-text">LABE</span>
</ds-badge>

So, the initial rendering of the docs somehow does not initialize the elements injected with ngcontent as the _ng* parameters are missing on the ds-counter component.

To Reproduce

https://stackblitz.com/~/github.com/markusnissl/storybook-demo

You see the background color of the number 10 is changing after the first interaction (Badge Component, With Counter).

System

Storybook Environment Info:

  System:
    OS: Linux 5.0 undefined
    CPU: (8) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Shell: 1.0 - /bin/jsh
  Binaries:
    Node: 18.18.0 - /usr/local/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 9.4.2 - /usr/local/bin/npm <----- active
    pnpm: 8.9.2 - /usr/local/bin/pnpm
  npmPackages:
    @storybook/addon-essentials: ^7.5.3 => 7.5.3 
    @storybook/addon-interactions: ^7.5.3 => 7.5.3 
    @storybook/addon-links: ^7.5.3 => 7.5.3 
    @storybook/addon-styling: ^1.3.7 => 1.3.7 
    @storybook/angular: ^7.5.3 => 7.5.3 
    @storybook/blocks: ^7.5.3 => 7.5.3 
    @storybook/testing-library: ^0.2.2 => 0.2.2 
    storybook: ^7.5.3 => 7.5.3

Additional context

No response

[jagdishpatil02]

Hi, from what I have observed, it seems that your counter component is experiencing an issue with loading. After examining 'badge.component.stories.ts,' I noticed that you declared a component named 'ds-badge,' and as a result, it is loading the badge component first. When I changed it to the counter component, it loaded the counter component instead. Therefore, you may need to maintain two separate files for this

[markusnissl]

Hi @jagdishpatil02,

thanks for reply. I want to visualize the counter inside the the badge component. I cannot have two separate files for this use case. The question is why on the initial load, the counter is not rendered in a different color and I have to change the type from the arguments that it works. I think this is a bug in storybook.

[markusnissl]

I just found applying the metadata on the "meta"-level works correctly, but why should not I have the option to apply it on the story level, if moduleMetadata can be applied here as well.

[jagdishpatil02]

@markusnissl, Yup that might be the bug, also there is one more property i.e. subcomponents, maybe you can try that out.

[Marklb]

I see the problem and I have a potential fix. I am testing my fix and will try to submit a PR for it tomorrow, unless I find that my change breaks something else or someone else takes care of it first.

It seems to be a problem that I was worried about when I first saw that part of the code, but I hadn't seen any issues about it. Now that I am debugging it, I am really surprised there haven't been more issues for this.

Workaround

You can disable inline rendering for that story, so it is rendered in an iframe and doesn't inherit the NgModule built for the previous story, which seems to be happening currently.

export const WithCounter: StoryWithCounter = {
  parameters: {
    docs: {
      story: {
        inline: false,
      },
    },
  },
  ...,
}

Problem

The problem appears to be the following block, which I think should not be avoiding the rebuilding of the NgModule. It may be an unnecessary step in most scenarios, but causes issues if the metadata changes, when decorators or render function are called. It also causes the metadata from the first story that uses that component to carry over to other stories, which is the problem here.

storybook/code/frameworks/angular/src/client/angular-beta/StorybookWrapperComponent.ts

Lines 63 to 79 in bffa2db

	// Only create a new module if it doesn't already exist
	// This is to prevent the module from being recreated on every story change
	// Declarations & Imports are only added once
	// Providers are added on every story change to allow for story-specific providers
	let ngModule = componentNgModules.get(storyComponent);
	if (!ngModule) {
	@NgModule({
	declarations,
	imports,
	exports: [...declarations, ...imports],
	})
	class StorybookComponentModule {}
	componentNgModules.set(storyComponent, StorybookComponentModule);
	ngModule = componentNgModules.get(storyComponent);
	}

[markusnissl]

@Marklb
thanks for you response and working on the issue. Highly appreciated!

Is this issue also related to my other Angular issue I created a few months ago of reusing the same story multiple times in docs? (#23875)

[valentinpalkovic]

@Marklb I would appreciate any kind of help improving this logic! This logic was mainly introduced to circumvent the "Type X is part of the declarations of 2 modules: StorybookComponentModule and StorybookComponentModule! ..." error. I don't like this part of the implementation and I would like to get rid of the caching mechanism completely. I am curious, whether you can find out the root cause of the Error and solving it differently.

[Marklb]

@markusnissl I don't think they are related, but may have actually got an idea about that one from something else I saw, while I was debugging this one. I can't check at the moment, but I will check what I am thinking tomorrow.

[Marklb]

@valentinpalkovic That error is what I thought the caching may have been for, and is why I was going to add more tests before I opened a PR. I have some other scenarios I was going to test for, but that may be a bigger issue. I will take a look, though.

[Marklb]

The multiple declarations did end up being a problem, when I started testing. I thought I had an idea, but I ended up finding similar problems. My change would fix some cases, but break others.

I am recreating the problem in an Angular app that has a very overly simplified implementation of what the Storybook framework does and I am going to see if that helps get some ideas from someone that maybe knows the internals Angular well, but isn't familiar with Storybook.

[Marklb]

I am still testing it, but I opened a draft PR with the change that seems to be working in a local reproduction. I am still not positive that it is the correct fix, but it does work on the cases that caused me to abandon my first solution. #24982

My solution is still the original idea of removing the cached NgModules, but I added a lock to make sure multiple Angular apps are not getting bootstrapped at the same time.