Drupal\Core\Asset\CssCollectionOptimizer is deprecated! [#3394740]

Problem/Motivation

In #3365122, we reverted the CSS collection optimizer service in Drupal 10.1+ to its state prior to Drupal 10.1.

This bought some time, but something else will have to be implemented for 11.0.

Proposed resolution

Given so much has changed in core, any path forward here will be disruptive. After reviewing the landscape of the core issues surrounding supporting the inlining of CSS, I've concluded that there likely is a strong case for supporting this module in Drupal 11+ given the likelihood of a performant core solution for inlining CSS is blocked by some very complicated issues and is likely very far off.

Adjust support range to drop Drupal 10 support. Too much has changed to support two majors here...
Steal some edge case tests from @larowlan from #3499829: Support inlining critical CSS for faster core web vitals 💙
Implement specialized asset collection optimizer and renderer services.

There will probably be more complexity here, but I don't see any way around that.
Decorate the asset chain cache to prevent downstream cache corruption.

The decorated cache backend should be able to "remember" which CIDs were associated with the request that could have possibly corrupt assets and prevent the corruption from propagating downstream.

Remaining tasks

This changeset needs some hardcore test coverage added.

User interface changes

None

API changes

Many. Things might spill over into a new major.

Data model changes

None

Comment	File	Size	Author
#4	3394740-4.patch	6.66 KB	luke.leber
#3	3394740-3.patch	6.52 KB	luke.leber
#2	3394740-2.patch	4.83 KB	luke.leber

Issue fork inline_all_css-3394740

Show commands

Start within a Git clone of the project using the version control instructions.

Add & fetch this issue fork’s repository

Or, if you do not have SSH keys set up on git.drupalcode.org:

Add & fetch this issue fork’s repository

3394740-drupalcoreassetcsscollectionoptimizer-is-deprecated changes, plain diff MR !11
Check out this branch for the first time

Check out existing branch, if you already have it locally

About issue forks

Comments

Comment #1

18 October 2023 at 00:29

Luke.Leber created an issue. See original summary.

Comment #2

luke.leber

English

Pennsylvania

commented 20 October 2023 at 21:41

Status	File	Size
new	3394740-2.patch	4.83 KB

Let's try this, to get started...

Comment #3

luke.leber

English

Pennsylvania

commented 21 October 2023 at 01:12

Status	File	Size
new	3394740-3.patch	6.52 KB

Let's fix some tests...

Comment #4

luke.leber

English

Pennsylvania

commented 21 October 2023 at 15:53

Status	File	Size
new	3394740-4.patch	6.66 KB

2 files were hidden/shown/deleted

Status	File	Size
hidden	3394740-2.patch	4.83 KB
hidden	3394740-3.patch	6.52 KB

Fix coding standard violation.

Comment #5

luke.leber

English

Pennsylvania

commented 21 October 2023 at 15:54

Status:

Active

» Needs review

Setting to NR; also requires manual testing against real sites.

Comment #6

alan delval commented 9 November 2023 at 00:07

Did not work. Tested and found that works sometimes due to how HttpKernel works. The first request file is created and returns an instance of Response, afterward it returns an instance of BinaryFileResponse because the file already exists on file system.

// Since Drupal 10, assets are lazily constructed.
$sub_request = Request::create($asset['data']);

$raw_request = $this->httpKernel->handle($sub_request, HttpKernelInterface::SUB_REQUEST);

if ($raw_request instanceof \Symfony\Component\HttpFoundation\BinaryFileResponse) {
    $css_request = $raw_request->getFile()->getContent();
}
else {
    $css_request = $raw_request->getContent();
}

    $css .= $css_request;

Comment #7

luke.leber

English

Pennsylvania

commented 9 November 2023 at 00:17

Thanks for the manual test!

It sounds like it's going to be hard to kernel test this...almost sounds race condition-ny...maybe we'll have to go heavier into functional test territory to land this.

Comment #8

luke.leber

English

Pennsylvania

commented 9 November 2023 at 00:18

Status:

Needs review

» Needs work

Comment #9

catch

he/him

English

commented 13 August 2024 at 09:25

I think it would be more maintainable long term to copy some of the logic in AssetControllerBase::deliver() (can be massively slimmed down because it doesn't need all the request validation) - build what would have been the filename contents from the contents of the $css_assets array, and store it in the Drupal object cache rather than as a file.

Comment #10

luke.leber

English

Pennsylvania

commented 10 February 2025 at 15:49

Comment #11

30 August 2025 at 02:06

luke.leber opened merge request !11

Comment #12

luke.leber

English

Pennsylvania

commented 31 August 2025 at 03:12

So there are still some major issues -- namely a failure to inline an asset seems to corrupt the asset resolver cache...and there doesn't seem to be a way to spy on the CIDs to invalidate in those cases 😭.