-
Notifications
You must be signed in to change notification settings - Fork 20
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
RFC: Filename based entrypoint management for block specific CSS #339
Comments
@10up/frontend-leads @Antonio-Laguna @nicholasio @joesnellpdx I would love to get your thoughts on this :) |
I like this! Totally voting for I have a few thoughts/potential concerns:
For your last comment, do you think we can follow the WordPress pattern of adding comments at the top of the file to handle this? Like
Where "Compile" means don't move this block to dist; we might include it from other file or just test without it. |
@fabiankaegy great idea! I'm not sure I fully get the downside here as this is opt-in? This is a handy mechanism but not forced but I might be missing something here. @xavortm some thoughts
This could be solved with https://github.com/csstools/postcss-plugins/tree/main/plugins/postcss-global-data provided you have things properly set up in their files
Definitely needs some architectural thinking, IMO global styles are there first and then blocks come in with more specificity by just order. In your example, heading styles should be probably global whereas those other styles come afterwards, scoped to that given group/component
As before I think in this cases you need to plan ahead anyways as you would without this approach. Write styles as low specific as possible. |
@xavortm great questions! And I'm not sure I have all the answers but here is what comes to mind initially :)
We already have this "issue" with the styles bundled with custom blocks. And the way that we've solved it on a few projects now is by using the These stylesheets cannot contain any actual output but allow us to share mixins, variables, etc actoss files :)
Very good question. I don't have a good answer for this currently. I think this will either have to be solved via CSS layers or outwright specificity. Usually
Yup the file based imports would be completely unaware of whether or not a block actually exists. All it would do is use the filename to call the Regarding the order of how it gets enqueued I'm not entirely certain I know the answer but I think so 🤔
I kind of like the idea of the metadata header in the stylesheets 🤔 I'd be interrested in exploring how we woulc strip this out for the production build of the asset but otherwise I think we could do some cool stuff with this 🤔 :) |
This comment was marked as resolved.
This comment was marked as resolved.
Yeah it would be opt in and for that directory only. I think what I was thinking about here is that I would still want all my block specific CSS to be located in one spot. Regardless of how it gets enqueued or not. But with this approach I would have to move individual files somewhere else if I didn't want them to be loaded automatically. But that is probably overthinking it anyways :) |
Also I'll note that #168 is very relevant to this proposal here |
Okay no layers yet 👍 😂 Going with just specificity is fine and not different to how it works currently :) |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
I created a protorype for this in a local scaffold. This here is the PHP we would need to add to the scaffold for this to work: /**
* Enqueue block specific styles.
*/
function enqueue_theme_block_styles() {
$stylesheets = glob( TENUP_THEME_DIST_PATH . '/blocks/autoenqueue/**/*.css' );
foreach ( $stylesheets as $stylesheet_path ) {
$block_type = str_replace( TENUP_THEME_DIST_PATH . '/blocks/autoenqueue/', '', $stylesheet_path );
$block_type = str_replace( '.css', '', $block_type );
$asset_file = TENUP_THEME_DIST_PATH . 'blocks/autoenqueue/' . $block_type . '.asset.php';
if ( ! file_exists( $asset_file ) ) {
$asset_file = require $asset_file;
} else {
$asset_file = [
'version' => filemtime( $stylesheet_path ),
'dependencies' => [],
];
}
wp_enqueue_block_style(
$block_type,
[
'handle' => "tenup-theme-{$block_type}",
'src' => TENUP_THEME_DIST_URL . 'blocks/autoenqueue/' . $block_type . '.css',
'path' => $stylesheet_path,
'version' => $asset_file['version'],
'dependencies' => $asset_file['dependencies'],
]
);
}
}
add_action( 'init', 'enqueue_theme_block_styles' ); This currently finds all the css files in the |
I moved the postcss global data plugin conversation into a separate issue since these two things can be handled separately from one another #340 |
For #168 the solution would likely need PHP code to automatically enqueue the JS generated from toolkit that would enable hot reloading. I'm starting to think it might be a good idea to ship a toolkit composer package (cc @darylldoyle ) to consolidate the way we're loading assets. We could have toolkit generate a manifest file that this package would read in order to load styles (this would avoid having to traverse directories in PHP). This package would replace the custom code we're adding to functions.php to enable HMR. |
I also created a quick prototype for how one could extend a 10up/toolkit config to support this: const config = require('10up-toolkit/config/webpack.config.js');
const { sync: glob } = require('fast-glob');
const { resolve, extname } = require('path');
function getBlockStylesheetEntrypoints() {
// get all stylesheets located in the assets/css/blocks directory and subdirectories
const blockStylesheetDirectory = resolve(process.cwd(), './assets/css/blocks');
// get all stylesheets in the blocks directory
const stylesheets = glob(
// glob only accepts forward-slashes this is required to make things work on Windows
`${blockStylesheetDirectory.replace(/\\/g, '/')}/**/*.css`,
{
absolute: true,
},
);
const additionalEntryPoints = {};
stylesheets.forEach((filePath) => {
const blockName = filePath
.replace(`${blockStylesheetDirectory}/`, '')
.replace(extname(filePath), '');
additionalEntryPoints[`autoenqueue/${blockName}`] = resolve(filePath);
});
return additionalEntryPoints;
}
const coreEntryPoints = config.entry();
const additionalEntryPoints = getBlockStylesheetEntrypoints();
config.entry = Object.assign(coreEntryPoints, additionalEntryPoints);
module.exports = config; |
Hey @dmtrmrv 👋 Have you had a chance to work on this yet? I'd love to move forward with this soon and just want to get a quick update on whether you already were able to start on it or whether you will have some time to work on it in the near future? :) No worries if not. I can always take over if that makes more sense 👍 |
Hey @fabiankaegy! I had some time to work on it before my PTO last week. Give me a couple more days and I'll send the PR over. Thank you for your patience with me on this. |
Currently, the mechanism of adding new individual entry points is rather manual. The file has to be added to
package.json
, and then you also have to manually enqueue the asset for WordPress. This is a primary factor why we are still creating one monolithic CSS bundle instead of per-block custom CSS.With the addition of the
useBlockAssets
hook we already made this a whole lot easier for custom block development. But when we want to add styles for specific blocks we tend to do this via the one bigfrontend.css
file which gets enqueued on every single page of the site.This leads to the issue that the majority of the styles in that file are not actually needed on the page and therefore we waste a lot of resources.
WordPress core has been providing us with a new improved way for handling this via individual block styles that only get loaded when the actual block is used on the page. This essentially allows us automatic code splitting if we enqueue our individual block stylesheets via the
wp_enqueue_block_style
function.It would be great to have this happen in a more automated fashion where we can have a designated folder in the
assets
directory where every file within that folder automatically gets added to the webpackentry
list as its own entrypoint.The generated dist files should then automatically get enqueued by WordPress for their respective block via the filename of the file.
In order to work properly this will need to allow folder names because the structure of the block names is
namespace/block-name
and it it important for us to know both pieces of information.Possible Downsides
The one downside to this approach is that unlike the automatic handling of the block assets of a custom block we are developing, with this we would have no control to intercept the mechanism for the automatic enqueueing here. With the block assets you can always not add a CSS file to the
block.json
or add it without thefile:
prefix for it to get ignored and then you have to manually handle it.Maybe it makes more sense to still handle the entrypoint addition manually via the
package.json
but then have WordPress automatically handle the enqueueing of the block CSS assets...The text was updated successfully, but these errors were encountered: