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.

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

Update wp fixtures to use forked version for compatibility with PHP 8.1 #307

Closed
wants to merge 1 commit into from

Conversation

dansmart-box
Copy link
Contributor

There is an issue we’re facing in upgrading to PHP 8.1 with WP CLI Fixtures.

This occurs on PHP 8.1 (and now confirmed on PHP 8.0) when running wp fixtures load. The error we’re getting is:

Fatal error: Declaration of Hellonico\Fixtures\Provider\Picsum::imageUrl($width = 640, $height = 480, $filters = [],
$format = 'jpg', $unused = false, $unused_ = false) must be compatible with Faker\Provider\Image::imageUrl($width = 640,
$height = 480, $category = null, $randomize = true, $word = null, $gray = false, $format = 'png')
in /var/www/.wp-cli/packages/vendor/hellonico/wp-cli-fixtures/src/Provider/Picsum.php on line 19
Taking a look at Picsum.php, it has a different number of parameters to the parent method in Image.php

Changing Picsum.php to add the additional parameter gets it working again.

It seems the interface of the Faker\Provider\Image::imageUrl() function was changed in version 1.20.0 (https://github.com/FakerPHP/Faker/blob/main/src/Faker/Provider/Image.php) - and at the very bottom of the Faker PR someone comments that it's broken Backwards Compatiblity: (FakerPHP/Faker#473).
We're finding that it's only PHP 8.x that is reporting this compatibility error (due to increased PHP 8 type checking - see the section on Contravariance in (https://php.watch/versions/8.0/lsp-errors)).

The Faker reference is in wp-cli-fixtures' composer.json: https://github.com/nlemoine/wp-cli-fixtures/blob/main/composer.json:

"require": {
"php": "^7.3 || ^8.0",
"fakerphp/faker": "^1.13",
"nelmio/alice": "^3.8",
"wp-cli/wp-cli": "^2.4"
},

This PR updates install.sh to use our forked repository until the PR on the nlemoine/wp-cli-fixtures repo can be merged.

@anthonywnz
Copy link
Contributor

Guess we don't need this anymore @dansmart-box ?

@anthonywnz
Copy link
Contributor

Maybe worth deleting https://github.com/boxuk/wp-cli-fixtures too (if you can)?

@dansmart-box
Copy link
Contributor Author

dansmart-box commented Oct 20, 2022 via email

@dansmart-box
Copy link
Contributor Author

dansmart-box commented Oct 20, 2022 via email

@anthonywnz anthonywnz closed this Oct 25, 2022
@anthonywnz anthonywnz deleted the update_wp_fixtures_php_8_1 branch October 25, 2022 14:46
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.

2 participants