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.

Sign up for GitHub

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

Enable multiple registrations #29

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Enable multiple registrations #29

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
2cb12a5
Enable multiple registrations
dpanta94 Dec 17, 2024
815003e
Fix static analysis
dpanta94 Dec 17, 2024
9cb46f6
Add testcases
dpanta94 Dec 17, 2024
23a3b6b
fix static analysis
dpanta94 Dec 17, 2024
520eca8
unroll magic method
dpanta94 Jan 10, 2025
70c559a
fix static analysis
dpanta94 Jan 10, 2025
ce549ec
update docblocks
dpanta94 Jan 10, 2025
1223e3b
added default registration values
dpanta94 Jan 10, 2025
2616c76
added test coverage
dpanta94 Jan 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
166 changes: 166 additions & 0 deletions src/Assets/Asset.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@

use InvalidArgumentException;

/**
* Class Asset.
*/
class Asset {
/**
* @var array The asset action.
Expand Down Expand Up @@ -1696,27 +1699,190 @@ public function set_as_registered() {
return $this;
}

/**
* Prints the asset using wp_print_scripts or wp_print_styles.
*
* @since 1.4.4
*
* @return static
*/
public function print_asset() {
$method = 'wp_print_' . $this->get_script_or_style() . 's';
$method( $this->get_slug() );
return $this;
}
Comment on lines +1709 to +1713
Copy link
Member

Choose a reason for hiding this comment

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

This is confusing to have along with do_print(). Is there a reason to keep this separated from do_print()? Should we change the behavior of do_print() to call wp_print_* directly so that we don't have the extra method that might get confused with print()?


/**
* Enqueues the asset using wp_enqueue_script or wp_enqueue_style.
*
* @since 1.4.4
*
* @return static
*/
public function enqueue_asset() {
$method = 'wp_enqueue_' . $this->get_script_or_style();
$method( $this->get_slug() );
return $this;
}
Comment on lines +1722 to +1726
Copy link
Member

Choose a reason for hiding this comment

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

This could be confused with enqueue(), which should be doing the enqueueing anyway. I'm not sure what the value from this is? The Asset::enqueue() method should be doing this already.

The question I have is this: What is broken about enqueue() (ultimately Assets::enqueue() and Assets::do_enqueue()) that is causing the creation of this method? As of right now, it feels redundant and confusing.

I totally admit that I might be missing something and there is a very very valid reason. I'm just unsure I see it without some tips/hints on what to look at.

The concept behind the Assets library

The goal is to tell WordPress about an asset so that it can be registered and enqueued at the right moment so the dev doesn't need to go to the precise location and call register and enqueue functions directly. If the dev wants to do that, then the Asset library isn't really adding much benefit.

The Asset is meant to hold information about the asset itself and Assets was for orchestrating the output. That was the original idea, at least.


/**
* Registers the asset using wp_register_script or wp_register_style.
*
* @since 1.4.4
*
* @param string $url The URL to the asset.
* @param array $dependencies The dependencies for the asset.
* @param string $version The version of the asset.
* @param bool|string $in_footer_or_media Whether to enqueue in the footer or the media type for the asset.
*
* @return static
*/
public function register_asset( string $url = '', array $dependencies = [], string $version = null, $in_footer_or_media = 'all' ) {
$url = $url ? $url : $this->get_url();
$dependencies = $dependencies ? $dependencies : $this->get_dependencies();
$version = $version ?? $this->get_version();
$in_footer_or_media = $in_footer_or_media ? $in_footer_or_media : ( $this->is_js() ? $this->is_in_footer() : $this->get_media() );

$method = 'wp_register_' . $this->get_script_or_style();
$method( $this->get_slug(), $url, $dependencies, $version, $in_footer_or_media );
return $this;
}
Comment on lines +1740 to +1749
Copy link
Member

Choose a reason for hiding this comment

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

This feels redundant with register(). If we really need this logic in here, is it best to replace the functionality of register()? To skip around that method sorta breaks the library's purpose.


/**
* Dequeues the asset using wp_dequeue_script or wp_dequeue_style.
*
* @since 1.4.4
*
* @return static
*/
public function dequeue_asset() {
$method = 'wp_dequeue_' . $this->get_script_or_style();
$method( $this->get_slug() );
return $this;
}

/**
* Deregisters the asset using wp_deregister_script or wp_deregister_style.
*
* @since 1.4.4
*
* @return static
*/
public function deregister_asset() {
$method = 'wp_deregister_' . $this->get_script_or_style();
$method( $this->get_slug() );
return $this;
}

/**
* Check if the asset is something.
*
* In the background uses wp_script_is or wp_style_is.
*
* @since 1.4.4
*
* @param string $what The what to check against.
*
* @return bool
*/
public function asset_is( string $what ): bool {
return ( 'wp_' . $this->get_script_or_style() . '_is' )( $this->get_slug(), $what );
}

/**
* Get the script or style based on the asset type.
*
* @since 1.4.4
*
* @return string
*/
protected function get_script_or_style(): string {
return 'js' === $this->get_type() ? 'script' : 'style';
}

/**
* Prints the asset
*
* @since 1.4.4
*
* @return static
*/
public function do_print() {
if ( $this->should_print() && ! $this->is_printed() ) {
$this->set_as_printed();
$this->print_asset();
}

// We print first, and tell the system it was enqueued, WP is smart not to do it twice.
$this->enqueue_asset();

if ( ! $this->is_css() ) {
return $this;
}

foreach ( $this->get_style_data() as $key => $value ) {
wp_style_add_data( $this->get_slug(), $key, $value );
}

return $this;
}

/**
* Performs the asset registration in WP.
*
* @since 1.4.4
*
* @return static
*/
public function do_register() {
$this->register_asset( $this->get_url(), $this->get_dependencies(), $this->get_version(), $this->is_js() ? $this->is_in_footer() : $this->get_media() );
$this->set_as_registered();

if ( $this->is_js() ) {
if ( empty( $this->get_translation_path() ) || empty( $this->get_textdomain() ) ) {
return $this;
}

wp_set_script_translations( $this->get_slug(), $this->get_textdomain(), $this->get_translation_path() );
return $this;
}


$style_data = $this->get_style_data();
if ( $style_data ) {
foreach ( $style_data as $datum_key => $datum_value ) {
wp_style_add_data( $this->get_slug(), $datum_key, $datum_value );
}
}

return $this;
}

/**
* Set the asset enqueue status to false.
*
* @since 1.0.0
* @since 1.4.4 - Actually dequeues the asset.
*
* @return static
*/
public function set_as_unenqueued() {
$this->is_enqueued = false;
$this->dequeue_asset();
return $this;
}

/**
* Set the asset registration status to false.
*
* @since 1.0.0
* @since 1.4.4 - Actually deregisters the asset.
*
* @return static
*/
public function set_as_unregistered() {
$this->is_registered = false;
$this->deregister_asset();
return $this;
}

Expand Down
83 changes: 9 additions & 74 deletions src/Assets/Assets.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -643,27 +643,7 @@ protected function do_enqueue( Asset $asset, bool $force_enqueue = false ): void
return;
}

if ( 'js' === $asset->get_type() ) {
if ( $asset->should_print() && ! $asset->is_printed() ) {
$asset->set_as_printed();
wp_print_scripts( [ $slug ] );
}
// We print first, and tell the system it was enqueued, WP is smart not to do it twice.
wp_enqueue_script( $slug );
} else {
if ( $asset->should_print() && ! $asset->is_printed() ) {
$asset->set_as_printed();
wp_print_styles( [ $slug ] );
}

// We print first, and tell the system it was enqueued, WP is smart not to do it twice.
wp_enqueue_style( $slug );

$style_data = $asset->get_style_data();
foreach ( $style_data as $key => $value ) {
wp_style_add_data( $slug, $key, $value );
}
}
$asset->do_print();

if ( ! empty( $asset->get_after_enqueue() ) && is_callable( $asset->get_after_enqueue() ) ) {
call_user_func_array( $asset->get_after_enqueue(), [ $asset ] );
Expand Down Expand Up @@ -713,49 +693,7 @@ public function register_in_wp( $assets = null ) {
continue;
}

$asset_slug = $asset->get_slug();

if ( 'js' === $asset->get_type() ) {
// Script is already registered.
if ( wp_script_is( $asset_slug, 'registered' ) ) {
continue;
}

wp_register_script( $asset_slug, $asset->get_url(), $asset->get_dependencies(), $asset->get_version(), $asset->is_in_footer() );

// Register that this asset is actually registered on the WP methods.
// @phpstan-ignore-next-line
if ( wp_script_is( $asset_slug, 'registered' ) ) {
$asset->set_as_registered();
}

if (
! empty( $asset->get_translation_path() )
&& ! empty( $asset->get_textdomain() )
) {
wp_set_script_translations( $asset_slug, $asset->get_textdomain(), $asset->get_translation_path() );
}
} else {
// Style is already registered.
if ( wp_style_is( $asset_slug, 'registered' ) ) {
continue;
}

wp_register_style( $asset_slug, $asset->get_url(), $asset->get_dependencies(), $asset->get_version(), $asset->get_media() );

// Register that this asset is actually registered on the WP methods.
// @phpstan-ignore-next-line
if ( wp_style_is( $asset_slug, 'registered' ) ) {
$asset->set_as_registered();
}

$style_data = $asset->get_style_data();
if ( $style_data ) {
foreach ( $style_data as $datum_key => $datum_value ) {
wp_style_add_data( $asset_slug, $datum_key, $datum_value );
}
}
}
$asset->do_register();

// If we don't have an action we don't even register the action to enqueue.
if ( empty( $asset->get_action() ) ) {
Expand Down Expand Up @@ -788,16 +726,14 @@ public function remove( $slug ) {
return false;
}

$type = $this->get( $slug )->get_type();
$asset = $this->get( $slug );

if ( $type === 'css' ) {
wp_dequeue_style( $slug );
wp_deregister_style( $slug );
} else {
wp_dequeue_script( $slug );
wp_deregister_script( $slug );
if ( ! $asset instanceof Asset ) {
return true;
}

$asset->dequeue_asset()->deregister_asset();

unset( $this->assets[ $slug ] );

return true;
Expand Down Expand Up @@ -836,9 +772,8 @@ public function print_group( $group, $echo = true ) {
if ( $asset->is_registered() ) {
continue;
}
'js' === $asset->get_type()
? wp_register_script( $slug, $asset->get_file(), $asset->get_dependencies(), $asset->get_version() )
: wp_register_style( $slug, $asset->get_file(), $asset->get_dependencies(), $asset->get_version() );

$asset->register_asset( $asset->get_file(), $asset->get_dependencies(), $asset->get_version() );
}

ob_start();
Expand Down
Loading
Loading