-
Notifications
You must be signed in to change notification settings - Fork 831
Forms: version iAPI modules with asset file hash instead of Jetpack version #45193
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
base: trunk
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
Significance: minor | ||
Type: changed | ||
|
||
Forms: change how Forms iAPI modules are versioned to avoid sticky cache busted versions |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,8 @@ | |
|
||
namespace Automattic\Jetpack\Forms\ContactForm; | ||
|
||
use Automattic\Jetpack\Constants; | ||
|
||
/** | ||
* This class serves as a container for what previously were standalone grunion functions. | ||
* In the long term we should aim to move things to other classes and gradually get rid of this rather than adding more. | ||
|
@@ -386,4 +388,20 @@ public static function get_export_filename( $source = '' ) { | |
sanitize_file_name( $source ) | ||
); | ||
} | ||
|
||
/** | ||
* Get version hash from generated `view.asset.php` files, or fallback to Jetpack version, to be used as cache bust for enqueing JS and CSS files. | ||
* | ||
* @param string $asset_file Path to view.asset.php file. | ||
* @return string either asset hash or Jetpack version. | ||
*/ | ||
public static function get_view_asset_version( $asset_file ) { | ||
$asset = file_exists( $asset_file ) ? require $asset_file : null; | ||
$jetpack_version = Constants::get_constant( 'JETPACK__VERSION' ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Alternatively we could fail loudly instead of falling back to JP version; now if the path is incorrect for example, dev might not notice. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Failing when we actually expect the file to exist is better IMHO. |
||
if ( empty( $jetpack_version ) ) { | ||
$jetpack_version = '0.1'; | ||
} | ||
|
||
return ! empty( $asset['version'] ) ? $asset['version'] : $jetpack_version; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could see this in Jetpack main plugin's
Assets
package as well.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why
view
in the method name? Won't it work for anyassets.php
file?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep! It does.
I'm thinking of renaming to better highlight the intent of using alongside
wp_enqueue_script_module()
. Or just leave it generic.