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

Extending wp scripts to handle scss #14847

Conversation

fabiankaegy
Copy link
Member

Description

As discussed in #14801 I added loaders for css, scss/sass that compile and afterwards run PostCSS with the postcss-preset-env defaults.

This is a first implementation that its not very well tested. I need some help with that, because I'm not sure how to best test the wp-scripts package by itself.

How has this been tested?

The code only passes the linting rules at this point

@gziolo gziolo added [Tool] WP Scripts /packages/scripts [Type] Enhancement A suggestion for improvement. [Type] Build Tooling Issues or PRs related to build tooling labels Apr 6, 2019
@oandregal
Copy link
Member

Can provide help on setting up a testbed for this. This is how I've done it in the past: oandregal/understanding-gutenberg#7

  • created an external plugin that uses @wordpress/scripts package
  • note that package.json uses local paths to point to @wordpress/scripts (see).
  • make sure any plugin that uses this don't have to install anything else other than @wordpress/scripts for the command to work.

I'm a bit out of the loop here in terms of what we want to achieve, but can help to put together an example/testbed if provided with the higher-level API for plugins. For example, is the idea that:

  • wp-scripts build / wp-scripts start take any SCSS/SASS and compile it to CSS somewhere?
  • to create a new command such as wp-scripts build-css that does the same?

@gziolo
Copy link
Member

gziolo commented Apr 24, 2019

@isvictorious, @mor10, @swissspidy or @ataylorme, how is this proposal looking so far?

This is a first implementation that its not very well tested. I need some help with that, because I'm not sure how to best test the wp-scripts package by itself.

@fabiankaegy, I think @nosolosw gave you some good hint on how this can be tested manually. It would be great to automate it with some sort of unit/integration testing. This is how webpack plugins are tested:

It isn't easy to set up but then it should be quite easy to maintain with Jest snapshots.

@ockham
Copy link
Contributor

ockham commented May 7, 2019

@youknowriad Any chance you'd reconsider your stance on using webpack loaders for the styling stack? 🙂 This is one of the main differences between @wordpress/scripts and @automattic/calypso-build (internal reference: p4TIVU-9fc-p2).

From what I'm reading (and what I recall from previous discussions), being able to build separate stylesheets from what would be imports into the same file is one of the major motivations to stick with a non-webpack-loader-based toolchain. I'm obviously biased, but I'm not sure that's really worth foregoing a webpack-based solution (which nicely integrates with the existing webpack-based bundling), and running a bunch of binaries (like node-sass and postcss) one after the other ourselves instead.

Case in point: Isn't entry-point level granularity fine-grained enough for style files? I.e. if we want separate styling for editor and frontend, doesn't it make sense to align the styling with the JS and simply have two different entrypoints (hence chunks)? To me, aligning CSS and JS chunks this way seems rather intuitive; I'm wondering if having more fine-grained control over style chunks is worth the added complexity 🙂

@youknowriad
Copy link
Contributor

I think this discussion clarifies my arguments #14847 (comment)

Case in point: Isn't entry-point level granularity fine-grained enough for style files? I.e. if we want separate styling for editor and frontend, doesn't it make sense to align the styling with the JS and simply have two different entrypoints (hence chunks)? To me, aligning CSS and JS chunks this way seems rather intuitive; I'm wondering if having more fine-grained control over style chunks is worth the added complexity

When you build a block, you need to import both the edit and the save for the block in the editor and which means these can't be separated into two different JS entry points while the styles are loaded differently. The backend loaded three stylesheets (editor, theme and style) while the frontend loads one (style) and an opt-in one (theme).

I understand that for a full SPA, this is not an issue but for WP themes and WP plugins, it's very important to be able to control the entry points of stylesheets and load them separately in different pages.

@kadamwhite
Copy link
Contributor

I'm not sure I understand why supporting the importing of CSS files within JS is at odds with splitting out CSS into different builds. We regularly use multiple entrypoints for this on complex projects without issue. If I'm developing a block and importing scss files within the JS code for a theme project, I'd import the any non-add_editor_style SCSS directly from the edit component, then import any frontend styles either from a base entrypoint for the editor stylesheet or from whatever frontend-logic script entrypoint the theme uses. One set of files gets loaded for use in the editor, the other for the frontend (or several potential bundles for the frontend and several potential bundles for the editor, if using multiple post types/conditional enqueues/etc) and I've yet to find that insufficient.

Is the issue here that if we permit the expected usage of the sass loader, we're going to force something internal to how Gutenberg has done this previously to change? Or have I misunderstood this issue? I'll admit it's a long thread and I may have gotten lost, but it doesn't feel like these things are at odds.

@kadamwhite
Copy link
Contributor

To contextualize the above comment: anecdotally, every time I've spoken with theme or plugin developers at a WordCamp over the past two months they've asked when SASS support is coming. As I've not been involved in the project I haven't had visibility into the answer, but it's a very common question. And most of those developers are following Webpack patterns and guidelines from the broader web community, which tend to favor importing the styles within the module. This is something I think therefore that it is very important we support, because if we say "we use all the same tools you do, but you have to use them in this strange (to your perspective) way" that's quite off-putting for any potential future contributors or other web developers who want to use patterns they know.

@mor10
Copy link
Contributor

mor10 commented Oct 8, 2019

My two cents:

Support both stand-alone stylesheets and CSS-in-JS, and make a consistent and followable standard for both based on wider web community best-practices. There are times when stand-alone stylesheets are the right decision, times when CSS-in-JS is the right decision, and times when either is the right decision. Likewise, there are times when stand-alone CSS or CSS-in-JS is decidedly the wrong decision, and people need to know the difference.

What I'm seeing a lot of these days is developers (WP and beyond) doubling or tripling down on one of the tow and doing everything with only one method. This is not a good starting point. Defining meaningful and reasonable best-practices and standards for developers to follow will be helpful for everyone.

As for Sass, it's slowly falling to the wayside in favor of pure CSS (currently via PostCSS). People will keep using it the same way people keep using jQuery, but the percentage of users will slowly diminish unless authority figures or projects actively encourage its use. I suggest providing Sass compilation as an option at the top of the stack as proposed, so that developers can use custom properties and all the other new CSS goodness alongside Sass and then process everything through PostCSS.

@gziolo gziolo marked this pull request as ready for review April 20, 2020 14:23
@gziolo
Copy link
Member

gziolo commented Apr 20, 2020

I opened #21730 to rebase this PR and continue the work started. I still need to digest the feedback shared so far before I close this one. @fabiankaegy thank you for all the work done so far ❤️

@gziolo gziolo closed this Apr 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Tool] WP Scripts /packages/scripts [Type] Build Tooling Issues or PRs related to build tooling [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants