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

Adding SASS / PostCSS build process to wp-scripts package #14801

Closed
fabiankaegy opened this issue Apr 3, 2019 · 23 comments · Fixed by #21730
Closed

Adding SASS / PostCSS build process to wp-scripts package #14801

fabiankaegy opened this issue Apr 3, 2019 · 23 comments · Fixed by #21730
Assignees
Labels
Good First Issue An issue that's suitable for someone looking to contribute for the first time [Package] Scripts /packages/scripts [Status] In Progress Tracking issues with work in progress [Type] Enhancement A suggestion for improvement.

Comments

@fabiankaegy
Copy link
Member

A discussion about having a Sass or PostCSS build process included in the wp-scripts package came up in the thread of #14590. Therefore I wanted to post it as an individual issue to keep conversations in one place.

@fabiankaegy
Copy link
Member Author

The goal of the wp-scripts package is, to make developing custom blocks as easy as possible. One big part of block development is the styling of these blocks. Because of the nature of blocks they have two separate stylesheets. One for the editing experience and one for the presentation on the page.

From what I've seen this far, there are many blocks being developed with a sass workflow. Even the core blocks use sass. Therefore I believe it might be a useful thing to include in the wp-scripts package.

@mor10 correctly pointed out though, that maybe going the PostCSS might also be an option. (#14590 (comment))
Maybe we can get some more input on this topic.

@gziolo gziolo added [Package] Scripts /packages/scripts [Type] Enhancement A suggestion for improvement. Good First Issue An issue that's suitable for someone looking to contribute for the first time labels Apr 4, 2019
@gziolo
Copy link
Member

gziolo commented Apr 4, 2019

I might get you confused with my previous comment about using Sass in Gutenberg :) It looks like we use both Sass and PostCSS. I did some more research and I found this PostCSS config:
https://github.com/WordPress/gutenberg/blob/master/bin/packages/post-css-config.js

This is how we convert *.scss files to CSS files shipped with npm packages:
https://github.com/WordPress/gutenberg/blob/master/bin/packages/build.js#L118-L155

I bet, it might be not great fit outside of Gutenberg but it should help to shape how processing CSS file might look like in @wordpress/scripts.

@youknowriad - do you have any recommendations on the topic?

@fabiankaegy fabiankaegy changed the title Adding SASS or PostCSS build process to wp-scripts package Adding SASS / PostCSS build process to wp-scripts package Apr 4, 2019
@youknowriad
Copy link
Contributor

I agree with the sentiment that SASS is less and less used in favor of PostCSS. I think this is a good thing but we're not there yet.

At the moment, we use SASS in Gutenberg and a lot of plugins already use it and I don't think we have valid reason to drop it and do a big refactor. We also don't have enough usage of PostCSS to be able to provide a good generic solution using it since we don't use it extensively ourselves.

I think if we were to add this command right now, I'll prefer if we do it with something we master and use:

  • Support SASS
  • Uses PostCSS to provide support for WordPress Admin Styles and RTL by default. (like we do in Gutenberg)

--
This doesn't mean we won't be able to replace SASS with full PostCSS in the future if needed. Since this is a development package and that upgrades are explicit, we can break backward compatibility for this kind of package (we follow SemVer) if needed.

@isvictorious
Copy link

I'm all for PostCSS but agree that there are still teams making that transition from Sass (myself included).

PostCSS or Sass processing are only items missing from WP-Scripts to make it a great singular dependency for building stuff in Gutenberg. As styling is an important part of Gutenberg I think they should be included.

Tooling & dependency management are going to be big issues as more people dive into Gutenberg development. Having a singular dependency to maintain for simple blocks makes dev adoption & workflow easier. At a recent 20+ person MeetUp I had for Gutenberg development, everyone got stuck on tooling & configuration.

In relation to @mor10's comment about PostCSS > Sass, could PostCSS be in the default configuration with documentation to change to a Sass workflow via Webpack configuration?

@mor10
Copy link
Contributor

mor10 commented Apr 4, 2019

could PostCSS be in the default configuration with documentation to change to a Sass workflow via Webpack configuration?

This is the path forward I would recommend. True, many developers are using Sass, and that's fine. However, the best-practice moving forward should be to use PostCSS, so the tool should have PostCSS as a built-in option with extended documentation on how to use Sass in a standardized way.

@mor10
Copy link
Contributor

mor10 commented Apr 4, 2019

@youknowriad: @ataylorme has done extensive work with PostCSS in WP Rig and should be able to provide input. The PostCSS team is also quite helpful when we've reached out to them.

From my perspective this is about setting the stage for future development. Sass sits firmly anchored in the past and old best-practices. Building it into tooling at this stage suggests to users that Sass is a viable path forward. That would be unfortunate since modern CSS with the assistance of PostCSS is the official path forward for the web platform. Building in further reliance on Sass at this point holds us back from what's happening elsewhere.

@ataylorme
Copy link

postcss-preset-env is really great. It takes care of a lot of the postcss plugins for you and will tweak support/settings based on your selected stage and browserlist declaration.

Here is an example postcss-preset-env config

postcssPresetEnv({
    // Start with a stage.
    stage: 3,
    // Override specific features you do (or don't) want.
    features: {
        // And, optionally, configure rules/features as needed.
        'custom-media-queries': {
            preserve: false
        },
        'custom-properties': {
            preserve: true
        },
        'nesting-rules': true
    }
})

@youknowriad
Copy link
Contributor

If we were to provide PostCSS by default, I think Gutenberg styles should be refactored to be PostCSS styles too. But for this to happen, we'll have to discuss it more broadly in #core-js and with the design team.

As I said above, I'm not against this idea myself but I don't want to derail the current priorities of our team but happy if there's someone willing to take a stab at it (after discussions with the teams above). A quick win could be to provide a build-styles command using our current setup while we figure out the migration separately.

@fabiankaegy
Copy link
Member Author

fabiankaegy commented Apr 5, 2019

What (sa/sc/c)ss files should the build process be looking for? Of course there are separate ones for the general styles and the editor styles. But I recently learned, that Gutenberg itself also uses a theme file.

Would this also make sense in the context of external plugins?

So should is be looking for:

  • style.(sa/sc/c)ss
  • editor.(sa/sc/c)ss
  • theme.(sa/sc/c)ss

or just

  • style.(sa/sc/c)ss
  • editor.(sa/sc/c)ss

@swissspidy
Copy link
Member

What (sa/sc/c)ss files should the build process be looking for?

What if it's up to the developer?

For example, I use https://github.com/webpack-contrib/mini-css-extract-plugin to extract CSS files one imports in JavaScript via import './edit.css' and then create a new stylesheet that combines all these files.

@fabiankaegy
Copy link
Member Author

The problem is that we don’t just want one output file, but at least two. The editor styles and the regular styles.
And you need some sort of way to sepperate the two

@ataylorme
Copy link

If we were to provide PostCSS by default, I think Gutenberg styles should be refactored to be PostCSS styles too

I respectfully disagree. Sass and PostCSS are not mutually exclusive. See this reply and follow-up from @SassCSS.

Just because PostCSS support is provided by wp-scripts does not mean Gutenberg styles need to be refactored.

Having the option for both would be ideal. For example, as a developer using wp-scripts in a block plugin, I could use Sass to handle imports/partials and PostCSS to add prefixes and polyfills for features such as custom properties if my source files went through both processes.

@youknowriad
Copy link
Contributor

Sass and PostCSS are not mutually exclusive. See this reply and follow-up from @sasscss.

That's entirely true, but what I'm trying to say is that if we use PostCSS, we need to know which PostCSS plugins to use, we have to test it and "prove it" ourselves otherwise it's going to be random choices. (It's very comparable to the usage of babel and our babel preset)

Sass could be used as well but PostCSS setups make most SASS features redundant and if we were to use them both today, we can just use the Gutenberg setup as we already do so but we limit PostCSS to just RTL and admin styles.

@ataylorme
Copy link

@youknowriad thank you for explaining your thinking more.

Sass could be used as well but PostCSS setups make most SASS features redundant

Which features do you see as being transferable? Here are some I think can be converted:

It's very comparable to the usage of babel and our babel preset

Yes, I agree. Using Babel encourages developers to use recent/future JS specifications, even if browser support is not caught up.

One advantage to the switch would be a similar use of CSS specifications. As browser support increases, and depending on the user's preference, polyfills and transformations would decrease over time. For example, custom properties are well supported.

@swissspidy
Copy link
Member

The problem is that we don’t just want one output file, but at least two. And you need some sort of way to sepperate the two

That's entirely possible. I was just giving an example / nudge into that direction :-)

@fabiankaegy
Copy link
Member Author

To extend wp-scripts I currently use this webpack.config.js. Which doesn't jet use any postcss, but compiles the scss for the two king of files.

@mor10
Copy link
Contributor

mor10 commented Apr 5, 2019

@fabiankaegy Just FYI, though it's commonly used in Gutenberg dev setups, the extract-text-webpack-plugin package is specifically not recommended for CSS:

Since webpack v4 the extract-text-webpack-plugin should not be used for css. Use mini-css-extract-plugin instead.

https://github.com/webpack-contrib/mini-css-extract-plugin

@fabiankaegy
Copy link
Member Author

Yes thanks for pointing that out again. I would do with the new mini version like @swissspidy also mentioned. Just haven’t given that one to build multiple css files.

@fabiankaegy
Copy link
Member Author

I created a basic first implementation of what we talked about here as a draft-pr. I'm not sure how to best test the wp-scripts package. Can someone help me by maybe recommending how to go about that?

I'm also not sure wether the postCss settings cover everything that they should cover.

Feedback is welcome :)

@isvictorious
Copy link

@fabiankaegy I will take a look and give feedback on Monday. At WordCamp Madrid and was speaking at NYPA conference yesterday. Jet lag :-/

@gziolo
Copy link
Member

gziolo commented Apr 28, 2020

#21730 is ready for feedback. The initial approach shares the same config for processing CSS as we can see for WordPress packages and Storybook. I even introduced a new npm package @wordpress/postcss-config to make it easier to change the config for all places. There were interesting proposals that could be included but I guess we can polish that separately unless I missed something obvious because of my lack of experience in this area.

@gziolo gziolo self-assigned this May 13, 2020
@mrleemon
Copy link
Contributor

mrleemon commented Jun 7, 2020

Just a question. I'm using the new Sass build process included in the wp-scripts package and I'm seeing a strange behavior in a plugin I'm developing which registers a PluginDocumentSettingPanel.

The main script includes a import './style.scss' line so the build process compiles the scss file into the /build folder. The build process goes fine with no errors in sight, the script gets enqueued correctly but, then, the PluginDocumentSettingPanel is not shown. If I remove the import './style.scss' line and rebuild the script, the PluginDocumentSettingPanel appears again.

Do any of you have any idea what could happen?

Thanks in advance

@gziolo
Copy link
Member

gziolo commented Jun 7, 2020

@mrleemon, it's a known issue. It's tracked in #22776. It's hard to tell what's the way to fix it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good First Issue An issue that's suitable for someone looking to contribute for the first time [Package] Scripts /packages/scripts [Status] In Progress Tracking issues with work in progress [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants