Ensure an Asset Version is Always Present #13
Conversation
This extension ensure that we always return some version by checking the file modified time or defaulting to the installed WP Version.
|
|
||
| $version = $this->version(); | ||
|
|
||
| if ( empty( $version ) ) { |
There was a problem hiding this comment.
This method is a generic helper to bump the asset version number with every plugin update so maybe we could introduce a new helper for getting a file-specific version string based on filemtime()?
| if ( ! empty( $file ) && file_exists( $file ) ) { | ||
| $version = (string) filemtime( $file ); | ||
| } else { | ||
| $version = $GLOBALS['wp_version']; |
There was a problem hiding this comment.
Could we pass the WP version string as a dependency to the __constructor() to avoid coupling this class with a global state.
There was a problem hiding this comment.
Not sure I see the value of adding the global check as there should always be a version in the file header. We control that and is expected. If it's not there then we should throw an error in the version() method.
if ( $this->is_debug() || $this->is_script_debug() ) {
return (string) time();
}
if ( ! empty( $file ) && file_exists( $file ) ) {
return (string) filemtime( $file );
}
return $this->version();
| @@ -164,7 +186,7 @@ public function asset_version() { | |||
| public function meta( $field = null ) { | |||
| static $meta; | |||
|
|
|||
| if ( ! isset( $meta ) ) { | |||
| if ( ! isset( $meta ) && function_exists( 'get_plugin_data' ) ) { | |||
There was a problem hiding this comment.
Could we switch to using get_file_data() which appears to be always available?
There was a problem hiding this comment.
We could but get_plugin_data does a better job at sane defaults.
If we continue to use it we need to add:
if ( ! function_exists( 'get_plugin_data' ) ) {
require_once( ABSPATH . 'wp-admin/includes/plugin.php' );
}
| * @param string $path_relative Path relative to this plugin directory root. | ||
| * @return string The path to the asset. | ||
| */ | ||
| public function asset_path( $path_relative = '' ) { |
There was a problem hiding this comment.
This should always expect a valid input similar to asset_url().
| * @return string The path to the asset. | ||
| */ | ||
| public function asset_path( $path_relative = '' ) { | ||
| return plugin_dir_path( $this->file ) . $path_relative; |
There was a problem hiding this comment.
We already have $this->dir so we could use something like:
return sprintf( '%s/%s', $this->dir, ltrim( $path_relative, '/' ) );
|
@kopepasah in the future could you avoid working from a personal fork as that makes it impossible to work collaboratively. |
|
Fixed in #16. |
Yep, not a problem. At the time, I did not have any access to do so. |
|
Thanks @kasparsd and @derekherman for pushing this forward! |
Fixes #12
This pull request ensures that when using the
$plugin->asset_version()method, a version will always be output.