-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
feature: noStoriesOf loader option support for custome story #4012
Conversation
var withStorySource = require('@storybook/addon-storysource').withStorySource; | ||
var __STORY__ = ${sourceJson}; | ||
var __ADDS_MAP__ = ${addsMap}; | ||
|
||
${result.source} | ||
`; | ||
} else { | ||
return ` | ||
export var __STORY__ = ${sourceJson}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can export them anyway, and reduce this conditions complexity
const result = injectDecorator(source, ADD_DECORATOR_STATEMENT, this.resourcePath, options); | ||
|
||
if (!result.changed) { | ||
if (!options.noStoriesOf && !result.changed) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that the whole point of the feature is that you don't want to inject the decorator, but you do wont to extract the source, so let's just call this parameter injectDecorator
and it will be true by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, it's greet!
if publish, hopes notify me here, thanks |
You mean you don't want to continue this PR? |
ehhh..., i have thought you will change it, sorry, understand mistake. |
Codecov Report
@@ Coverage Diff @@
## master #4012 +/- ##
==========================================
+ Coverage 40.52% 40.57% +0.05%
==========================================
Files 469 469
Lines 5636 5641 +5
Branches 745 748 +3
==========================================
+ Hits 2284 2289 +5
Misses 2984 2984
Partials 368 368
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now I like it 🥇
} | ||
); | ||
|
||
it('don\'t injects stories decorator after the all "storiesOf" functions', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it('does not inject
} | ||
); | ||
|
||
it('don\'t injects stories decorator after the all "storiesOf" functions', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same =)
} | ||
); | ||
|
||
it('don\'t injects stories decorator after the all "storiesOf" functions', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and here
options.parser | ||
); | ||
const { injectDecorator = true } = options; | ||
const { changed, source: newSource, comments } = injectDecorator |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's check it with injectDecorator === true
Please merge from master, it should solve CI issues. |
ok |
Issue:#3993