-
Notifications
You must be signed in to change notification settings - Fork 845
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-webdriver 2.0 - RFC 💬 #657
Comments
Thanks for this @OndraM , I agree that we should now drop the JSONWire support. Things have progressed rapidly since the last patch for this. Personally I'd prefer to see us keep PHP7.1 support for a couple of reasons:
I'll review the patch at the end of this week. Thanks |
I guess JSONWire and PHP version support drops are fine. Getting in touch with some of the bigger dependents might be useful... |
It would be better if PHP 7.1 was supported. |
Regarding Codeception I pinged @DavertMik. |
Those changes are ok for Panther! One more thing that could be nice: we had to stop shipping Panther with the default Symfony installation because PHP WebDriver depends of Using Symfony HttpClient will allow the library to be used on all installations, even if CURL is missing (in this case Symfony HttpClient automatically switches to an implementation built on top of |
All good for Laravel Dusk. Not sure if @staudenmeir has any additional concerns? PHP 7.1 is EOL in three months so it should be dropped. It only encourages people to use EOL versions of PHP when you keep on supporting them. We're dropping support for it in Laravel 6 which is due in a few weeks. The biggest issue we've had lately with Laravel Dusk is version parity for the chrome driver with the Chrome browser: laravel/dusk#641 |
Actually, Symfony 4 (LTS) will still support PHP 7.1 until 2023 (because RedHat/Ubuntu/Debian will still support it for a few years too). For PHP WebDriver to be shipped with Symfony 4.4 LTS (to be released in November), it has to support PHP 7.1. (Symfony 5, to be released in November too, will require PHP 7.2). |
I don't see any issues for Dusk either. |
it would be great if some W3C support could be merged in 1.x first (with #560 for instance). This way, supporting W3C drivers (needed for modern browsers) would be independent from migrating to a new major version of the library (which could have BC breaks and require work). |
I'm working on finishing #560. It's a real blocker for Panther (and most other libs I guess). While there is no ETA for v2, it would be great if we can merge it ASAP. |
I agree to make 2.0 with the only W3C protocol. However, bringing such support without feature cut could be a tricky issue. This really depends on how strict implementation of W3C protocol will be. For instance, there is no So, after all, I agree with @stof. To start moving completely to W3C we need to introduce W3C into 1.x branch, receive feedback from users, and then move to 2.0 dropping JSONWire support and cleaning things up with a better understanding of edge cases. Asking users to partially rewrite their code to WebDriver support with no extra benefits but only limitations can be a problem. So, migrating to php-webdrvier v2 should happen only when everyone is assured that this migration will go smoothly. Also, just to note, we are not time-limited by new Selenium 4, it will continue supporting JSONWire protocol. So let's move slow, but break things :) |
Ok, thanks everyone for comments and ideas. So the plan I suggest is:
What do you think, is this viable for everyone? |
@dunglas About But I agree we should think about the |
Awesome :)
I agree 💯
Exactly
My main goal was to be able to use Panther with Geckodriver (which only supports the W3C flavor). With #560 the whole test suites of both Panther and PHP Webdriver are green with Geckodriver, without Selenoum (I added this case to the Travis matrix). So the implementation maybe don't cover some parts of the W3C protocol, but most of it should be covered. I agree with the the process you describe. Regarding http-client, it will be marked as stable in Symfony 4.4 (to be released next month). Using it in 2.0 only looks like the best solution, indeed! |
For those using php-webdriver with Mink, I have written a new Mink Driver for this. The SilverStripe driver had a number of bugs which were unaddressed. https://packagist.org/packages/andrewnicols/mink-facebook-web-driver I'm currently only focusing on W3C compliance with the aim to drop support for JSONWire at the same time as this |
As part of 2.0, we should also change This will however mean that everyone using php-webdriver will have to update all file imports... But I think we can prepare recipe for rector from @TomasVotruba, which will change the namespace (and possibly update also other BC breakes), so that the update to php-webdriver 2.0 could be almost automatic without need of manual work. |
This is request for comments about the next major version of php-webdriver (2.0). Once we understand what we want/need to do, we can divide the work among interested volunteers and get it rollin'! 🚀
Suggested objectives
Support only W3C WebDriver
Support only the new protocol and drop support of JsonWire protocol. This will allow us to focus our limited resources to make long-term working implementation of PHP language bindings.
In my opinion, maintaining compatibility with the old protocol is not effective, as the situation has developed rapidly since issue #469 was originally written:
So I don't see an actual need for the JsonWire protocol.
Drop PHP 5.6 and 7.0 support
Those version of PHP are no longer supported. We may also drop PHP 7.1 support, as it is in security-support mode (and only for few next months).With PHP 7.1+ or 7.2+ we can use many of its new features, most importantly types in method parameters and return values.If someone cannot use those new versions of PHP, he is probably working in rigid environment and maybe don't use the latest version of browsers - so one can still use the 1.x version of php-webdriver. Or has to upgrade 🤷♂️ .EDIT: this was already done in php-webdriver 1.x.
Clean object dependencies
Historically, this library object model is based on interfaces, which often breaks LSP. It also causes headaches for static analysis. This could be taken care of in the new version.
Other suggestions?
Are there any other issues we should solve?
Any missing features (see Java version changelog)?
Or something which should be done together with BC break?
Please comment!
The text was updated successfully, but these errors were encountered: