Opened 40 hours ago
Last modified 8 hours ago
#54504 reopened task (blessed)
Update Requests library to version 2.0.0
Reported by: | jrf | Owned by: | SergeyBiryukov |
---|---|---|---|
Milestone: | 5.9 | Priority: | normal |
Severity: | normal | Version: | |
Component: | External Libraries | Keywords: | has-patch needs-dev-note php80 php81 |
Focuses: | Cc: |
Description
The Requests library has released a new major version: 2.0.0.
This is a major release and contains breaking changes.
Most important changes to be aware of for this release:
- All code is now namespaced, though there is a full BC-layer available and the old class names are still supported, though using them will generate a deprecation notice (which can be silenced by plugins if they'd need to support multiple WP versions). An upgrade guide is available and I'd recommend for this change + a link to the upgrade guide to be included in the WP 5.9 dev-note.
- A lot of classes have been marked
final
. This should generally not affect userland code as care has been taken to not apply thefinal
keyword to classes which are known to be extended in userland code. - Extensive input validation has been added to Requests. When Requests is used as documented though, this will be unnoticable.
- A new
WpOrg\Requests\Requests::has_capabilities()
method has been introduced which can be used to address #37708 - A new
WpOrg\Requests\Response::decode_body()
method has been introduced which may be usable to simplify some of the WP native wrapper code. - Remaining PHP 8.0 compatibility fixed (support for named parameters)
- PHP 8.1 compatibility
Full changelog: https://github.com/WordPress/Requests/releases/tag/v2.0.0
Website (updated): https://requests.ryanmccue.info/
It is recommended for WordPress to update the bundled version of Requests.
I've prepared a PR for the update and will link it to this ticket.
Change History (15)
This ticket was mentioned in PR #1624 on WordPress/wordpress-develop by jrfnl.
40 hours ago
- Keywords has-unit-tests added
#3
@
40 hours ago
- Keywords has-unit-tests removed
GH PR is ready for review and if found okay, for commit: https://github.com/WordPress/wordpress-develop/pull/1624
This ticket was mentioned in Slack in #core by sergey. View the logs.
35 hours ago
#8
follow-up:
↓ 9
@
29 hours ago
Can we consider renaming the namespace from WpOrg
to something more generic, such as WordPress
or WP
?
That appears to be the first WordPress-specific namespace used within WordPress, and setting the name as WpOrg
seems like it might be rather ugly later on.
The inclusion of Org
is also kind of close to the namespace used on WordPress.org of WordPressdotorg
.. So if this was intended on being "Not WordPress, but a WordPress.org thing" then that longer format may have been more appropriate, I'm not sure.
I realise this has been something that has been decided "upstream" in Requests, but raising this here since it's where the merge has happened.
#9
in reply to:
↑ 8
@
29 hours ago
- Resolution fixed deleted
- Status changed from closed to reopened
Replying to dd32:
Can we consider renaming the namespace from
WpOrg
to something more generic, such asWordPress
orWP
?
That appears to be the first WordPress-specific namespace used within WordPress, and setting the name as
WpOrg
seems like it might be rather ugly later on.
Thanks for bringing this up! That was my concern here too, but I didn't want it to block the merge.
The inclusion of
Org
is also kind of close to the namespace used on WordPress.org ofWordPressdotorg
.. So if this was intended on being "Not WordPress, but a WordPress.org thing" then that longer format may have been more appropriate, I'm not sure.
I realise this has been something that has been decided "upstream" in Requests, but raising this here since it's where the merge has happened.
For reference, it appears to be implemented in this PR: Requests 2.0.0: introduce namespaces:
Acronyms are "proper cased", i.e. first character uppercase, the rest of the acronym lowercase.
See this comment specifically for more details.
Either WordPressdotorg
or WPorg
, or simply WordPress
or WP
would be my preference too, though it slightly deviates from the "proper cased" rule used for other acronyms. Could it perhaps be an exception?
Reopening for discussion.
#11
@
28 hours ago
Using something like WordPress
makes sense to me too. As it isn't an acronym, it should be fine to use CamelCaps.
WpOrg
(lowercase or uppercase) is also a commonly used abbreviation for wordpress.org
.
It's possible that WpOrg
was used in an effort to say "This is created under the WordPress Organisation on GitHub". In this case, using WordPress
may incorrectly imply that WordPress is a dependency for Requests.
It would be good to get some insight into the reason(s) why WpOrg
was chosen or, more specifically, why WordPress
wasn't chosen. It's quite possible that we're unaware of sound reasoning for it that we should consider when discussing an alternative namespace.
#12
follow-up:
↓ 13
@
19 hours ago
@dd32 I appreciate the concern, but Requests 2.0.0 has been released and the discussion about the namespace was had publicly in that repo, in open videocalls and in WP Slack months ago.
Input at the time would have been very welcome, but was not received and it's too late to change this now.
The choice for the namespace was discussed extensively and various options were considered during those discussions.
We originally were going to use just Requests\
as the "prefix", but that was causing problems with the BC-layer. In our opinion, including the BC-layer was imperative to allow for plugins supporting multiple versions of WP and using parts of Requests directly, so that meant we had to reconsider the namespace.
We considered using WordPress\Requests\
, but as Requests is a stand-alone project, we did not want to create a situation were the namespace chosen by Requests could ever become a blocker for WordPress itself adopting namespaces at some point in the future.
We also considered that Requests is used by more projects than just WordPress and that the namespace prefix should not cause undue friction in other projects using Requests, so we ended up compromising on WpOrg\Requests\
.
While it is, of course, perfectly possible to change it for the copied version included in WP, this would make all future merges of Requests a lot more complex, so I'd strongly recommend against that.
#13
in reply to:
↑ 12
@
16 hours ago
Replying to jrf:
We also considered that Requests is used by more projects than just WordPress and that the namespace prefix should not cause undue friction in other projects using Requests, so we ended up compromising on
WpOrg\Requests\
.
Thanks for the context! Since PHP namespaces are case-insensitive, is there any chance to capitalize that as WPorg\Requests\
, so that the capitalization of WP
is consistent in lines like this and across the project in general?
new WP_REST_Request( WPorg\Requests\Requests::POST, $route );
#14
@
15 hours ago
Since PHP namespaces are case-insensitive, is there any chance to capitalize that as
WPorg\Requests\
, so that the capitalization of WP is consistent in lines like this and across the project in general?
@SergeyBiryukov Good question. We did actually discuss that as well.
For the PSR-4 class name to path translation in the autoloaders (both the supported Composer autoloader as well as the custom autoloader), the namespace and class names have to be treated case-sensitively as otherwise this would break when code is run on case-sensitive OS-es, like *nix and MacOS.
To then treat the namespace prefix case-insensitively would be inconsistent, especially as this could only be supported when using the custom autoloader, but would break when using the Composer autoloader, which is why it was decided to not support that.
### External libraries: upgrade to Requests 2.0.0
### Upgrade references to the Requests library in the WP code
Trac ticket: https://core.trac.wordpress.org/ticket/54504