spawnia / sailor

A typesafe GraphQL client for PHP
MIT License
78 stars 18 forks source link

Update package dependencies and only allow installs on at least PHP 8.x #89

Closed morloderex closed 1 year ago

morloderex commented 1 year ago

Changes

This Pull Request upgrades the package to use the latest versions of its dependencies. Furthermore it also now requires PHP ^8.0.x.

Lastly i have changed the return types and the method expectations as Nette/Generator v4.0 have splitted out the ClassType object into multiple independent classes.

I found that when composer installed Nette/Generator at version 4.x that i was very hard to implement native PHP enums as it required me basically reinvent everything myself due to PHP's type-system rules and the new classes that Nette/Generator introduced in version 4.0.

Breaking changes

spawnia commented 1 year ago

I plan to support PHP 7.4 for quite some time, probably a couple of years. If you have valuable changes besides the PHP bump, please extract.

morloderex commented 1 year ago

@spawnia the thing is that Nette/Generator is quite weird.

The version difference between 3.x and 4.x is quite large.

And when using PHP ~8.0 version 4 is pulled down and due to the split of EnumType and ClassType and PHP,s type system it seems impossible to generate PHP native enums.

Maybe we should not allow Nette/Generator v4.x and stick with version 3 if you want to keep support for PHP 7.4?

simPod commented 1 year ago

Not allowing v4 will prevent consumers from upgrading other libraries using it. That is not the way.

morloderex commented 1 year ago

Not allowing v4 will prevent consumers from upgrading other libraries using it. That is not the way.

@simPod @spawnia But the issue here is that code between the two versions of nette/php-generator are not compatible with each other at all. Sense in v3 you would use the ClassType object and the old code was typehinted as to only allow a ClassType. In V4 the ClassType object has been widen to EnumType, ClassType, TraitType and InterfaceType. objects all extending a ObjectLike class.

The properties and methods which used to be on ClassType in version v3 has been moved to the other classes which make it impossible to generate Native PHP enums with this library by using V4 of nette/php-generator sense the methods on ClassType on order to do so (which we typehind to) is not present anymore in v4.

@spawnia Due to these issues with nette/php-generator v4 i think only allowing v3 for the time being is still the way to go sense you want to support PHP 7.4?

spawnia commented 1 year ago

I propose the following:

morloderex commented 1 year ago

@spawnia Alright i will close this pr and open up another one with your suggested changes.

CRC-Mismatch commented 1 year ago

I propose the following:

* widen or remove type hints where v3 and v4 conflict

* dynamically recognize which version is used, e.g. by the given class or presence of a certain methods and act accordingly

* add a config option to use native enums over the current string constants

Would you be willing to accept a proposal using only PHP-Parser instead? Although it's really lower-level than Nette's generator, it doesn't seem too hard to use, and also seems to keep up with latest PHP + compatible with PHP 7.4 with no "weirdery" :shrug: (probably PHPStan will have its complaints about that...)

spawnia commented 1 year ago

Yeah, I would be fine with switching to PHP-Parser. I have built custom Rector rules with it that did a decent amount of code generation, it was not too hard indeed.