Fix/cache requests to remote asset checks#118
Conversation
zlateska
left a comment
zlateska
left a comment
There was a problem hiding this comment.
In the case where a status code of 200 is stored in the transient, if the files become unavailable at the remote URL, the local files (both CSS and JS) will not be loaded. We could consider the following options:
- Reduce the transient expiration time to 1 hour: This would result in less time with broken CSS and JS, but at the cost of increased requests compared to checking once a day.
- Use JavaScript to determine if the files have loaded: Dynamically load them if they are unavailable. This approach could potentially remove the need to validate the remote file in advance, but it would be more complex to develop.
includes/CSSUtilities.php
Outdated
| return false; | ||
| } | ||
| // Reverse the url because transient key length is limited and truncated and the unique part of the URL is its end. | ||
| $transient_key = 'nfd_css_utilities_valid_' . strrev($url ); |
There was a problem hiding this comment.
Should we use md5 instead of strrev for generating the transient key? md5 provides a fixed-length, unique hash that fits well within transient key length limits.
$transient_key = 'nfd_css_utilities_valid_' . md5($url);
There was a problem hiding this comment.
@zlateska I trust your judgement and think md5 is a good fit here
There was a problem hiding this comment.
I'm not too keen on MD5. We really only have two unique values here so don't need a true hash (it's computationally expensive) and although I'm reversing the string, it still lets us see which is the css and which is the js transient. Since it's a private function, we could comment the function with the possible inputs and use one of:
pathinfo( $url, PATHINFO_EXTENSION )=>cssbasename( $url )=>utilities.css
In the case of a single site admin, the file should be cached locally for that 24 hours, then if the files are not available next time they're polled, the local version will be used. Where this would be problem would be the second admin (or browser) trying to load the script a few hours later and it's not available remotely. I chose one-day for the expiry time because that's what was currently in use for the |
3 participants
Proposed changes
Currently, CSSUtilities::is_valid_remote_file() is not cached, resulting in eight remote calls when loading the WordPress editor as each of the CSS and JS's location and version are determined, once through
_wp_get_iframed_editor_assets()and once throughwp_common_block_scripts_and_styles().This PR:
time()as a once-per-day changing value withstrtotime( 'midnight' )::get_asset_url()and::get_asset_version()functions in favour of ternary operationsType of Change
Video
Checklist
Further comments