Support css-modules #4405
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,83 @@ | ||
| import semver from 'semver'; | ||
|
|
||
| let MiniCssExtractPlugin; | ||
| let normalizeCondition; | ||
|
|
||
| export function isReactScriptsInstalled() { | ||
| try { | ||
| // eslint-disable-next-line global-require, import/no-extraneous-dependencies | ||
| const reactScriptsJson = require('react-scripts/package.json'); | ||
| if (semver.lt(reactScriptsJson.version, '2.0.0')) return false; | ||
|
|
||
| // eslint-disable-next-line global-require, import/no-extraneous-dependencies, prefer-destructuring | ||
| normalizeCondition = require('webpack/lib/RuleSet').normalizeCondition; | ||
|
There was a problem hiding this comment. Why do you need to save There was a problem hiding this comment. We need Though now that I think about it, this is more necessary for the What are your thoughts? There was a problem hiding this comment. I don't see problems with adding it as a dep |
||
| // eslint-disable-next-line global-require, import/no-extraneous-dependencies | ||
| MiniCssExtractPlugin = require('mini-css-extract-plugin'); | ||
|
There was a problem hiding this comment. Same here. Do we need it as a dep? There was a problem hiding this comment. We don't need this as a dependency since we're first checking if There was a problem hiding this comment. So you can require it already in the |
||
| return true; | ||
| } catch (e) { | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| export function getStyleRules(rules) { | ||
|
There was a problem hiding this comment. I'd love to get some comments throughout this function. Not certain what's occurring. It looks like a way to concatenate across the subset of style loading possibilities given the array of file suffixes we're sifting? |
||
| // Extensions of style rules we're interested in | ||
| const extensions = ['.css', '.scss', '.sass', '.module.css', '.module.scss', '.module.sass']; | ||
|
|
||
| return rules.reduce((styleRules, rule) => { | ||
| if (rule.test && extensions.some(normalizeCondition(rule.test))) { | ||
| // If the base test is for styles, return early | ||
| return styleRules.concat(rule); | ||
| } | ||
|
|
||
| if (!rule.test && rule.oneOf) { | ||
| styleRules.push(...getStyleRules(rule.oneOf)); | ||
| } | ||
|
|
||
| if (!rule.test && rule.rules) { | ||
| styleRules.push(...getStyleRules(rule.rules)); | ||
| } | ||
|
|
||
| return styleRules; | ||
| }, []); | ||
| } | ||
|
|
||
| export function applyCRAWebpackConfig(baseConfig) { | ||
| // Remove style rules from baseConfig | ||
| const baseRulesExcludingStyles = baseConfig.module.rules.filter( | ||
|
|
||
| rule => !rule.test || !['.css', '.scss', '.sass'].some(normalizeCondition(rule.test)) | ||
| ); | ||
|
|
||
| // Load create-react-app config | ||
| let craWebpackConfig; | ||
| if (baseConfig.mode === 'production') { | ||
|
There was a problem hiding this comment. please extract to |
||
| // eslint-disable-next-line global-require, import/no-extraneous-dependencies | ||
| craWebpackConfig = require('react-scripts/config/webpack.config.prod'); | ||
| } else { | ||
| // eslint-disable-next-line global-require, import/no-extraneous-dependencies | ||
| craWebpackConfig = require('react-scripts/config/webpack.config.dev'); | ||
| } | ||
|
|
||
| // Concat will ensure rules is an array | ||
| const craStyleRules = getStyleRules([].concat(craWebpackConfig.module.rules)); | ||
|
There was a problem hiding this comment. could we not simply do: Interested to hear more on "ensur[ing] rules is an array" There was a problem hiding this comment. Whoops. I was thinking |
||
|
|
||
| // Add css minification for production | ||
| const plugins = [...baseConfig.plugins]; | ||
| if (baseConfig.mode === 'production') | ||
| plugins.push( | ||
| new MiniCssExtractPlugin({ | ||
|
There was a problem hiding this comment. Do we really need this, though? There was a problem hiding this comment. Nope! I just thought minification would be good for production. Also, if we don't want to include this plugin, then we have to filter out the related rule from all of the style rules. Adding the plugin was less work than removing it. There was a problem hiding this comment. Does this exclude interop for CRAv1? I don't really care about that, but it could be something to consider. There was a problem hiding this comment. FWIW we dont really need autoprefixer in the default ruleset either 🤷♂️ I'm a fan of good defaults 👍 There was a problem hiding this comment. @kylemh This preset checks for |
||
| // Options similar to the same options in webpackOptions.output | ||
| // both options are optional | ||
| filename: 'static/css/[name].[contenthash:8].css', | ||
| chunkFilename: 'static/css/[name].[contenthash:8].chunk.css', | ||
| }) | ||
| ); | ||
|
|
||
| return { | ||
| ...baseConfig, | ||
| module: { | ||
| ...baseConfig.module, | ||
| rules: [...baseRulesExcludingStyles, ...craStyleRules], | ||
| }, | ||
| plugins, | ||
| }; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| import { logger } from '@storybook/node-logger'; | ||
| import { applyCRAWebpackConfig, isReactScriptsInstalled } from './cra_config'; | ||
|
|
||
| export function webpackFinal(config) { | ||
igor-dv marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| if (!isReactScriptsInstalled()) { | ||
| logger.info('=> Using base config because react-scripts is not installed.'); | ||
| return config; | ||
| } | ||
|
|
||
| logger.info('=> Loading create-react-app config.'); | ||
|
|
||
| return applyCRAWebpackConfig(config); | ||
| } | ||
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 should be
"4.0.0-rc.3"