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.

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

Add moduleMetdata decorator for supplying common Angular metadata #2959

Merged
merged 17 commits into from
Feb 19, 2018
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
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
1 change: 1 addition & 0 deletions app/angular/index.d.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { NgModuleMetadata, ICollection } from './dist/client/preview/angular/types';
export { moduleMetadata } from './dist/client/preview/angular/decorators';

export interface IStorybookStory {
name: string;
Expand Down
2 changes: 2 additions & 0 deletions app/angular/src/client/index.js
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
export { storiesOf, setAddon, addDecorator, configure, getStorybook } from './preview';

export { moduleMetadata } from './preview/angular/decorators';
71 changes: 71 additions & 0 deletions app/angular/src/client/preview/angular/decorators.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
import { moduleMetadata } from './decorators';
Copy link
Member

@igor-dv igor-dv Feb 13, 2018

Choose a reason for hiding this comment

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

I think we also need to support an arrays concatenation...

for example:

const result = moduleMetadata({
  imports: [FooModule],
})(() => ({
  component: MockComponent,
  moduleMetadata: {
    imports: [BarModule],
  }
}));

I would expect to have both FooModule and BarModule

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have modified it to combine rather than override.


class MockModule {}
class MockModuleTwo {}
class MockService {}
class MockComponent {}

describe('moduleMetadata', () => {
it('should add metadata to a story without it', () => {
const result = moduleMetadata({
imports: [MockModule],
providers: [MockService],
})(() => ({
component: MockComponent,
}));

expect(result).toEqual({
component: MockComponent,
moduleMetadata: {
declarations: [],
entryComponents: [],
imports: [MockModule],
schemas: [],
providers: [MockService],
},
});
});

it('should combine with individual metadata on a story', () => {
const result = moduleMetadata({
imports: [MockModule],
})(() => ({
component: MockComponent,
moduleMetadata: {
imports: [MockModuleTwo],
providers: [MockService],
},
}));

expect(result).toEqual({
component: MockComponent,
moduleMetadata: {
declarations: [],
entryComponents: [],
imports: [MockModule, MockModuleTwo],
schemas: [],
providers: [MockService],
},
});
});

it('should return the original metadata if passed null', () => {
const result = moduleMetadata(null)(() => ({
component: MockComponent,
moduleMetadata: {
providers: [MockService],
},
}));

expect(result).toEqual({
component: MockComponent,
moduleMetadata: {
declarations: [],
entryComponents: [],
imports: [],
schemas: [],
providers: [MockService],
},
});
});
});
21 changes: 21 additions & 0 deletions app/angular/src/client/preview/angular/decorators.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { NgModuleMetadata } from './types';

export const moduleMetadata = (metadata: Partial<NgModuleMetadata>) => (storyFn: () => any) => {
const story = storyFn();
const storyMetadata = story.moduleMetadata || {};
metadata = metadata || {};
Copy link
Member

Choose a reason for hiding this comment

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

Can the default be introduced in the method signature?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's how I did it originally, but default params affect undefined or no value, but not null. So I'd need to add a check for null and assignment to {} anyway.


return {
...story,
moduleMetadata: {
declarations: [...(metadata.declarations || []), ...(storyMetadata.declarations || [])],
entryComponents: [
...(metadata.entryComponents || []),
...(storyMetadata.entryComponents || []),
],
imports: [...(metadata.imports || []), ...(storyMetadata.imports || [])],
schemas: [...(metadata.schemas || []), ...(storyMetadata.schemas || [])],
providers: [...(metadata.providers || []), ...(storyMetadata.providers || [])],
},
};
};
52 changes: 52 additions & 0 deletions docs/src/pages/basics/guide-angular/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -113,3 +113,55 @@ npm run storybook

Now you can change components and write stories whenever you need to.
You'll get those changes into Storybook in a snap with the help of webpack's HMR API.

## Module Metadata

If your component has dependencies on other Angular directives and modules, these can be supplied using the `moduleMetadata` property on an individual story:

```js
import { CommonModule } from '@angular/common';
import { storiesOf } from '@storybook/angular';
import { MyButtonComponent } from '../src/app/my-button/my-button.component';
import { MyPanelComponent } from '../src/app/my-panel/my-panel.component';
import { MyDataService } from '../src/app/my-data/my-data.service';

storiesOf('My Panel', module)
.add('Default', () => ({
component: MyPanelComponent,
moduleMetadata: {
imports: [CommonModule],
schemas: [],
declarations: [MyButtonComponent],
providers: [MyDataService],
}
}));
```

If you have metadata that is common between your stories, this can configured once using the `moduleMetadata()` decorator:

```js
import { CommonModule } from '@angular/common';
import { storiesOf, moduleMetadata } from '@storybook/angular';
import { MyButtonComponent } from '../src/app/my-button/my-button.component';
import { MyPanelComponent } from '../src/app/my-panel/my-panel.component';
import { MyDataService } from '../src/app/my-data/my-data.service';

storiesOf('My Panel', module)
.addDecorator(
moduleMetadata({
imports: [CommonModule],
schemas: [],
declarations: [MyButtonComponent],
providers: [MyDataService],
})
)
.add('Default', () => ({
component: MyPanelComponent
}))
.add('with a title', () => ({
component: MyPanelComponent,
props: {
title: 'Foo',
}
}));
```
Original file line number Diff line number Diff line change
@@ -1,5 +1,107 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`Storyshots Common ngModule metadata simple 1`] = `
<storybook-dynamic-app-root
cfr={[Function CodegenComponentFactoryResolver]}
data={[Function Object]}
target={[Function ViewContainerRef_]}
>
<storybook-simple-service-component>


<p>
Static name:
</p>


<ul>





</ul>


</storybook-simple-service-component>
</storybook-dynamic-app-root>
`;

exports[`Storyshots Common ngModule metadata template 1`] = `
<storybook-dynamic-app-root
cfr={[Function CodegenComponentFactoryResolver]}
data={[Function Object]}
target={[Function ViewContainerRef_]}
>
<ng-component>
<storybook-simple-service-component
ng-reflect-name="Static name"
>


<p>
Static name:
</p>


<ul>





</ul>


</storybook-simple-service-component>
</ng-component>
</storybook-dynamic-app-root>
`;

exports[`Storyshots Common ngModule metadata with knobs 1`] = `
<storybook-dynamic-app-root
cfr={[Function CodegenComponentFactoryResolver]}
data={[Function Object]}
target={[Function ViewContainerRef_]}
>
<ng-component>


<storybook-name
ng-reflect-field="foobar"
>
<h1>
CustomPipe: foobar
</h1>
</storybook-name>


<storybook-simple-service-component
ng-reflect-name="Static name"
>


<p>
Static name:
</p>


<ul>





</ul>


</storybook-simple-service-component>


</ng-component>
</storybook-dynamic-app-root>
`;

exports[`Storyshots Custom Pipe Default 1`] = `
<storybook-dynamic-app-root
cfr={[Function CodegenComponentFactoryResolver]}
Expand Down
39 changes: 38 additions & 1 deletion examples/angular-cli/src/stories/custom-metadata.stories.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { storiesOf } from '@storybook/angular';
import { storiesOf, moduleMetadata } from '@storybook/angular';
Copy link
Member

Choose a reason for hiding this comment

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

I think better having a separate example of this usage, also because it's necessary to check it in conjunction with addon-knobs.
How will it work with template instead of component prop?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added. Seems to work fine with template. I don't see any reason why they should affect each other, as this only touches the metadata, and passes other parts of the story through untouched.

import { withKnobs, text } from '@storybook/addon-knobs/angular';

import { NameComponent } from './moduleMetadata/name.component';
Expand Down Expand Up @@ -64,3 +64,40 @@ storiesOf('Custom ngModule metadata', module)
},
};
});

storiesOf('Common ngModule metadata', module)
Copy link
Member

Choose a reason for hiding this comment

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

Can you please extract it to a separate file? So it will play nicely with #2885 and #2918

Copy link
Member

Choose a reason for hiding this comment

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

I agree, it'd be better to have a separate example that shows:

  1. An example with this decorator (basic usage)
  2. Decorator + Knobs addon
  3. Decorator + Knobs + template
  4. Decorator + knobs + component

We don't have official docs for most of this and we usually point people to the examples so we try to keep as many as we can with all the use cases

Copy link
Contributor Author

@jonrimmer jonrimmer Feb 17, 2018

Choose a reason for hiding this comment

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

I guess I'm not really seeing what value these examples would have? All this decorator does is allow you to declare a set of metadata that is shared amongst your subsequent stories. Whether you're using knobs, template, component, etc. is completely orthogonal to it. Since there is already examples of those things, I'm not sure how duplicating them here would really help understanding it?

Wouldn't it make more sense to modify all the other examples to use this decorator where appropriate, i.e. instead of supplying duplicate metadata, as in custom-pipes and custom-providers? Then the metadata example file could just be:

  1. An example with metadata supplied on the story.
  2. Metadata supplied using this decorator.
  3. Metadata supplied on the story that combines with that from the decorator.

.addDecorator(
moduleMetadata({
imports: [],
schemas: [],
declarations: [ServiceComponent],
providers: [DummyService],
})
)
.add('simple', () => ({
component: ServiceComponent,
props: {
name: 'Static name',
},
}))
.add('template', () => ({
template:
'<storybook-simple-service-component [name]="name"></storybook-simple-service-component>',
props: {
name: 'Static name',
},
}))
.addDecorator(withKnobs)
.add('with knobs', () => ({
template: `
<storybook-name [field]="field"></storybook-name>
<storybook-simple-service-component [name]="name"></storybook-simple-service-component>
`,
props: {
field: text('field', 'foobar'),
name: 'Static name',
},
moduleMetadata: {
declarations: [NameComponent, CustomPipePipe],
},
}));
2 changes: 1 addition & 1 deletion scripts/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ const createOption = ({ defaultValue, option, name, extraParam }) => ({

const tasks = {
core: createProject({
name: `Core & React & Vue & Polymer ${chalk.gray('(core)')}`,
name: `Core & React & Vue & Polymer & Angular ${chalk.gray('(core)')}`,
defaultValue: true,
option: '--core',
projectLocation: path.join(__dirname, '..'),
Expand Down