Core: Deprecate default postcss config, recommend addon-postcss

[phated]

Issue: #12668

What I did

I deprecated the implicit addition of the postcss-loader to the preview webpack config inside @storybook/core and skip adding it completely if users depend on the new @storybook/addon-postcss package.

This new addon is much more flexible in configuring each loader and is versioned independently of Storybook, so it can track PostCSS updates in the future.

I also deprecated the default plugins that Storybook adds for users and added a migration guide to set up a postcss config file if consumers want those packages.

Additionally, I updated the html-kitchen-sink example to show how an end-user can swap out the PostCSS version to v8, even before the plugin is upgraded (handy for v9, v10, etc!).

How to test

• Is this testable with Jest or Chromatic screenshots? ❌
• Does this need a new example in the kitchen sink apps? I updated the html-kitchen-sink example, which shows it can be added and configured correctly.
• Does this need an update to the documentation? Added the migration docs! Happy to add any more that are deemed necessary.

If your answer is yes to any of these, please make sure to include it in your PR.

[phated]

@shilman This isn't quite ready to be promoted from a draft, but it'd be great if you can review to decide if this is a good direction to go. Happy to chat about it some time this week.

[shilman]

Looking great 💯

My only concern (and it's a small one) is the peer dependency -- is there any way of providing a "sensible default"?

[phated]

@shilman What is your goal in avoiding the peerDep? If we don't have users install the version of postcss they want to use, we are going to run into the same issue that currently exists with @storybook/core—specifically, this addon isn't independently versioned, so you can't increase the PostCSS version when they bump a major and break plugins like autoprefixer.

[merceyz]

Could the PnP tests be updated to cover this please? Should make sure the dependencies are handled correctly

[phated]

Could the PnP tests be updated to cover this please? Should make sure the dependencies are handled correctly

I have no idea what this is and I have no experience with PnP.

FWIW, we decided that addon-postcss is going to embed it's dependencies directly and not rely on any peerDep stuff. Not sure if that matters at all.

[phated]

Updated the PR comment body! I think this is in a solid place now and needs review.

@shilman @merceyz If someone can guide me in the PnP tests, I'm happy to make those changes, too. I just don't know anything about them.

[shilman]

Aside from a couple typos, looking great!

[shilman]

@merceyz @gaetanmaisse I'm merging this and releasing to move the project along. We can update with the PnP stuff once we have a little more clarity there.

[merceyz]

I have no idea what this is and I have no experience with PnP.

https://yarnpkg.com/features/pnp - TL;DR; You can't access anything you haven't declared

@shilman @merceyz If someone can guide me in the PnP tests, I'm happy to make those changes, too. I just don't know anything about them.

Would basically just update the list of e2e tests to run through PnP here

storybook/.circleci/config.yml

Line 366 in 1b7fed0

command: yarn test:e2e-framework --use-yarn-2-pnp sfcVue cra

e2e tests are defined here https://github.com/storybookjs/storybook/blob/431d209ec7d3c1002cffcf3eef5be250584d905e/scripts/run-e2e-config.ts so pick one that uses the new postcss stuff and add it to the list above

[phated]

Ah cool! So that means the html-kitchen-sink example will run in both PnP and non-PnP mode? I'll give it a shot. Is there a way for me to run/debug it locally?

[merceyz]

So that means the html-kitchen-sink example will run in both PnP and non-PnP mode?

As it is setup now, no, the CI config I linked to only tests sfcVue and cra

I'll give it a shot. Is there a way for me to run/debug it locally?

yarn test:e2e-framework --use-yarn-2-pnp <e2e test here>

[phated]

Hmm, is it not necessary to have cypress and a local registry set up?

[phated]

Alright, I've researched this more and it seems that the yarn PnP e2e tests use the scaffolds that the CLI provides and none of those are going to include addon-postcss, since it's an external addon that people opt into. I don't think it makes sense to e2e test this.

[gaetanmaisse]

It would make sense to have a test with multiples addons but we are not there yet, we talked about using the SB CLI to add them during the E2E process. Feels free to ping me on discord!

[vdh]

Is the goal of this is to allow upgrading to postcss^8? Because if so, there's still going to be issues with the css-loader^3.0.0 / css-loader^3.6.0 dependencies pinning to postcss^7.0.32.

[phated]

@vdh if you take a look at the example, it proves that this works. Those loaders don't pull in autoprefixer or any plugins (config loading isn't part of postcss itself, only the loader). Additionally, PostCSS can support plugins using older versions if they are written in a specific fashion (see postcss-rebecca-purple in the example).

[kaelig]

I'm confused about the message introduced in this PR:

info => Using implicit CSS loaders
(node:94445) DeprecationWarning: Relying on the implicit PostCSS loader is deprecated and will be removed in Storybook 7.0.
If you need PostCSS, include '@storybook/addon-postcss' in your '.storybook/main.js' file.

See https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#deprecated-implicit-postcss-loader for details.

If I'm not relying on PostCSS, how do I disable the message?

[phated]

@kaelig you don't disable the warning. It will go away when storybook 7 is released.

[shilman]

@phated shouldn't we detect if the user has @storybook/addon-postcss in their main.js and skip the warning if they do?

[phated]

@shilman that's only for people that actually want to use postcss, users that don't want to use it are just shown the warning to know the implicit addition of postcss will be removed in 7.0

[kaelig]

users that don't want to use it are just shown the warning to know the implicit addition of postcss will be removed in 7.0

This is definitely useful to know for people configuring Storybook. In contrast, I'm on a team where I configure Storybook for hundreds of devs (who are on the side of "using" storybook), and I'd like to prevent them from seeing this message in case they think there's something wrong with our config.

I'd love the ability to hide this message, or perhaps you can provide a workaround with a webpack plugin that would filter out that particular message?

[phated]

@kaelig It's just a node flag https://nodejs.org/api/util.html#util_util_deprecate_fn_msg_code

[shilman]

@kaelig @phated looks to me like installing the addon-postcss should already disable the warning, no?

https://github.com/storybookjs/storybook/blob/next/lib/core/src/server/preview/base-webpack.config.ts#L67-L72

[kaelig]

Yes, however I'm not sure we'll want to install it. I'll try both approaches and see!

[prateekj16]

storybook with postcss8 works with npm but not with yarn