Skip to content

Prevent assets from being added to assets array twice#31

Merged
borkweb merged 3 commits intomainfrom
fix/prevent-duplicate-asset-entries
Jan 19, 2025
Merged

Prevent assets from being added to assets array twice#31
borkweb merged 3 commits intomainfrom
fix/prevent-duplicate-asset-entries

Conversation

@borkweb
Copy link
Contributor

@borkweb borkweb commented Jan 19, 2025

When adding assets, an asset would be added to the Assets::$assets array with the slug as the index and with a numeric index. There's no reason for the numeric index, as it just causes extra looping and attempts to enqueue.

Before:

[
  'my-style'  => Asset /* my-style */,
  0           => Asset /* my-style */,
  'my-script' => Asset /* my-script */,
  1           => Asset /* my-script */,
]

After

[
  'my-style'  => Asset /* my-style */,
  'my-script' => Asset /* my-script */,
]

When adding assets, an asset would be added to the `Assets::$assets` array with the slug as the index and with a numeric index. There's no reason for the numeric index, as it just causes extra looping and attempts to enqueue.

WP must be outputting script tags with single quotes now. Updated tests to reflect that.
@borkweb borkweb requested review from bordoni and dpanta94 January 19, 2025 21:46
@borkweb borkweb merged commit c6787fc into main Jan 19, 2025
3 checks passed
@borkweb borkweb deleted the fix/prevent-duplicate-asset-entries branch January 19, 2025 21:52
Comment on lines +696 to +700
if ( ! $assets instanceof Asset ) {
throw new \InvalidArgumentException( 'Assets in register_in_wp() must be of type Asset' );
}

$assets = [ $assets->get_slug() => $assets ];
Copy link
Member

Choose a reason for hiding this comment

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

What the check here is doing doesn't seem enough to be sure about what we want to achieve. We might pass an invalid array and the error would still be there and it could be even more serious.

It should be instead:

if ( ! is_array( $assets ) {
    $assets = [ $assets ];
}

foreach ( $assets as $asset {
    if ( ! $asset instanceof Asset ) {
        throw
    }
    
    $this->assets[ $asset->get_slug() ] = $asset;
}

return;

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants