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

[4.x]: {% js %} and {% css %} tags are ignored inside any {% cache %} tags with pending image transforms #11602

Closed
realjoshharrison opened this issue Jul 14, 2022 · 8 comments

Comments

@realjoshharrison
Copy link

realjoshharrison commented Jul 14, 2022

What happened?

Description

Any {% js %} and {% css %} tags used inside a {% cache %} tag are not registered if the cache tag also contains pending image transforms.

Reproduced on a clean Craft 4 install with no plugins.

Steps to reproduce

  1. Make sure there's an image asset in the Assets library
  2. Load up a template containing this:
{# These tags all register as expected #}

{% css 'global-style.css' %}

{% css %}
body::before {
  content: 'Global style';
  display: block;
}
{% endcss %}

{% js 'global-script.js' %}

{% js %}
console.log('Global hello');
{% endjs %}


{#
Inside this {% cache %} tag, the css and js bits will not be registered 
until the image transforms finish generating
#}

{% cache %}

  {% css 'block-style.css' %}

  {% css %}
  body::after {
    content: 'Block style';
    display: block;
  }
  {% endcss %}

  {% js 'block-script.js' %}

  {% js %}
  console.log('Block hello');
  {% endjs %}

  {% set asset = craft.assets.one() %}
  {{ asset.getImg({ width: 300, height: 300 }, ['1.5x', '2x', '3x']) }}

{% endcache %}

Expected behavior

All CSS and JS inside the {% cache %} tag is registered, regardless of image transform status.

Actual behavior

All CSS and JS registered inside the {% cache %} tag gets ignored until the image transforms finish generating.

Craft CMS version

4.1.4.1

PHP version

8.0.8

Operating system and version

MAMP Pro, Darwin 21.5.0, Apache 2.4.48 (MacOS 12.4)

Database type and version

MySQL 5.7.34

Image driver and version

ImageMagick 6.9.6-2

Installed plugins and versions

None

@realjoshharrison
Copy link
Author

realjoshharrison commented Jul 14, 2022

My guess is it's to do with the ordering of craft\services\TemplateCaches::endTemplateCache lines 136-149. The css and js buffers have just been cleared, then the function bails due to pending transforms without re-registering anything.

if ($withResources) {
$view = Craft::$app->getView();
$bufferedJs = $view->clearJsBuffer(false, false);
$bufferedScripts = $view->clearScriptBuffer();
$bufferedCss = $view->clearCssBuffer();
$bufferedJsFiles = $view->clearJsFileBuffer();
$bufferedCssFiles = $view->clearCssFileBuffer();
}
// If there are any transform generation URLs in the body, don't cache it.
// stripslashes($body) in case the URL has been JS-encoded or something.
if (StringHelper::contains(stripslashes($body), 'assets/generate-transform')) {
return;
}

I don't know what the implications would be, but would moving the return to before the if ($withResources) { block sort this?

@sunscreem
Copy link

Is there a chance this occurs in Craft 3 too? I've been having a problem for sometime that would be explained by this.

@realjoshharrison
Copy link
Author

Possibly – support for cached JS/CSS inline tags was added in Craft 3.7 (#7758), but support for registering external JS/CSS files such as {% css 'path/to/style.css' %} was only added in Craft 4 (#9987)

@realjoshharrison
Copy link
Author

I think a temporary workaround is to add this to config/general.php:

[
    'generateTransformsBeforePageLoad' => true,
]

https://craftcms.com/docs/3.x/config/config-settings.html#generatetransformsbeforepageload

This isn't really ideal though, as pages with lots of transforms can take an age to load on first view...

@brandonkelly
Copy link
Member

Thanks for reporting! Just fixed for the next Craft 3 and 4 releases.

You can get the fix early by changing your craftcms/cms constraint in composer.json to v3.x-dev as 3.7.48 (Craft 3) or dev-develop as 4.1.4.1 (Craft 4) and running composer update.

@realjoshharrison
Copy link
Author

Thanks so much for the speedy fix @brandonkelly! I can’t yet see it in the changelog on the develop branch – is it definitely fixed for 4.x as well as 3?

@brandonkelly
Copy link
Member

@realjoshharrison I generally haven’t been copying Craft 3.x changes’ release notes over to the Craft 4 changelog, but I should have. Just fixed that (18dda57).

@brandonkelly
Copy link
Member

brandonkelly commented Jul 26, 2022

Craft 3.7.49 and 4.2.0 are out with this fix 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants