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

API for conditional loading assets #100

Closed
mitogh opened this issue Jan 18, 2022 · 7 comments
Closed

API for conditional loading assets #100

mitogh opened this issue Jan 18, 2022 · 7 comments
Labels
Needs Discussion Anything that needs a discussion/agreement wontfix This will not be worked on

Comments

@mitogh
Copy link
Member

mitogh commented Jan 18, 2022

Currently there's no API in core that allow to do that, aside from:

  • wp_enqueue_script
  • wp_enqueue_style

Having an API in place would help to load conditionally CSS and JS only when is required, for instance:

This CSS is only for the single.php page or this JS is only required for this custom post type or archive page.

Having an API on top of the existing ones to help Theme developers to only register specific files for specific conditions would be useful to decrease the overall size of the assets and only deliver meaningful and useful resources only when required instead of serving the same assets regardless of the current navigation path.


Related core issues:

@swissspidy
Copy link
Member

The easiest way to conditionally enqueue assets is by doing it just when you need it, for example in a block's render callback (or well, its block.json file nowadays) or in your shortcode's callback or your template file. Then the file will be enqueued in the footer.

An alternative is do do that conditional check within the wp_enqueue_scripts hook callback that should be used for enqueueing assets.

Not sure there needs to be a new API on top of that, which could potentially be used in the wrong way, for example by enqueueing assets too often.

Would love to hear more about potential use cases and advantages.

@sgomes
Copy link
Contributor

sgomes commented Jan 19, 2022

An argument can certainly be made that the "default way" of doing things is bad for performance, and we should be trying to nudge developers in the direction of splitting up their CSS per template, page, path, or whatever makes sense for their theme. A single theme CSS file isn't a great approach, in general.

However, I'm not sure if a new API is the best way of getting there, as I expect most of the work involved is in splitting things up in the first place, not adding a few PHP conditions here and there. Those are pretty straightforward, e.g.:

function mytheme_enqueue_styles() {
[...]
  if ( is_page_template( 'about.php' ) ) {
    wp_enqueue_style( 'about-styles', $CSS_PATH . '/includes/about.css', [] );
  }
[...]
}

@mitogh mitogh added Needs Discussion Anything that needs a discussion/agreement [Focus] JavaScript labels Jan 19, 2022
@aristath
Copy link
Member

Just noting here that since the future of WP is undeniably blocks and block-themes, it would make sense to focus there.
With that in mind, we have already implemented conditional styles loading for styles in blocks.
Core/themes/plugins can register a style for a specific block, and also add additional styles to a block. These are then only printed when that block exists in the current page.
We've also implemented inlining small stylesheets, so we've got styles 100% covered.
We started with styles and not scripts because it's a lot easier to handle (JS may have dependencies etc), but after the WP 5.9 release we plan on starting to implement these optimizations for scripts as well.

IMO, conditionally loading scripts and styles on a per-page basis doesn't make a lot of sense if we can do it more granular (on a per-block basis).

Granted, these optimizations don't apply to sites that don't use blocks or a block theme, but long-term it's the safest way to handle things because everything is a block. If something is not a block, it eventually will be 😆
The impact won't be immediate since it depends on the adoption of block themes, but eventually I feel that this approach will greatly improve performance & sustainability for all WordPress sites 👍

@swissspidy
Copy link
Member

We've also implemented inlining small stylesheets, so we've got styles 100% covered.

I wouldn't necessarily say 100% :-) Ideally only critical CSS is inlined, not just small styles. That might be something for a future enhancement there.

@aristath
Copy link
Member

Agreed. There's always room for improvement! 👍

@dainemawer
Copy link
Contributor

Agreed with @sgomes here - I think code splitting via routes is an incredibly powerful way of reducing redundant Code Coverage. For me thats one of the biggest pain-points with WP - we tend to load so much unconditional CSS that do nothing for the page but bloat it and ruin performance. I have created a ticket that explores PurgeCSS which could have some benefits and essentially remove the need for an API to handle route-based CSS:

#119

@swissspidy swissspidy added the wontfix This will not be worked on label May 10, 2024
@swissspidy
Copy link
Member

Revisiting some old issues on the repo, the consensus is that a dedicated API for conditionally loading assets is not the way to go. Closing as wontfix.

@swissspidy swissspidy closed this as not planned Won't fix, can't repro, duplicate, stale May 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Discussion Anything that needs a discussion/agreement wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

5 participants