[bug] Fix automatic Angular module metadata extraction for forRoot imports #7224
Conversation
…tps://github.com/pascaliske/storybook into 7106-angular-fix-automatic-component-declaration
pascaliske
requested review from
alterx,
igor-dv and
thomasbertet
as code owners
|
This pull request is automatically deployed with Now. Latest deployment for this branch: https://monorepo-git-fork-pascaliske-7106-angular-fix-auto-2d5086.storybook.now.sh |
pascaliske
changed the title
|
Please don't close this. Angular is broken currently. |
| @@ -31,11 +31,14 @@ const createComponentFromTemplate = (template: string) => { | |||
| })(componentClass); | |||
| }; | |||
| const extractNgModuleMetadata = (importItem: any): NgModule => { | |||
| const target = importItem && importItem.ngModule ? importItem.ngModule : importItem; | |||
There was a problem hiding this comment.
Instead of checking if importItem exists can we instead add an interface for importItem: any? I haven't checked but maybe angular already provide such an interface. Or we just write down what we need
interface StorybookAngularMetaData {
ngModule?: AngularModule;
}
Though, as I am writing this I just noticed we need dynamic access to importItem[decoratorKey]
@ndelangen haven't we encountered the same type conflict in one of your PRs where we need some fix types and additionally [key: string]: object ?
@pascaliske what do you think?
Since this is urgent I don't want to achieve the most beautiful solution. We can leave this as is if it'll take further discussion 🙂
There was a problem hiding this comment.
It would definitely be good to type out importItem with an interface but we still need to check for the existence to gracefully cover edge cases where the item could be undefined, I think...
There was a problem hiding this comment.
LGTM - just left 1 comment
Though, by clicking through the demo I found this and it throws an error
https://monorepo-git-fork-pascaliske-7106-angular-fix-auto-2d5086.storybook.now.sh/examples/angular-cli/?path=/story/custom-dependencies--inputs-and-inject-dependencies
Is this related to this PR?
https://monorepo-git-fork-pascaliske-7106-angular-fix-auto-2d5086.storybook.now.sh/examples/angular-cli/?path=/story/addon-centered--centered-component as well throws an error but seems unrelated 🙇
Edit Compared it with the current published storybook version https://monorepo.storybook.now.sh/examples/angular-cli/?path=/story/custom-dependencies--inputs-and-inject-dependencies
It's not related
|
One thing before merging this: Can you add a story to the angular example for having at least a manual test for the moment? I can do it as well. Just let me know |
See #7224 for more deatil on the bugfix.
Just pushed a story |
|
I also added the missing storyshots which I forgot... 😄 |
Issues: #7106, #7157
What I did
As described in #7106 the automatic detection for component declarations in Angular storybooks does not work with the really common pattern of
forRoot-imports. This PR fixes the behaviour.The
importItemfor extracting theNgModule-metadata will be checked for a keyngModulefirst which exists in the case of anforRoot-import because the method does not return theNgModuledirectly but an object containing it instead (calledModuleWithProviders<MyModule>).Besides that this PR fixes the error mentioned in #7157 which will be thrown if the decorator key
__annotations__can not be found on the target which is the case on importing AOT-compiled modules. I added another condition for checking if the decorator key is defined.How to test
The additional example stories introduced in #6346 will still suffice for testing.