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 7.4 support #41

Closed
oprypkhantc opened this issue Feb 4, 2020 · 6 comments
Closed

PHP 7.4 support #41

oprypkhantc opened this issue Feb 4, 2020 · 6 comments

Comments

@oprypkhantc
Copy link
Contributor

Hey there.

In php 7.4, using array access syntax on scalars/null now emits a notice: https://github.com/Dwolla/dwolla-swagger-php/blob/master/lib/models/Customer.php#L99

I've seen Dwolla/dwolla-openapi#1 and it was able to generate the SDK using it. Is there any chance of this being updated to use it?

@spencerhunter
Copy link
Member

Hi @oprypkhantc , There are plans to update the existing dwolla-swagger-php SDK as well as investigating writing a new one from scratch. The existing dwolla-swagger-php SDK was generated using the swagger 2.0.0 spec and came with a host of issues. Updating to openAPI 3.x would likely require updates to the swagger codegen project to work with the Dwolla API. If we decide to go down that path, we'll likely do a major version bump to this SDK.

@oprypkhantc
Copy link
Contributor Author

As I said, I was able to generate a complete Dwolla API using upstream swagger-codegen and new 3.x spec, so swagger-codegen's fork shouldn't be needed anymore.

We're using a fork of this for now. Thank you for the reply.

@acirinelli
Copy link

Does someone have a solution for this? I have other packages that require 7.4, so I am stuck between keeping 7.3 just for Dwolla, or moving to 7.4 for everyone else.

@incentfit
Copy link

We have this issue as well. We actually already moved to 7.4 and we're now getting a lot of notices being thrown. Surprisingly, our transactions seem to still be going through okay. I've included a screenshot of what we're seeing when doing a basic transaction:
image

@incentfit
Copy link

I took a quick look through your source code and I believe I've isolated the problem. Throughout your library you guys use syntax like this for function declerations:
public function __construct(array $data = null)
The problem is that you're declaring a variable $data as an array but then assigning it a null value (which is not an array). PHP 7.4 adds support for typed properties so what you need to do here is one of two things:

  1. Change the default value from null to an empty array [](my recommendation)
  2. Change the array type to ?array which tells PHP that a null value is okay
  3. Just get rid of the array type entirely and let PHP handle figuring out the type for you

@oprypkhantc
Copy link
Contributor Author

Just use our fork (or fork from it): https://github.com/tenantcloud/dwolla-swagger-php/tree/php-74

"require": {
    "dwolla/dwollaswagger": "^1.100",
},
"repositories": [
    {
      "type": "git",
      "url": "https://github.com/tenantcloud/dwolla-swagger-php"
    }
  ],

We've been using it in production for almost two months now; so far so good.

@oprypkhantc oprypkhantc closed this as not planned Won't fix, can't repro, duplicate, stale Mar 25, 2024
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

No branches or pull requests

4 participants