Skip to content

Conversation

@Luc45
Copy link
Member

@Luc45 Luc45 commented Jun 30, 2022

How to test the changes in this Pull Request:

  1. Clone this repository
  2. Run composer install (Bonus point if your local PHP version is lower than PHP 8)
  3. Copy this repository in wp-content/plugins of a WordPress site that has WooCommerce installed.
  4. Run generate actions such as wp wc generate products 10

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully run tests with your changes locally?

Changelog entry

Lower required PHP version from PHP 8 to PHP 7.1.

FOR PR REVIEWER ONLY:

  • I have reviewed that everything is sanitized/escaped appropriately for any SQL or XSS injection possibilities. I made sure Linting is not ignored or disabled.

@Luc45 Luc45 self-assigned this Jun 30, 2022
@Luc45 Luc45 marked this pull request as draft June 30, 2022 15:04
@Luc45 Luc45 marked this pull request as ready for review June 30, 2022 17:00
@Luc45 Luc45 requested review from zhongruige and removed request for zhongruige June 30, 2022 17:00
@alopezari
Copy link

alopezari commented Jul 4, 2022

Thanks for creating this PR @Luc45!

I tried to use it today for creating some products and orders and, once I activated the plugin through WP CLI, I found the following error when I navigated anywhere in my store (which uses PHP 7.4):

Fatal error: Declaration of Automattic\WooCommerce\Container::has($id) must be compatible with Psr\Container\ContainerInterface::has(string $id): bool in /srv/users/userff65ed31/apps/userff65ed31/public/wp-content/plugins/woocommerce/src/Container.php on line 106

Notice: Function is_embed was called incorrectly. Conditional query tags do not work before the query is run. Before then, they always return false. Please see [Debugging in WordPress](https://wordpress.org/support/article/debugging-in-wordpress/) for more information. (This message was added in version 3.1.0.) in /srv/users/userff65ed31/apps/userff65ed31/public/wp-includes/functions.php on line 5831

Notice: Function is_search was called incorrectly. Conditional query tags do not work before the query is run. Before then, they always return false. Please see [Debugging in WordPress](https://wordpress.org/support/article/debugging-in-wordpress/) for more information. (This message was added in version 3.1.0.) in /srv/users/userff65ed31/apps/userff65ed31/public/wp-includes/functions.php on line 5831
There has been a critical error on this website. Please check your site admin email inbox for instructions.

[Learn more about troubleshooting WordPress.](https://wordpress.org/support/article/faq-troubleshooting/)

I created another test store using the same PHP version and installed WC Smooth Generator compiled from the latest release tag, it worked properly.

Is there any chance that issue was introduced somehow by this PR?

@Luc45
Copy link
Member Author

Luc45 commented Jul 5, 2022

Nice catch @alopezari !

I have locked the psr/container version to 1.0.0, which is the same as WooCommerce's: https://github.com/woocommerce/woocommerce/blob/trunk/plugins/woocommerce/composer.json#L23

Faker, which is a dependency of this package, requires psr/container "^1.0 || ^2.0" too: https://github.com/FakerPHP/Faker/blob/main/composer.json#L18

Since WooCommerce dependency resolution tree is independent from this package, both codes might require different versions of the interface and still run under the same context. So we had psr/container 2 being loaded, while WooCommerce expects 1.

This is an annoying conflict that will always be present in WordPress ecosystem, at least until we figure out a way to keep a centralized dependency resolution process in Core, which there's no proposal for AFAIK.

This problem is solved, for now.

@alopezari
Copy link

Thanks, @Luc45! I've used it now and worked great 👍

@jonathansadowski jonathansadowski requested review from a team and jonathansadowski and removed request for a team September 23, 2022 16:46
@jonathansadowski jonathansadowski merged commit 7e9ff1e into trunk Sep 26, 2022
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