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

Deprecations with strlen on null. #565

Closed
przepompownia opened this issue Nov 21, 2022 · 4 comments
Closed

Deprecations with strlen on null. #565

przepompownia opened this issue Nov 21, 2022 · 4 comments

Comments

@przepompownia
Copy link

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
Copy link
Contributor

I had opened #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 facebook/facebook-business-sdk-codegen#50 merged and imported into this repo as well.

@stcheng
Copy link
Contributor

stcheng commented Nov 23, 2022

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

@przepompownia
Copy link
Author

przepompownia commented Nov 29, 2022

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

@natewiebe13
Copy link
Contributor

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.

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 a pull request may close this issue.

3 participants