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

Allow optional component declaration #6346

Conversation

MaximSagan
Copy link
Contributor

Issue: #5542

Typically, when developing features in Angular, it is advantageous to use "feature modules" which provide a "context" for declared components including required imports, declarations, and providers.

To simulate this context in Storybook, we may want to use the component that has already been declared in its FeatureModule, rather than having to recreate this context manually (with imports, providers, and additional declarations) just to get the component to perform its basic functionality.

However, currently, the only way to display a component declared in its feature module is to refer to it indirectly in the template property (e.g. template: '<my-component [prop1]="prop1"></my-component>') which is used by Storybook to dynamically create a component called DynamicComponent. While this works, we lose the following benefits:

  • Direct mapping between props and the @inputs and @outputs of the component we want to display.
  • Any potential benefits of type-checking for property names.

If we want to get the above benefits, we need to specify the component by its class (e.g. component: MyComponent). Unfortunately, the default behavior of Storybook for Angular is to declare specified components in a dynamic module, which prevents us from using the version of the component declared in our feature module (with its appropriate context), due to the fact that a component can only be declared in one module (else Angular will error out).

What I did

I added an optional flag called requiresComponentDeclartion to the NgStory object that the user specifies in the ".add()" method. When this flag is set to false, Storybook will not declare the specified component in the DynamicModule. This flag is set to true by default, which leaves the existing behavior (hence this is not a breaking change).

How to test

  • Is this testable with Jest or Chromatic screenshots?
    Maybe? Please advise.
  • Does this need a new example in the kitchen sink apps?
    Yes, added.
  • Does this need an update to the documentation?
    The Storybook for Angular documentation does not seem to go very deep into specific configuration options.

@MaximSagan
Copy link
Contributor Author

Sorry, as with my other PR, snapshots were missing. Should be good now.

@vercel
Copy link

vercel bot commented Apr 2, 2019

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-dont-enforce-a45397.storybook.now.sh

@ndelangen
Copy link
Member

ndelangen commented Apr 26, 2019

@kroeder @gaetanmaisse @amcdnl Would you be able to review this possibly? Sounds useful.

@storybookjs storybookjs deleted a comment from stale bot Apr 26, 2019
@ndelangen ndelangen self-assigned this Apr 26, 2019
# Conflicts:
#	app/angular/src/client/preview/angular/helpers.ts
@codecov
Copy link

codecov bot commented Apr 26, 2019

Codecov Report

Merging #6346 into next will decrease coverage by 2.23%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##             next    #6346      +/-   ##
==========================================
- Coverage   40.44%   38.21%   -2.24%     
==========================================
  Files         637      648      +11     
  Lines        8759     9862    +1103     
  Branches      639      393     -246     
==========================================
+ Hits         3543     3769     +226     
- Misses       5118     6033     +915     
+ Partials       98       60      -38
Impacted Files Coverage Δ
app/angular/src/client/preview/angular/helpers.ts 32.75% <0%> (+4.18%) ⬆️
.../storyshots-core/src/frameworks/angular/helpers.ts 0% <0%> (ø) ⬆️
addons/actions/src/preview/action.ts 90.9% <0%> (-9.1%) ⬇️
app/angular/src/server/angular-cli_utils.js 43.75% <0%> (-3.13%) ⬇️
lib/client-api/src/story_store.js 74.57% <0%> (-1.74%) ⬇️
addons/knobs/src/components/Panel.js 80% <0%> (-1.71%) ⬇️
lib/core/src/client/preview/start.js 72.94% <0%> (-0.93%) ⬇️
addons/ondevice-knobs/src/types/Date.js 0% <0%> (ø) ⬆️
app/polymer/src/client/index.js 0% <0%> (ø) ⬆️
app/html/standalone.js 0% <0%> (ø) ⬆️
... and 512 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fadd746...27e30e5. Read the comment docs.

@vercel vercel bot temporarily deployed to staging April 26, 2019 16:52 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants