gocardless / gocardless-pro-php

GoCardless Pro PHP Client
MIT License
97 stars 40 forks source link

Creation of dynamic property deprecation warnings on PHP 8.2 #155

Open hjheath opened 1 year ago

hjheath commented 1 year ago

This library seems to be hitting this deprecation warning: https://php.watch/versions/8.2/dynamic-properties-deprecated

Steps to repro:

Results:

PHP Deprecated:  Creation of dynamic property GoCardlessPro\Core\ApiClient::$http_client is deprecated in /Users/hheath/code/php_sandbox/vendor/gocardless/gocardless-pro/lib/Core/ApiClient.php on line 24

Deprecated: Creation of dynamic property GoCardlessPro\Core\ApiClient::$http_client is deprecated in /Users/hheath/code/php_sandbox/vendor/gocardless/gocardless-pro/lib/Core/ApiClient.php on line 24
PHP Deprecated:  Creation of dynamic property GoCardlessPro\Core\ApiClient::$error_on_idempotency_conflict is deprecated in /Users/hheath/code/php_sandbox/vendor/gocardless/gocardless-pro/lib/Core/ApiClient.php on line 25

Deprecated: Creation of dynamic property GoCardlessPro\Core\ApiClient::$error_on_idempotency_conflict is deprecated in /Users/hheath/code/php_sandbox/vendor/gocardless/gocardless-pro/lib/Core/ApiClient.php on line 25
Nimisoere commented 1 year ago

Thanks Henry,

We're looking to update the PHP library to support v8+. We'll update this ticket once this is done

davidearl commented 1 year ago

I also see deprecation warnings for PHP8.2 which I am running in a development version specifically in order to catch these in my code prior to Debian Bookwork release which is now only a couple of months away which will come with PHP8.2 as standard. (8.2 is already a quarter of the way through as current version of PHP, and all version 7s are now well past their end-of-life dates).

In particular, the assignment to customer_bank_accounts in Client.php in method customerBankAccounts raises a deprecation notice..

The biggest change in my code, and probably will be in yours too, is the requirement for all class properties on non-anonymous classes to be explicitly declared (or an exception marked for the whole class). Though they are "only" warnings at this stage, I catch all PHP notices and send myself an alert, so it is a nuisance to keep getting notifications about third-party code that I can't do much about.

Anyway, just putting this here so I hear from GitHub when you update this ticket.

dpslwk commented 1 year ago

any movement on this? php 8.2 has been out for 9 months, 8.1 leaves active support in 2 months and 8.3 is already on RC2

davidearl commented 1 year ago

Debian 12 Bookworm is now out, where PHP8.2 is the standard install. PHP8.1 drops out of active support in only 1 month.

This problem is starting to become urgent.

edouard-protected-eu commented 1 year ago

A temporary solution is to use the attached patch with the package cweagans/composer-patches :

{
    "name": "acme/demo",
    "version": "1.0.0",
    "require": {
        "gocardless/gocardless-pro": "^5.3.0",
        "cweagans/composer-patches": "^1.7"
    },
    "config": {
        "allow-plugins": {
            "cweagans/composer-patches": true
        }
    },
    "extra": {
        "patches": {
            "gocardless/gocardless-pro": {
                "Fix issue gocardless/gocardless-pro #155": "./gocardless-pro-php-issue-155.patch"
            }
        },
        "composer-exit-on-patch-failure": true
    }
}

By adding this to your composer.json, it will automatically patch gocardless-pro-php to remove the warnings

gocardless-pro-php-issue-155.patch

colincameron commented 11 months ago

PHP 8.1 is now out of active support and 8.3 has been released. Is there any update on this issue?

davedevelopment commented 4 months ago

Is this likely to be fixed any time soon? We're at a point where we need to fork and maintain our own version or make infrastructure changes to ignore logs from these deprecations.

tomroyal commented 4 months ago

I contacted GoCardless support yesterday, received only the following:

The Engineering team is aware of this issue and has it on their to-do list. Sadly, we are not able to confirm the timeline for when this will be resolved.

¯_(ツ)_/¯