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

Move more things to core #2788

Merged
merged 12 commits into from
Jan 20, 2018
74 changes: 3 additions & 71 deletions app/angular/src/server/config/defaults/webpack.config.js
Original file line number Diff line number Diff line change
@@ -1,73 +1,5 @@
// import webpack from 'webpack';
import autoprefixer from 'autoprefixer';
import { createDefaultWebpackConfig } from '@storybook/core/server';
import { includePaths } from '../utils';

// Add a default custom config which is similar to what React Create App does.
module.exports = storybookBaseConfig => {
const newConfig = { ...storybookBaseConfig };

newConfig.module.rules = [
...storybookBaseConfig.module.rules,
{
test: /\.css$/,
use: [
require.resolve('style-loader'),
{
loader: require.resolve('css-loader'),
options: {
importLoaders: 1,
},
},
{
loader: require.resolve('postcss-loader'),
options: {
ident: 'postcss', // https://webpack.js.org/guides/migrating/#complex-options
plugins: () => [
require('postcss-flexbugs-fixes'), // eslint-disable-line
autoprefixer({
browsers: [
'>1%',
'last 4 versions',
'Firefox ESR',
'not ie < 9', // React doesn't support IE8 anyway
],
flexbox: 'no-2009',
}),
],
},
},
],
},
{
test: /\.json$/,
include: includePaths,
loader: require.resolve('json-loader'),
},
{
test: /\.(ico|jpg|jpeg|png|gif|eot|otf|webp|svg|ttf|woff|woff2)(\?.*)?$/,
include: includePaths,
loader: require.resolve('file-loader'),
query: {
name: 'static/media/[name].[hash:8].[ext]',
},
},
{
test: /\.(mp4|webm|wav|mp3|m4a|aac|oga)(\?.*)?$/,
include: includePaths,
loader: require.resolve('url-loader'),
query: {
limit: 10000,
name: 'static/media/[name].[hash:8].[ext]',
},
},
];

newConfig.resolve.alias = {
...storybookBaseConfig.resolve.alias,
// This is to support NPM2
'babel-runtime/regenerator': require.resolve('babel-runtime/regenerator'),
};

// Return the altered config
return newConfig;
};
module.exports = storybookBaseConfig =>
createDefaultWebpackConfig(storybookBaseConfig, includePaths);
3 changes: 2 additions & 1 deletion app/angular/src/server/config/webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import path from 'path';
import webpack from 'webpack';
import CaseSensitivePathsPlugin from 'case-sensitive-paths-webpack-plugin';
import HtmlWebpackPlugin from 'html-webpack-plugin';
import { managerPath } from '@storybook/core/client';
import WatchMissingNodeModulesPlugin from './WatchMissingNodeModulesPlugin';

import { includePaths, excludePaths, nodeModulesPaths, loadEnv, nodePaths } from './utils';
Expand All @@ -13,7 +14,7 @@ export default function(configDir) {
const config = {
devtool: 'cheap-module-source-map',
entry: {
manager: [require.resolve('./polyfills'), require.resolve('../../client/manager')],
manager: [require.resolve('./polyfills'), managerPath],
preview: [
require.resolve('./polyfills'),
require.resolve('./globals'),
Expand Down
3 changes: 2 additions & 1 deletion app/angular/src/server/config/webpack.config.prod.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import path from 'path';
import webpack from 'webpack';
import UglifyJsPlugin from 'uglifyjs-webpack-plugin';
import HtmlWebpackPlugin from 'html-webpack-plugin';
import { managerPath } from '@storybook/core/client';

import babelLoaderConfig from './babel.prod';
import { includePaths, excludePaths, loadEnv, nodePaths } from './utils';
Expand All @@ -11,7 +12,7 @@ import { version } from '../../../package.json';
export default function(configDir) {
const entries = {
preview: [require.resolve('./polyfills'), require.resolve('./globals')],
manager: [require.resolve('./polyfills'), path.resolve(__dirname, '../../client/manager')],
manager: [require.resolve('./polyfills'), managerPath],
};

const config = {
Expand Down
16 changes: 0 additions & 16 deletions app/polymer/src/client/index.js
Original file line number Diff line number Diff line change
@@ -1,17 +1 @@
// import deprecate from 'util-deprecate';

// NOTE export these to keep backwards compatibility
// import { action as deprecatedAction } from '@storybook/addon-actions';
// import { linkTo as deprecatedLinkTo } from '@storybook/addon-links';

export { storiesOf, setAddon, addDecorator, configure, getStorybook } from './preview';

// export const action = deprecate(
// deprecatedAction,
// '@storybook/react action is deprecated. See: https://github.com/storybooks/storybook/tree/master/addons/actions'
// );

// export const linkTo = deprecate(
// deprecatedLinkTo,
// '@storybook/react linkTo is deprecated. See: https://github.com/storybooks/storybook/tree/master/addons/links'
// );
7 changes: 0 additions & 7 deletions app/polymer/src/client/manager/index.js

This file was deleted.

39 changes: 0 additions & 39 deletions app/polymer/src/client/manager/preview.js

This file was deleted.

53 changes: 0 additions & 53 deletions app/polymer/src/client/manager/provider.js

This file was deleted.

74 changes: 3 additions & 71 deletions app/polymer/src/server/config/defaults/webpack.config.js
Original file line number Diff line number Diff line change
@@ -1,73 +1,5 @@
// import webpack from 'webpack';
import autoprefixer from 'autoprefixer';
import { createDefaultWebpackConfig } from '@storybook/core/server';
Copy link
Member

@ndelangen ndelangen Jan 20, 2018

Choose a reason for hiding this comment

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

How about moving the webpack configs into a separate package?

So this becomes much easier:
https://storybook.js.org/configurations/custom-webpack-config/#full-control-mode--default

Copy link
Member Author

@igor-dv igor-dv Jan 20, 2018

Choose a reason for hiding this comment

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

What do you mean? Will it be something like @storybook/configs ? TBH, I think we need to make some progress in core extraction because this copypasta across all the apps makes me crazy 😷

Copy link
Member

@Hypnosphi Hypnosphi Jan 20, 2018

Choose a reason for hiding this comment

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

@storybook/webpack-config sounds good to me. We only need to publish the default one, because it's the only one that should be imported from outside

Copy link
Member

@Hypnosphi Hypnosphi Jan 20, 2018

Choose a reason for hiding this comment

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

But maybe it would be an even better solution just to pass genDefaultConfig as third argument to custom webpack config function, so that users could write full control + defaults like this, without any extra imports:

module.exports = (baseConfig, env, genDefaultConfig) => {
  const config = genDefaultConfig(baseConfig, env);

  // Extend it as you need.
};

Or maybe even like this:

module.exports = (baseConfig, env, defaultConfig) => {
  // Extend `defaultConfig` as you need.
};

Copy link
Member Author

Choose a reason for hiding this comment

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

Makes sense to me. It's way much simpler than publishing one more package

Copy link
Member Author

Choose a reason for hiding this comment

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

I mean from the user's point of view =)

Copy link
Member

Choose a reason for hiding this comment

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

I'd rather not create yet another package. @Hypnosphi the proposals above seem pretty reasonable--are they backwards compatible or breaking changes?

Copy link
Member

@Hypnosphi Hypnosphi Jan 20, 2018

Choose a reason for hiding this comment

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

@shilman We should deprecate the old way using utils-deprecate for now, and then remove it in next major
So the answer is yes, it can be backwards-compatible

import { includePaths } from '../utils';

// Add a default custom config which is similar to what React Create App does.
module.exports = storybookBaseConfig => {
const newConfig = { ...storybookBaseConfig };

newConfig.module.rules = [
...storybookBaseConfig.module.rules,
{
test: /\.css$/,
use: [
require.resolve('style-loader'),
{
loader: require.resolve('css-loader'),
options: {
importLoaders: 1,
},
},
{
loader: require.resolve('postcss-loader'),
options: {
ident: 'postcss', // https://webpack.js.org/guides/migrating/#complex-options
plugins: () => [
require('postcss-flexbugs-fixes'), // eslint-disable-line
autoprefixer({
browsers: [
'>1%',
'last 4 versions',
'Firefox ESR',
'not ie < 9', // React doesn't support IE8 anyway
],
flexbox: 'no-2009',
}),
],
},
},
],
},
{
test: /\.json$/,
include: includePaths,
loader: require.resolve('json-loader'),
},
{
test: /\.(ico|jpg|jpeg|png|gif|eot|otf|webp|svg|ttf|woff|woff2)(\?.*)?$/,
include: includePaths,
loader: require.resolve('file-loader'),
query: {
name: 'static/media/[name].[hash:8].[ext]',
},
},
{
test: /\.(mp4|webm|wav|mp3|m4a|aac|oga)(\?.*)?$/,
include: includePaths,
loader: require.resolve('url-loader'),
query: {
limit: 10000,
name: 'static/media/[name].[hash:8].[ext]',
},
},
];

newConfig.resolve.alias = {
...storybookBaseConfig.resolve.alias,
// This is to support NPM2
'babel-runtime/regenerator': require.resolve('babel-runtime/regenerator'),
};

// Return the altered config
return newConfig;
};
module.exports = storybookBaseConfig =>
createDefaultWebpackConfig(storybookBaseConfig, includePaths);
4 changes: 3 additions & 1 deletion app/polymer/src/server/config/webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import webpack from 'webpack';
import CaseSensitivePathsPlugin from 'case-sensitive-paths-webpack-plugin';
import HtmlWebpackPlugin from 'html-webpack-plugin';
import CopyWebpackPlugin from 'copy-webpack-plugin';
import { managerPath } from '@storybook/core/client';

import WatchMissingNodeModulesPlugin from './WatchMissingNodeModulesPlugin';
import {
getConfigDir,
Expand All @@ -20,7 +22,7 @@ export default function() {
const config = {
devtool: 'cheap-module-source-map',
entry: {
manager: [require.resolve('./polyfills'), require.resolve('../../client/manager')],
manager: [require.resolve('./polyfills'), managerPath],
preview: [
require.resolve('./polyfills'),
require.resolve('./globals'),
Expand Down
4 changes: 2 additions & 2 deletions app/polymer/src/server/config/webpack.config.prod.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import path from 'path';
import webpack from 'webpack';
import HtmlWebpackPlugin from 'html-webpack-plugin';
import CopyWebpackPlugin from 'copy-webpack-plugin';
import { managerPath } from '@storybook/core/client';
import babelLoaderConfig from './babel.prod';
import { getConfigDir, includePaths, excludePaths, loadEnv, nodePaths } from './utils';
import { getPreviewHeadHtml, getManagerHeadHtml } from '../utils';
Expand All @@ -10,7 +10,7 @@ import { version } from '../../../package.json';
export default function() {
const entries = {
preview: [require.resolve('./polyfills'), require.resolve('./globals')],
manager: [require.resolve('./polyfills'), path.resolve(__dirname, '../../client/manager')],
manager: [require.resolve('./polyfills'), managerPath],
};

const config = {
Expand Down
7 changes: 0 additions & 7 deletions app/react/src/client/manager/index.js

This file was deleted.

39 changes: 0 additions & 39 deletions app/react/src/client/manager/preview.js

This file was deleted.

Loading