markhuot / craftql

A drop-in GraphQL server for Craft CMS
Other
319 stars 53 forks source link

Upgrade graphql-php version #236

Open rodrigosaraiva opened 5 years ago

rodrigosaraiva commented 5 years ago

We are having problems with the version that you are using for the graphql-php package. Your package is using the version 0.11.0, and the new version 0.13.0 fix the following problems:

[-][-][-][error][yii\base\ErrorException:2] yii\base\ErrorException: "continue" targeting switch is equivalent to "break". Did you mean to use "continue 2"? in /var/www/html/vendor/webonyx/graphql-php/src/Validator/Rules/OverlappingFieldsCanBeMerged.php:355 [-][-][-][error][yii\base\ErrorException:2] yii\base\ErrorException: "continue" targeting switch is equivalent to "break". Did you mean to use "continue 2"? in /var/www/html/vendor/webonyx/graphql-php/src/Executor/Executor.php:532

markhuot commented 5 years ago

What version of PHP is this? I've held off on updating because the latest version of graphql-php requires PHP 7.1 which is more than CraftCMS requires (7.0). So, CraftQL is in a tough spot.

Related: I have a 2.x-dev version that I'm working on that would be the appropriate place to make this update, if I'm going to.

rodrigosaraiva commented 5 years ago

The version is 7.2.11 for PHP and Craft 3.1.15. I received this nicely answer from graphql-php: https://github.com/webonyx/graphql-php/issues/450

markhuot commented 5 years ago

Okay, thanks @rodrigosaraiva. I'm going to exploring reving the version number on the 2.x branch and see if anything breaks. I'll let you know once that's done and you should be able to try it out.

markhuot commented 5 years ago

I'm still working on this. There are a number of backwards breaks that I need to work through before this is merged. I will keep you updated in this ticket as I work through it though.

rodrigosaraiva commented 5 years ago

@markhuot Thank you Mark. We bought this plugin because we are working in a project to a bank, and it is very important to have it updated. For now we are working with manual changes on it. It is not a huge problem, but it will block us from update plugins for a while.

markhuot commented 5 years ago

@rodrigosaraiva, did you have any issues with the update? E.g., I know the Directive::FIELD class changed a bit and causes issues.

kellypacker commented 5 years ago

I'm getting this error with PHP 7.1.26, Craft 3.1.21.1 and CraftQL 1.3.1. Is there a version of PHP and Craft that will work with this version of CraftQL? Or maybe a different version of CraftQL?

PhillipRichdale commented 5 years ago

Same problem here. PHP 7.3.4, Craft 3.1.26, CraftQL 1.3.2. Very frustrating, spent the entire day trying to fix this. :-( Any updates on the issue?

PhillipRichdale commented 5 years ago

The fix is to replace all the "continue;"s with "break;"s, as said in the PHP Warnings.

zimkha commented 5 years ago

hi i have the same problem with my graphql. I have this error that prevents me from advancing on my project. Please, how to solve this error because I galley in this moment

Jones-S commented 5 years ago

It suddenly appeared on my end as well. I had it before and switching from php 7.3 to 7.2 resolved it back then.

But now I tried 7.3, 7.2, 7.1 and 7.0. Still no luck. Is there anything else I could do to resolve this?

---- Edit: I could solve it in my case by just disabling all php errors... But this is just a temporary and ugly solution.

narration-sd commented 5 years ago

In a quick look over the landscape, it appears that you may probably actually still be running PHP 7.3.

Jones-S commented 5 years ago

@narration-sd Thanks for your answer. I have indeed 7.2 running. I checked with phpinfo(). I do have error_display enabled now, and it does work. I am not sure why. Maybe it wasn't 7.2 before? 🤷‍♂

Also it is a bit weird, because I told my MAMP not to display any errors, but the phpinfo says they are still enabled. So it all seems a bit messy here. Anyway, it's back, up and running and I am happy. 👍

narration-sd commented 5 years ago

Well, good, if your happy :)

The ditch /vendor plus ditch composer.lock is a good safe bet though at any time, and could get you around some 'unusual' situations.

MAMP, though -- if it's not, or not always due to some need like machine reboot, etc., following your instructions as to which PHP to use, you'd have to deal with separately. You might talk with @jalendport about it; he's pretty sharp about things like this...

narration-sd commented 5 years ago

missed stating the third step re: composer -- it's composer install after you remove /vendor and, crucially, composer.lock...I think composer update works in that case also....

Jones-S commented 5 years ago

Yeah. I normally follow these steps as well. I think it was the first thing I did. Remove the packages, install them again. I dislike it though that I always have to add a new token and click myself through all the toggles to allow certain graphql access. I like that in the craft core graphql there is a toggle all button. 👍

narration-sd commented 5 years ago

Well, can't help on the tokens; as they say in some film, 'it is what it is'...

I would talk to @jalendport, Jalen Davenport, though - he's also quite active on Craft Discord. and you can look up his handle there. By my observation, MAMP can be tricky, and you'd like a way to tie down this PHP version thing, seem after this episode. Anything you can make more solid, more happy...!

Jones-S commented 5 years ago

Sure no worries. it is what it is. :) Thanks for the pointer.