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

Feat: HTML Post Processing #5

Merged
merged 23 commits into from
Mar 5, 2021
Merged

Conversation

vkazakevich
Copy link
Collaborator

@vkazakevich vkazakevich commented Feb 24, 2021

Resolve #4

@vkazakevich vkazakevich changed the title Feat: HTML Post Processing [WIP Feat: HTML Post Processing [WIP] Feb 24, 2021
@vkazakevich vkazakevich marked this pull request as draft February 24, 2021 01:28
@gregpriday
Copy link
Member

This is looking good so far. Some of my initial feedback.

So I suggest we make a single Laravel job called something like OptimizeHtml. This job would load the HTML content from the filesystem, save it to a temporary file, then pass that filename to the optimization pipeline. Once the pipeline is done, it would save the file back to the filesystem.

This reduces the responsibility of each step in the optimization pipeline. All it needs to do is perform its optimizations to the file, then pass on some information to the next step. It doesn't need to worry about the filenames and it knows it has a locally stored file to deal with.

This would make a difference if someone is using S3 to store the files and is using multiple queue workers.

A developer should be able to configure the pipeline. Choosing which steps are run, and which aren't. They should also be able to add their own optimization steps - much like they'd add HTTP Middleware.

'optimizations' => [
   \SiteOrigin\PageCache\CriticalCss::class,
   \SiteOrigin\PageCache\MinifyHtml::class,
   \SiteOrigin\PageCache\AutoImgixSrcset::class,
]


protected function injectCriticalCss($contents, $css)
{
return str_replace('</head>', "<style>{$css}</style>" . PHP_EOL . "</head>", $contents);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't actually tested this, but I think this module handles injecting the new CSS while also deferring the existing stylesheets.

@gregpriday
Copy link
Member

Oh wait, I see how you'd do this. So you'd create a base job class that handles setting the temporary filename, and then have all of these jobs inherit from that class. Then inside the main OptimizeHtml, you'd just create the temp file and send that to a job chain and run all the jobs synchronously to ensure they all run on the same instance with access to the temporary file.

@vkazakevich vkazakevich marked this pull request as ready for review February 26, 2021 01:13
@vkazakevich vkazakevich changed the title Feat: HTML Post Processing [WIP] Feat: HTML Post Processing Feb 28, 2021
@gregpriday
Copy link
Member

@vkazakevich I actually removed all the CDN responsibility and replaced it with simple events. This allows other projects to handle an after-optimization event however they want. In the case of Briefer, we can hook into our existing CloudFlare service class.

@gregpriday gregpriday merged commit 7feaef7 into develop Mar 5, 2021
@gregpriday gregpriday deleted the feature/html-post-processing branch March 5, 2021 09:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

HTML Post Processing
2 participants