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

Support different extensions for "config" and "addons" files #3913

Merged
merged 10 commits into from
Jul 25, 2018

Conversation

igor-dv
Copy link
Member

@igor-dv igor-dv commented Jul 23, 2018

Issue: #3737

What I did

When one uses TS (not only, though), she will probably prefer using .ts in all of the files in a codebase.
Currently, we are forcing people to use config.js and addons.js to configure their stuff.
I've used the same approach as in #3785 to fix this.

Notice, I've needed to wrap .ts transformer in jest to the following pattern:

ts-transformer -> babel-transformer

in order to support require.context in TS with Jest 🤕

@codecov
Copy link

codecov bot commented Jul 23, 2018

Codecov Report

Merging #3913 into master will decrease coverage by 1.93%.
The diff coverage is 47.36%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3913      +/-   ##
==========================================
- Coverage   41.55%   39.61%   -1.94%     
==========================================
  Files         455      431      -24     
  Lines        5179     5452     +273     
  Branches      899      741     -158     
==========================================
+ Hits         2152     2160       +8     
- Misses       2487     2910     +423     
+ Partials      540      382     -158
Impacted Files Coverage Δ
lib/core/src/server/loadCustomBabelConfig.js 0% <ø> (ø)
lib/core/src/server/loadCustomWebpackConfig.js 0% <ø> (ø) ⬆️
lib/core/src/server/config/utils.js 0% <0%> (ø) ⬆️
lib/core/src/server/config.js 0% <0%> (ø) ⬆️
lib/core/src/server/config/interpret-files.js 100% <100%> (ø)
...yshots/storyshots-core/src/frameworks/configure.js 100% <100%> (ø) ⬆️
app/mithril/src/client/preview/render.js 0% <0%> (ø) ⬆️
...t-native/src/preview/components/StoryView/index.js 0% <0%> (ø) ⬆️
addons/a11y/src/components/Report/index.js 0% <0%> (ø) ⬆️
app/html/src/server/index.js 0% <0%> (ø) ⬆️
... and 185 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 a57b65b...81c1e32. Read the comment docs.

@igor-dv igor-dv requested a review from ndelangen as a code owner July 23, 2018 22:18
@igor-dv
Copy link
Member Author

igor-dv commented Jul 24, 2018

I have a strange issue with this test:

image

I've committed the snapshot but now it's failing in CircleCI only 🤷‍♂️

I am not sure why it's failed in the beginning though...

Anyone with Mac/Linux can try my branch + run the tests?

@ndelangen
Copy link
Member

I'll check it out

@@ -2,8 +2,8 @@ import findCacheDir from 'find-cache-dir';
import { logger } from '@storybook/node-logger';
import { createDefaultWebpackConfig } from './config/defaults/webpack.config';
import devBabelConfig from './config/babel';
import loadBabelConfig from './babel_config';
import loadCustomConfig from './loadCustomWebpackConfig';
import loadBabelConfig from './loadCustomBabelConfig';
Copy link
Member

@pksunkara pksunkara Jul 24, 2018

Choose a reason for hiding this comment

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

Why not name this import loadCustomBabelConfig from './loadCustomBabelConfig';

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, will rename

@igor-dv
Copy link
Member Author

igor-dv commented Jul 24, 2018

its's green now... Dafuq merge from master did?

@ndelangen
Copy link
Member

@igor-dv apply additional magic

@ndelangen
Copy link
Member

LGTM

@igor-dv igor-dv merged commit 989aa6c into master Jul 25, 2018
@igor-dv igor-dv deleted the support-different-config-files-exts branch July 25, 2018 09:27
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.

3 participants