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

Php 8.2 Deprecation Warning Fixes #387

Merged

Conversation

anktd
Copy link
Collaborator

@anktd anktd commented Dec 4, 2024

Recently a bug was reported about deprecated warning on installing the Blockonomics plugin -> Ticket here.

Fix: Changed the parent slug in add_submenu_page() from empty string to 'options.php' to resolve PHP 8.1 deprecation warnings while maintaining hidden menu functionality ➞ f7784a3

  • Before this, we were using null:
    • Using it caused PHP 8.1 deprecation warnings in WordPress core functions
    • Specifically triggers warnings in:
      • strpos(): When WordPress checks menu hierarchy
      • str_replace(): During menu slug processing
    • This occurs because WordPress core functions expect string types but receive null
  • Using empty string ('') (as the suggested fix by user report):
    • Resolves the initial strpos/str_replace warnings
    • However, triggers new deprecation warning in strip_tags()
    • This happens because WordPress internally converts empty string to null during menu processing
    • The conversion occurs before the string reaches admin-header.php
  • Using 'options.php':
    • References WordPress core options handling system
    • Maintains proper string type throughout WordPress menu processing
    • Prevents type coercion issues that lead to deprecation warnings
    • Standard WordPress pattern for hidden admin pages
    • Doesn't create visible menu items while ensuring proper page registration

Other fixes in this PR:

  • Remove incorrect wpdb::prepare usage in uninstall hook by removing unnecessary prepare call for static queries ➞ 20fa454

  • Added explicit property declaration private $api_key; to the Blockonomics_Setup class to comply with PHP 8.2's deprecation of dynamic properties ➞ 573320a

@anktd
Copy link
Collaborator Author

anktd commented Dec 4, 2024

Tested changes working with Php 8.2.0, 8.1.13 and 7.4.33 ✅

@anktd anktd requested a review from ashthecoder05 December 4, 2024 07:38
Copy link
Collaborator

@ashthecoder05 ashthecoder05 left a comment

Choose a reason for hiding this comment

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

Looks good! Added some clarifying questions for my understanding 🎖️

php/Blockonomics.php Outdated Show resolved Hide resolved
blockonomics-woocommerce.php Show resolved Hide resolved
php/Blockonomics.php Outdated Show resolved Hide resolved
@anktd anktd requested a review from ashthecoder05 December 4, 2024 10:08
@shivaenigma shivaenigma merged commit 7b08dee into blockonomics:master Dec 4, 2024
1 check passed
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