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.

Sign up for GitHub

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

Jump to bottom

[bug] Fix automatic Angular module metadata extraction for forRoot imports #7224

Merged
merged 6 commits into from
Jul 25, 2019
Merged

[bug] Fix automatic Angular module metadata extraction for forRoot imports #7224

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
22ef233
fix(angular): fix ng module metadata extraction for forRoot imports
Jun 28, 2019
5ad8d18
fix(angular): fix ng module metadata extraction for forRoot imports
Jun 28, 2019
6bf4e50
Merge branch '7106-angular-fix-automatic-component-declaration' of ht…
Jun 28, 2019
3a4f414
test(angular): add test story for angular forRoot bugfix
Jul 23, 2019
34386b9
test(angular): update storyshots for angular stories
Jul 23, 2019
38ba058
Merge branch 'next' into pr/pascaliske/7224
kroeder Jul 25, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions addons/storyshots/storyshots-core/src/frameworks/angular/helpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,14 @@ const createComponentFromTemplate = (template: string) => {
})(componentClass);
};
const extractNgModuleMetadata = (importItem: any): NgModule => {
const target = importItem && importItem.ngModule ? importItem.ngModule : importItem;
Copy link
Member

@kroeder kroeder Jul 23, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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...

const decoratorKey = '__annotations__';
const decorators: any[] =
Reflect && Reflect.getOwnPropertyDescriptor
? Reflect.getOwnPropertyDescriptor(importItem, decoratorKey).value
: importItem[decoratorKey];
Reflect &&
Reflect.getOwnPropertyDescriptor &&
Reflect.getOwnPropertyDescriptor(target, decoratorKey)
? Reflect.getOwnPropertyDescriptor(target, decoratorKey).value
: target[decoratorKey];

if (!decorators || decorators.length === 0) {
return null;
Expand Down Expand Up @@ -86,15 +89,15 @@ export const initModuleData = (storyObj: NgStory): any => {
? createComponentFromTemplate(template)
: component;

const componentRequiesDeclaration =
const componentRequiresDeclaration =
isCreatingComponentFromTemplate ||
!getExistenceOfComponentInModules(
component,
moduleMetadata.declarations,
moduleMetadata.imports
);

const componentDeclarations = componentRequiesDeclaration
const componentDeclarations = componentRequiresDeclaration
? [AppComponent, AnnotatedComponent]
: [AppComponent];

Expand Down
13 changes: 8 additions & 5 deletions app/angular/src/client/preview/angular/helpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,14 @@ const createComponentFromTemplate = (template: string, styles: string[]) => {
};

const extractNgModuleMetadata = (importItem: any): NgModule => {
const target = importItem && importItem.ngModule ? importItem.ngModule : importItem;
const decoratorKey = '__annotations__';
const decorators: any[] =
Reflect && Reflect.getOwnPropertyDescriptor
? Reflect.getOwnPropertyDescriptor(importItem, decoratorKey).value
: importItem[decoratorKey];
Reflect &&
Reflect.getOwnPropertyDescriptor &&
Reflect.getOwnPropertyDescriptor(target, decoratorKey)
? Reflect.getOwnPropertyDescriptor(target, decoratorKey).value
: target[decoratorKey];

if (!decorators || decorators.length === 0) {
return null;
Expand Down Expand Up @@ -110,15 +113,15 @@ const initModule = (storyFn: IStoryFn) => {
? createComponentFromTemplate(template, styles)
: component;

const componentRequiesDeclaration =
const componentRequiresDeclaration =
isCreatingComponentFromTemplate ||
!getExistenceOfComponentInModules(
component,
moduleMetadata.declarations,
moduleMetadata.imports
);

const componentDeclarations = componentRequiesDeclaration
const componentDeclarations = componentRequiresDeclaration
? [AppComponent, AnnotatedComponent]
: [AppComponent];

Expand Down
16 changes: 14 additions & 2 deletions examples/angular-cli/src/stories/module-context/chips.module.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { NgModule } from '@angular/core';
import { NgModule, ModuleWithProviders } from '@angular/core';
import { CommonModule } from '@angular/common';
import { ChipComponent } from './chip.component';
import { ChipsGroupComponent } from './chips-group.component';
Expand All @@ -16,4 +16,16 @@ import { CHIP_COLOR } from './chip-color.token';
},
],
})
export class ChipsModule {}
export class ChipsModule {
public static forRoot(): ModuleWithProviders {
return {
ngModule: ChipsModule,
providers: [
{
provide: CHIP_COLOR,
useValue: '#eeeeee',
},
],
};
}
}
62 changes: 62 additions & 0 deletions examples/angular-cli/src/stories/module-context/module-context-forRoot.stories.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
import { storiesOf, moduleMetadata } from '@storybook/angular';
import { withKnobs, text, object } from '@storybook/addon-knobs';
import { action } from '@storybook/addon-actions';

import { ChipsModule } from './chips.module';
import { ChipsGroupComponent } from './chips-group.component';
import { ChipComponent } from './chip.component';

storiesOf('Custom|Feature Module as Context with forRoot', module)
.addDecorator(withKnobs)
.addDecorator(
moduleMetadata({
imports: [ChipsModule.forRoot()],
})
)
.add(
'Component with self and dependencies declared in its feature module',
() => {
const props: { [K in keyof ChipsGroupComponent]?: any } = {
chips: object('Chips', [
{
id: 1,
text: 'Chip 1',
},
{
id: 2,
text: 'Chip 2',
},
]),
removeChipClick: action('Remove chip'),
removeAllChipsClick: action('Remove all chips clicked'),
};
return {
component: ChipsGroupComponent,
props,
};
},
{
notes: `This component includes a child component, a pipe, and a default provider, all which come from
the specified feature module.`,
}
)
.add('Component with default providers', () => {
const props: { [K in keyof ChipComponent]?: any } = {
displayText: text('Display Text', 'My Chip'),
removeClicked: action('Remove icon clicked'),
};
return {
component: ChipComponent,
props,
};
})
.add('Component with overridden provider', () => {
const props: { [K in keyof ChipComponent]?: any } = {
displayText: text('Display Text', 'My Chip'),
removeClicked: action('Remove icon clicked'),
};
return {
component: ChipComponent,
props,
};
});