Angular - Avoid component redeclaration without additional configuration

[MaximSagan]

Issue: #5542

Previously I submitted PR #6346, which has just been merged. This fixed the above-mentioned issue, but it was somewhat unsatisfactory because it requires the story definition to explicitly state that the component must not be re-declared (through the additional "requiresComponentDeclartion" flag introduced in that PR). While this works, it adds an extra layer of configuration to achieve functionality that should work out of the box.

What I did

Instead of having the "requiresComponentDeclaration" flag, no additional configuration is required, and Storybook looks at the imported modules and their declarations to determine whether the main component has already been declared. (This is done by extracting the NgModule metadata.) If the component has already been declared, it will not be redeclared, ensuring the "MyComponent is part of the declarations of 2 modules" error does not occur. If it has not been declared, it will be declared as part of the "DynamicModule" that includes the Storybook "AppComponent" (as is the current behavior).

How to test

The additional example stories introduced in #6346 will still suffice for testing.

[vercel]

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

Latest deployment for this branch: https://monorepo-git-fork-maximsagan-angular-avoid-compone-4c0de9.storybook.now.sh

[igor-dv]

@shilman , are you sure we want the #6666 PR to be merged ? 😈

[igor-dv]

I like this automation.

1. What can we do so the helpers.ts will be more friendly ? It's already hard to figure it out, so maybe worth extracting things
2. was there any release containing this requiresComponentDeclartion prop for story ? Can its removal be considered as breaking change ?

[shilman]

@igor-dv I don't know whether we should merge it, that's why I kicked it over to you 😂

As far as I understand it, according to @MaximSagan the original PR (which was already merged) was kind of a breaking change, and this achieves the same result without the break.

If that's the case and it's actually an improvement over what got merged, I think we should probably try to get it into 5.1. If I misunderstood any of that, we might want to punt it until 5.2.

What do you think?

[igor-dv]

So the previous one was merged a few days ago. I assume it's ok to bring this in then.

I would like to see the simplification of the helpers.ts though

[MaximSagan]

@igor-dv @shilman I can work on simplifying helpers.ts, but shall that be part of this PR?

[igor-dv]

I am ok it will be a separate one. But we need something to do with this.
@kroeder, I don't remember already, did you suggest to combine the angular helpers from storyshots with the helpers from the app/angular in someway ? Back then I wanted to extract them all to a separate package.

[igor-dv]

Approving it, though changes for the helpers are needed