[Experimental] Presets support #4027

igor-dv · 2018-08-16T19:05:10Z

Issue

The purpose of presets is to be able to easily integrate to the different points of SB configuration.
Users will be able to combine different presets in their app and create their own.
All official presets will be located in the presets monorepo.
Every supported app will contain framework-presets, that will be passed down to the core (e.g. framework-preset-vue)
Core will have core-prests (e.g. core-preset-dev)
In the end, presets will be chained and executed.

API

Each preset may expose some of these methods (We can think of more stuff):

module.exports = {
  babel: (config, options), // extend babel config
  webpack: (config, options), // extend webpack config
  preview: (config, options), // extend preview entry-point
  manager: (config, options), // extend manager entry-point
}

Considering presets.js (or presets.ts) in user's code:

module.exports = [
  '@storybook/preset-typescript',
  {
    name: '@storybook/preset-scss',
    options: {
      cssLoaderOptions: {
        modules: true,
        localIdentName: '[name]__[local]--[hash:base64:5]'
      }
    }
  }
];

Note

presets.js will be a part of the future single configuration file. For now, when loading a cusomt presets.js file, there will be a warning.
~~We can think of our supported apps as a combination of presets as well~~ Done

igor-dv · 2018-08-16T22:20:49Z

@pksunkara, having in mind your work for vue-cli plugin, can you see what is missing?

codecov · 2018-08-17T11:19:03Z

Codecov Report

Merging #4027 into master will increase coverage by 48.04%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           master    #4027       +/-   ##
===========================================
+ Coverage   40.59%   88.63%   +48.04%     
===========================================
  Files         483        6      -477     
  Lines        5747       44     -5703     
  Branches      770        2      -768     
===========================================
- Hits         2333       39     -2294     
+ Misses       3042        4     -3038     
+ Partials      372        1      -371

Impacted Files	Coverage Δ
...toryshots-core/src/frameworks/svelte/renderTree.js
addons/backgrounds/mithril.js
app/riot/src/client/preview/render-riot.js
addons/knobs/src/components/types/Select.js
...yshots/storyshots-core/src/frameworks/rn/loader.js
addons/storysource/src/loader/parsers/index.js
lib/ui/src/modules/ui/configs/handle_routing.js
...ents/stories_panel/stories_tree/tree_decorators.js
...-native/src/preview/components/OnDeviceUI/style.js
app/react-native/src/bin/storybook-build.js
... and 436 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 a2a2a91...e6972e7. Read the comment docs.

pksunkara · 2018-08-17T10:50:11Z

lib/core/src/server/presets.js

+}
+
+function getPresets(configDir) {
+  const presets = serverRequire(path.resolve(configDir, 'presets'));


I think we should move to just a single config file called storybook.config.js which is built out of presets.

I totally agree, and as I wrote in the Misc ☝️ , it should be a part of the single config file.
However, I don't know what will be implemented/published first. Also, I don't want to create PRs with hundreds of changes in them =)

I've added a warning when using custom presets. It will be only for the presets testing purpose.

pksunkara · 2018-08-17T10:51:00Z

lib/core/src/server/presets.js

+
+  return {
+    extendBabel: config => applyPresets(loadedPresets, config, 'extendBabel'),
+    extendWebpack: config => applyPresets(loadedPresets, config, 'extendWebpack'),


Have you seen https://github.com/mozilla-neutrino/webpack-chain? It would be useful in our architecture.

I will check this.

This package looks interesting. Anyway, we can integrate it later. If we will go with it, it will be a main part of the core, so I would like to here other team members opinion as well =)

pksunkara · 2018-08-17T11:02:04Z

lib/core/src/server/presets.js

+  const loadedPresets = loadPresets(presets);
+
+  return {
+    extendBabel: config => applyPresets(loadedPresets, config, 'extendBabel'),


Do we want to do this? Can't we somehow merge extendWebpack and extendBabel? The webpack-chain I mentioned below should be useful.

I think it's useful to separate different entry points of integration. extendPreview and extendManager technically also could be removed 🤷‍♂️

lib/core/src/server/presets.js

pksunkara · 2018-08-17T12:11:47Z

lib/core/src/server/config.js

+
+  const baseConfig = getBaseConfig({ ...options, babelOptions, presets });
+  const initialConfig = wrapInitialConfig(baseConfig, configDir);
+  const config = presets.extendWebpack(initialConfig);
  const defaultConfig = wrapDefaultConfig(createDefaultWebpackConfig(config));


We should remove these wrappers and rely on presets.

In the long run - yes (Also wrote about it in Misc). But again, I would like to start using presets, without a huge refactoring in one PR.

What I think we should do is instead of refactoring, we can support the new method of config without changing anything from the first. And once we are happy with the presets, we can remove the old one.

…reset-webpack-custom

…pack

# Conflicts: # lib/core/src/server/config/utils.js # lib/core/src/server/config/webpack.config.js # lib/core/src/server/config/webpack.config.prod.js

…k-core-presets # Conflicts: # app/vue/src/server/options.js # lib/core/src/server/config/webpack.config.js

igor-dv · 2018-09-03T21:48:34Z

lib/core/src/server/mergeBabel.js

@@ -0,0 +1,72 @@
+/* eslint-disable no-param-reassign,no-shadow */


I've opened an issue - neutrinojs/babel-merge#14
In any case, I will move it out of here later.

pksunkara · 2018-09-04T22:20:14Z

Looks good! I just have 2 questions.

I am not sure about this because I don't know much about angular. But the only angular apps we support is anglular-cli based?
I thought custom presets.js was supposed to override everything rather than just getting appended to the list of presets.

ndelangen · 2018-09-06T10:58:35Z

@pksunkara Right now custom presets are kind of a unsupported feature, so I'm find with this for now.

In the future I think there should just be a complete list of add presets in the (to be created) unified config file.

igor-dv · 2018-09-06T19:56:22Z

I am not sure about this because I don't know much about angular. But the only angular apps we support is anglular-cli based?

That is what all angular people use =) But, IMO you can bootstrap angular app without any starter, and use SB as well, but there will be more manual work to do

pksunkara · 2018-09-06T19:57:17Z

This PR is good to be merged. I am just trying to see why tests are failing.

pksunkara · 2018-09-06T19:58:47Z

Nevermind, it's been fixed in master.

igor-dv · 2018-09-06T19:59:30Z

Yey 🎆 Work of my life 😭

Hypnosphi · 2018-09-08T17:40:03Z

lib/core/src/server/babel-core-proxy.js

+let babelCore = null;
+
+try {
+  babelCore = require('@babel/core');


This can accidentally resolve to something only accessible from node_modules/@storybook/core/node_modules not from user's app.

We need to add babel-core peerDependency and ask users to install bridge if we need to use Babel directly

That what I've tried to prevent, but I assume better to have a peer on a bridge. I will do this after the PR to babel-merge

Hypnosphi · 2018-09-08T17:46:31Z

@igor-dv Why do we need to merge Babel configs on our side?

igor-dv · 2018-09-09T23:55:27Z

I wanted to simplify the API for presets, so apps will also use it. And wrapDefaultBabel Config was a pain in the ass for the API. So I thought if we will merge the defaults, it will remove the need of this wrapper.

Hypnosphi · 2018-09-10T00:14:54Z

was a pain in the ass

Can you please provide some examples of this pain? I'm just afraid that maintaining a merger can be a greater one (see all the issues with our webpack merger)

igor-dv · 2018-09-10T00:32:44Z

The pain was to consolidate the API between framework, core and custom presets. Take a look in #4107 - where I tried to kill the wrapper. babel-merge package actually does the merge well. But maybe you have a better idea ?

Hypnosphi · 2018-09-10T00:49:04Z

This doesn't look too painful to me:
https://github.com/storybooks/storybook/blob/05e64383420d24ba490272cb10c222256943ea1a/app/react/src/server/wrapDefaultBabelConfig.js

Add initial presets support

307b7e9

igor-dv added feature request in progress ci: do not merge core labels Aug 16, 2018

igor-dv self-assigned this Aug 16, 2018

igor-dv requested review from alterx and tmeasday as code owners August 16, 2018 19:05

Fix presets.reduce

f9faee9

Merge remote-tracking branch 'origin/master' into core/presets

9c31a7e

pksunkara requested changes Aug 17, 2018

View reviewed changes

igor-dv added 6 commits August 17, 2018 16:43

Add tests for presets.js

f258617

Initial implementation of the core and framework presets

77cd59a

Fix presets.test.js

a64cdf4

Combine all presets to one and add core-preset-babel-cache and core-p…

60176a5

…reset-webpack-custom

Build "entries" with presets and inject them to the presets.extendWeb…

d08fac0

…pack

Logging presets with "=>"

06d5178

igor-dv mentioned this pull request Aug 19, 2018

[WIP] Refactor core and frameworks to work with presets #4043

Merged

igor-dv added 10 commits August 19, 2018 19:55

Set empty array as a default for the corePresets and frameworkPresets

fdb41b9

Merge remote-tracking branch 'origin/master' into core/presets

0550fba

# Conflicts: # lib/core/src/server/config/utils.js # lib/core/src/server/config/webpack.config.js # lib/core/src/server/config/webpack.config.prod.js

Merge remote-tracking branch 'origin/core/presets' into core/framewor…

f72389d

…k-core-presets # Conflicts: # app/vue/src/server/options.js # lib/core/src/server/config/webpack.config.js

Naming

2ef180d

Fix unwanted change

1e47756

Add storybookDefault to babel default config for the future purpose.

846faf4

Revert

6e813ef

Get rid of wrapDefaultBabelConfig

12426fa

Add mergeBabel

885ad81

Merge remote-tracking branch 'origin/master' into core/presets

7fb590e

Add warning about custom presets

a3c2fda

ndelangen changed the title ~~[WIP] Presets support~~ Presets support Sep 3, 2018

igor-dv mentioned this pull request Sep 3, 2018

Support of babel 6 and babel 7 neutrinojs/babel-merge#14

Open

igor-dv added 3 commits September 3, 2018 19:27

Simplify presets API

6c89676

Fix presets.test.js

4df7b87

Ooops

e6972e7

igor-dv commented Sep 3, 2018

View reviewed changes

igor-dv added needs review and removed ci: do not merge in progress labels Sep 3, 2018

ndelangen added this to the v4.0.0 milestone Sep 4, 2018

pksunkara approved these changes Sep 4, 2018

View reviewed changes

pksunkara merged commit b2b7359 into master Sep 6, 2018

pksunkara deleted the core/presets branch September 6, 2018 19:58

Hypnosphi reviewed Sep 8, 2018

View reviewed changes

Hypnosphi changed the title ~~Presets support~~ [Experimental] Presets support Sep 8, 2018

igor-dv added presets and removed needs review labels Sep 14, 2018

Yogu mentioned this pull request Oct 5, 2018

Full-control webpack config with default results in duplicate style loader rules #4296

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Experimental] Presets support #4027

[Experimental] Presets support #4027

igor-dv commented Aug 16, 2018 •

edited

Loading

igor-dv commented Aug 16, 2018

codecov bot commented Aug 17, 2018 •

edited

Loading

pksunkara Aug 17, 2018

igor-dv Aug 17, 2018

igor-dv Sep 3, 2018

pksunkara Aug 17, 2018

igor-dv Aug 17, 2018

igor-dv Aug 19, 2018

pksunkara Aug 17, 2018

igor-dv Aug 17, 2018

pksunkara Aug 17, 2018

igor-dv Aug 17, 2018

pksunkara Aug 17, 2018

igor-dv Sep 3, 2018

pksunkara commented Sep 4, 2018

ndelangen commented Sep 6, 2018

igor-dv commented Sep 6, 2018

pksunkara commented Sep 6, 2018

pksunkara commented Sep 6, 2018

igor-dv commented Sep 6, 2018 •

edited

Loading

Hypnosphi Sep 8, 2018

igor-dv Sep 9, 2018

Hypnosphi commented Sep 8, 2018

igor-dv commented Sep 9, 2018 •

edited

Loading

Hypnosphi commented Sep 10, 2018 •

edited

Loading

igor-dv commented Sep 10, 2018

Hypnosphi commented Sep 10, 2018

		@@ -0,0 +1,72 @@
		/* eslint-disable no-param-reassign,no-shadow */

[Experimental] Presets support #4027

[Experimental] Presets support #4027

Conversation

igor-dv commented Aug 16, 2018 • edited Loading

Issue

API

Note

igor-dv commented Aug 16, 2018

codecov bot commented Aug 17, 2018 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pksunkara commented Sep 4, 2018

ndelangen commented Sep 6, 2018

igor-dv commented Sep 6, 2018

pksunkara commented Sep 6, 2018

pksunkara commented Sep 6, 2018

igor-dv commented Sep 6, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Hypnosphi commented Sep 8, 2018

igor-dv commented Sep 9, 2018 • edited Loading

Hypnosphi commented Sep 10, 2018 • edited Loading

igor-dv commented Sep 10, 2018

Hypnosphi commented Sep 10, 2018

igor-dv commented Aug 16, 2018 •

edited

Loading

codecov bot commented Aug 17, 2018 •

edited

Loading

igor-dv commented Sep 6, 2018 •

edited

Loading

igor-dv commented Sep 9, 2018 •

edited

Loading

Hypnosphi commented Sep 10, 2018 •

edited

Loading