From deb453ecb7a47c1d83bf8898266e90c1ca99f1bf Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Mon, 21 Sep 2020 16:53:12 +0200 Subject: [PATCH 1/3] fix(fast-refresh): set babel plugin correctly in the preset - Rather than in the webpack config, set it in babel, so that there aren't multiple loaders and instead just the plugin is added together with the presets. Examples were moved to cra-kitchen-sink because official removes the babel-loader taht comes from storybook. --- .../src/server/framework-preset-react.ts | 57 ++++++++++--------- examples/cra-kitchen-sink/.storybook/main.js | 3 + .../src/components/FastRefreshExample.js} | 4 +- .../src/stories/fast-refresh.stories.js | 9 +++ .../addon-docs/react-refresh.stories.js | 8 --- lib/core/types/index.ts | 1 + 6 files changed, 44 insertions(+), 38 deletions(-) rename examples/{official-storybook/stories/addon-docs/react-refresh-example.js => cra-kitchen-sink/src/components/FastRefreshExample.js} (67%) create mode 100644 examples/cra-kitchen-sink/src/stories/fast-refresh.stories.js delete mode 100644 examples/official-storybook/stories/addon-docs/react-refresh.stories.js diff --git a/app/react/src/server/framework-preset-react.ts b/app/react/src/server/framework-preset-react.ts index f75037e8571b..c2180d367dd9 100644 --- a/app/react/src/server/framework-preset-react.ts +++ b/app/react/src/server/framework-preset-react.ts @@ -1,10 +1,27 @@ import { TransformOptions } from '@babel/core'; import ReactRefreshWebpackPlugin from '@pmmmwh/react-refresh-webpack-plugin'; import type { Configuration } from 'webpack'; + import { logger } from '@storybook/node-logger'; -import type { StorybookOptions } from './types'; +import type { StorybookOptions } from '@storybook/core/types'; + +export async function babel(config: TransformOptions, options: StorybookOptions) { + const isDevelopment = options.configType === 'DEVELOPMENT'; + const reactOptions = await options.presets.apply('reactOptions', {}, options); + const fastRefreshEnabled = + isDevelopment && (reactOptions.fastRefresh || process.env.FAST_REFRESH === 'true'); + + if (!fastRefreshEnabled) { + return config; + } + + return { + ...config, + plugins: [require.resolve('react-refresh/babel'), ...(config.plugins || [])], + }; +} -export function babelDefault(config: TransformOptions) { +export async function babelDefault(config: TransformOptions) { return { ...config, presets: [ @@ -16,35 +33,19 @@ export function babelDefault(config: TransformOptions) { }; } -export function webpackFinal(config: Configuration, { reactOptions }: StorybookOptions) { - const isDevelopment = config.mode === 'development'; +export async function webpackFinal(config: Configuration, options: StorybookOptions) { + const isDevelopment = options.configType === 'DEVELOPMENT'; + const reactOptions = await options.presets.apply('reactOptions', {}, options); const fastRefreshEnabled = - isDevelopment && (reactOptions?.fastRefresh || process.env.FAST_REFRESH === 'true'); - if (fastRefreshEnabled) { - logger.info('=> Using React fast refresh feature.'); + isDevelopment && (reactOptions.fastRefresh || process.env.FAST_REFRESH === 'true'); + + if (!fastRefreshEnabled) { + return config; } + + logger.info('=> Using React fast refresh feature.'); return { ...config, - module: { - ...config.module, - rules: [ - ...config.module.rules, - fastRefreshEnabled && { - test: /\.[jt]sx?$/, - exclude: /node_modules/, - use: [ - { - loader: require.resolve('babel-loader'), - options: { - plugins: [require.resolve('react-refresh/babel')], - }, - }, - ], - }, - ].filter(Boolean), - }, - plugins: [...config.plugins, fastRefreshEnabled && new ReactRefreshWebpackPlugin()].filter( - Boolean - ), + plugins: [...(config.plugins || []), new ReactRefreshWebpackPlugin()], }; } diff --git a/examples/cra-kitchen-sink/.storybook/main.js b/examples/cra-kitchen-sink/.storybook/main.js index 0f293afbb70d..16290a03d47c 100644 --- a/examples/cra-kitchen-sink/.storybook/main.js +++ b/examples/cra-kitchen-sink/.storybook/main.js @@ -3,6 +3,9 @@ const path = require('path'); module.exports = { stories: ['../src/stories/**/*.stories.@(js|mdx)'], logLevel: 'debug', + reactOptions: { + fastRefresh: true, + }, addons: [ '@storybook/preset-create-react-app', { diff --git a/examples/official-storybook/stories/addon-docs/react-refresh-example.js b/examples/cra-kitchen-sink/src/components/FastRefreshExample.js similarity index 67% rename from examples/official-storybook/stories/addon-docs/react-refresh-example.js rename to examples/cra-kitchen-sink/src/components/FastRefreshExample.js index 07924321e48a..d9e382ffa97c 100644 --- a/examples/official-storybook/stories/addon-docs/react-refresh-example.js +++ b/examples/cra-kitchen-sink/src/components/FastRefreshExample.js @@ -1,11 +1,11 @@ import React from 'react'; -export const Refresh = () => { +export const FastRefreshExample = () => { const [value, setValue] = React.useState('abc'); return ( <> setValue(event.target.value)} /> -

Change the input value then this text in the component.

+

Change the input value then this text in the component file.

The state of the input should be kept.

); diff --git a/examples/cra-kitchen-sink/src/stories/fast-refresh.stories.js b/examples/cra-kitchen-sink/src/stories/fast-refresh.stories.js new file mode 100644 index 000000000000..f96f85877dcf --- /dev/null +++ b/examples/cra-kitchen-sink/src/stories/fast-refresh.stories.js @@ -0,0 +1,9 @@ +import React from 'react'; +import { FastRefreshExample } from '../components/FastRefreshExample'; + +export default { + title: 'React Fast Refresh', + component: FastRefreshExample, +}; + +export const Default = () => ; diff --git a/examples/official-storybook/stories/addon-docs/react-refresh.stories.js b/examples/official-storybook/stories/addon-docs/react-refresh.stories.js deleted file mode 100644 index 57efe1072b08..000000000000 --- a/examples/official-storybook/stories/addon-docs/react-refresh.stories.js +++ /dev/null @@ -1,8 +0,0 @@ -import React from 'react'; -import { Refresh } from './react-refresh-example'; - -export default { - title: 'Core/React Refresh', -}; - -export const Default = () => ; diff --git a/lib/core/types/index.ts b/lib/core/types/index.ts index d0c0c7f02337..afc126a01fcc 100644 --- a/lib/core/types/index.ts +++ b/lib/core/types/index.ts @@ -40,6 +40,7 @@ export interface StorybookOptions { configType: 'DEVELOPMENT' | 'PRODUCTION'; presetsList: Preset[]; typescriptOptions: TypescriptOptions; + [key: string]: any; } /** From d7600147ed084b7c50d11da5b76495925beccc3e Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Mon, 21 Sep 2020 17:18:28 +0200 Subject: [PATCH 2/3] test(e2e): add env file with fast refresh for CRA --- scripts/run-e2e-config.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/scripts/run-e2e-config.ts b/scripts/run-e2e-config.ts index ab8b6e792de9..43665790fc6f 100644 --- a/scripts/run-e2e-config.ts +++ b/scripts/run-e2e-config.ts @@ -137,7 +137,11 @@ export const react_typescript: Parameters = { export const cra: Parameters = { name: 'cra', version: 'latest', - generator: 'npx create-react-app@{{version}} {{name}}-{{version}}', + generator: [ + 'npx create-react-app@{{version}} {{name}}-{{version}}', + 'cd {{name}}-{{version}}', + 'echo "FAST_REFRESH=true" > .env', + ].join(' && '), }; export const cra_typescript: Parameters = { From f55b4b61d4fe04967cea8ad94f585eeaa698e185 Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Mon, 21 Sep 2020 18:09:37 +0200 Subject: [PATCH 3/3] test(preset-react): update tests for new implementation --- .../src/server/framework-preset-react.test.ts | 100 +++++++++++++----- 1 file changed, 71 insertions(+), 29 deletions(-) diff --git a/app/react/src/server/framework-preset-react.test.ts b/app/react/src/server/framework-preset-react.test.ts index 5592bf4aa46c..653fd063b3f5 100644 --- a/app/react/src/server/framework-preset-react.test.ts +++ b/app/react/src/server/framework-preset-react.test.ts @@ -1,54 +1,96 @@ import webpack from 'webpack'; +import ReactRefreshWebpackPlugin from '@pmmmwh/react-refresh-webpack-plugin'; import * as preset from './framework-preset-react'; import type { StorybookOptions } from './types'; +const mockApply = jest.fn(); +jest.mock('@pmmmwh/react-refresh-webpack-plugin', () => { + return jest.fn().mockImplementation(() => { + return { apply: mockApply }; + }); +}); + describe('framework-preset-react', () => { - const babelLoaderPath = require.resolve('babel-loader'); const reactRefreshPath = require.resolve('react-refresh/babel'); const webpackConfigMock: webpack.Configuration = { - mode: 'development', plugins: [], module: { rules: [], }, }; + const babelConfigMock = {}; - describe('webpackFinal', () => { - it('should return a config with fast refresh plugin when fast refresh is enabled', () => { - const config = preset.webpackFinal(webpackConfigMock, { - reactOptions: { fastRefresh: true }, - } as StorybookOptions); + const storybookOptions: Partial = { + configType: 'DEVELOPMENT', + presets: { + apply: async () => ({ + fastRefresh: true, + }), + }, + presetsList: [], + }; + + const storybookOptionsDisabledRefresh: Partial = { + configType: 'DEVELOPMENT', + presets: { + apply: async () => ({ + fastRefresh: false, + }), + }, + }; + + describe('babel', () => { + it('should return a config with fast refresh plugin when fast refresh is enabled', async () => { + const config = await preset.babel(babelConfigMock, storybookOptions as StorybookOptions); - expect(config.module.rules).toEqual([ - { - test: /\.[jt]sx?$/, - exclude: /node_modules/, - use: [ - { - loader: babelLoaderPath, - options: { - plugins: [reactRefreshPath], - }, - }, - ], - }, - ]); + expect(config.plugins).toEqual([reactRefreshPath]); }); - it('should not return a config with fast refresh plugin when fast refresh is disabled', () => { - const config = preset.webpackFinal(webpackConfigMock, { - reactOptions: { fastRefresh: false }, + it('should return unchanged config without fast refresh plugin when fast refresh is disabled', async () => { + const config = await preset.babel( + babelConfigMock, + storybookOptionsDisabledRefresh as StorybookOptions + ); + + expect(config).toEqual(babelConfigMock); + }); + + it('should return unchanged config without fast refresh plugin when mode is not development', async () => { + const config = await preset.babel(babelConfigMock, { + ...storybookOptions, + configType: 'PRODUCTION', } as StorybookOptions); - expect(config.module.rules).toEqual([]); + expect(config).toEqual(babelConfigMock); + }); + }); + + describe('webpackFinal', () => { + it('should return a config with fast refresh plugin when fast refresh is enabled', async () => { + const config = await preset.webpackFinal( + webpackConfigMock, + storybookOptions as StorybookOptions + ); + + expect(config.plugins).toEqual([new ReactRefreshWebpackPlugin()]); + }); + + it('should return unchanged config without fast refresh plugin when fast refresh is disabled', async () => { + const config = await preset.webpackFinal( + webpackConfigMock, + storybookOptionsDisabledRefresh as StorybookOptions + ); + + expect(config).toEqual(webpackConfigMock); }); - it('should not return a config with fast refresh plugin when mode is not development', () => { - const config = preset.webpackFinal({ ...webpackConfigMock, mode: 'production' }, { - reactOptions: { fastRefresh: true }, + it('should return unchanged config without fast refresh plugin when mode is not development', async () => { + const config = await preset.webpackFinal(webpackConfigMock, { + ...storybookOptions, + configType: 'PRODUCTION', } as StorybookOptions); - expect(config.module.rules).toEqual([]); + expect(config).toEqual(webpackConfigMock); }); }); });