search

facebook / facebook-php-business-sdk

PHP SDK for Meta Marketing API
https://developers.facebook.com/docs/business-sdk
Other
837 stars 519 forks source link

Deprecations with `strlen` on null. #565

Closed przepompownia closed 2 years ago

przepompownia commented 2 years ago

I'd like only to upgrade this library to work with PHP 8.1 (it works with 8.0, other person can verify it on the FB side). I don't want to create any personal FB account to resolve the below problem.

In https://github.com/facebook/facebook-php-business-sdk/blob/main/src/FacebookAds/Object/ServerSide/CustomData.php#L462 normalize() fails on null values. To reproduce, in https://github.com/facebook/facebook-php-business-sdk/blob/main/src/FacebookAds/Object/ServerSide/CustomData.php#L462 set some value to null and observe deprecation warnings.

PHP Deprecated:  strlen(): Passing null to parameter #1 ($string) of type string is deprecated in .../facebook-php-business-sdk/src/FacebookAds/Object/ServerSide/CustomData.php on line 462

It matters on environments where such errors are converted to exceptions.

natewiebe13 commented 2 years ago

I had opened https://github.com/facebook/facebook-php-business-sdk/pull/554 but was directed to the codegen repo to resolve them, but not all of the classes existed there. Since then, 8.1 updates have been merged in, so I'll take another stab at seeing if we can get the remaining instances merged in.

Otherwise, we're also going to need https://github.com/facebook/facebook-business-sdk-codegen/pull/50 merged and imported into this repo as well.

stcheng commented 2 years ago

@natewiebe13 thanks for addressing this issue. the corresponding PRs will be merged after this thanksgiving holiday.

przepompownia commented 2 years ago

Thank you @natewiebe13, @stcheng What do you think about tag this commit?

natewiebe13 commented 2 years ago

There were also deprecations fixed in the codegen repo. If we get a new tag before v16, hopefully the codegen changes get imported for that tag as well.