Problem/Motivation

There's only one usage in core and very few in contrib which could be replaced with $url->access()

Steps to reproduce

- http://codcontrib.hank.vps-private.net/search?text=renderAccess&filename= shows usage of both methods in contrib code

Proposed resolution

- Deprecate Url::toRenderArray() and Url::renderAccess()
- File follow-up to remove both in 11.0 #3343153: Remove deprecated code from \Drupal\Core\Url

Remaining tasks

- change record
- patch and test
- review/commit

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#14 3342975-14.patch4.01 KBquietone
#14 interdiff-10-14.txt3.61 KBquietone
#10 3342975-10.patch4.07 KBandypost
#10 interdiff.txt3.72 KBandypost
#8 3342975-7.patch4.08 KBandypost
#8 interdiff.txt1.49 KBandypost
#6 3342975-6.patch4.07 KBandypost
#6 interdiff.txt2.07 KBandypost
#4 3342975-4.patch2.84 KBandypost
#4 interdiff.txt656 bytesandypost
#2 3342975-2.patch2.81 KBandypost

Comments

andypost created an issue. See original summary.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Active » Needs review
Issue tags: +Needs change record updates
StatusFileSize
new 3342975-2.patch2.81 KB

CR and initial patch, as there's contrib usage CR needs to be updated for Url::renderAccess() (marked @internal ATM)

andypost’s picture

andypost
he/him
Primary language Russian
commented 
+++ b/core/modules/shortcut/src/Form/SetCustomize.php
@@ -55,7 +55,9 @@ public function form(array $form, FormStateInterface $form_state) {
-      ] + $url->toRenderArray();
+        '#url' => $url,
+        '#options' => $url->getOptions(),

There's access check few lines above so no need for $render_array['#access_callback'] = [get_class(), 'renderAccess'];

andypost’s picture

andypost
he/him
Primary language Russian
commented
StatusFileSize
new interdiff.txt656 bytes
new 3342975-4.patch2.84 KB

and now unset() no longer needed

catch’s picture

catch
he/him
Primary language English
commented

I checked the contrib usages, they are all doing:

if ($url->renderAccess($url->toRenderArray()))

Which seems like an extremely long way to do:

if ($url->access())

There are only two modules with a couple of usages each, so I think we could go ahead and deprecate both methods here.

andypost’s picture

andypost
he/him
Primary language Russian
commented
StatusFileSize
new interdiff.txt2.07 KB
new 3342975-6.patch4.07 KB

Updated CR https://www.drupal.org/node/3342977

and new patch with todo link for removal #3343153: Remove deprecated code from \Drupal\Core\Url

andypost’s picture

andypost
he/him
Primary language Russian
commented
Title: Deprecate Url::toRenderArray » Deporecate Url::toRenderArray() and Url::renderAccess()

better title

andypost’s picture

andypost
he/him
Primary language Russian
commented
StatusFileSize
new interdiff.txt1.49 KB
new 3342975-7.patch4.08 KB

Fix CS for deprecation

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs review » Needs work 
  1. +++ b/core/lib/Drupal/Core/Url.php
@@ -770,8 +770,14 @@ public function toString($collect_bubbleable_metadata = FALSE) {
+    @trigger_error('The ' . __METHOD__ . '() is deprecated in drupal:10.1.0 and is removed from drupal:11.0.0. Use to inline its implementation. See https://www.drupal.org/node/3342977', E_USER_DEPRECATED);

    Maybe 'There is no direct replacement' instead of 'inline its implementation? - The uses in contrib should be refactored to something else entirely. 

  2. +++ b/core/lib/Drupal/Core/Url.php
@@ -836,8 +842,14 @@ public function access(AccountInterface $account = NULL, $return_as_object = FAL
   public static function renderAccess(array $element) {
+    @trigger_error('The ' . __METHOD__ . '() is deprecated in drupal:10.1.0 and is removed from drupal:11.0.0. Use to inline its implementation. See https://www.drupal.org/node/3342977', E_USER_DEPRECATED);
     return $element['#url']->access();

    Same here.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs work » Needs review
Related issues: +#3342991: Deprecate LinkGeneratorInterface::generateFromLink()
StatusFileSize
new interdiff.txt3.72 KB
new 3342975-10.patch4.07 KB

Thanks, changed to suggested

andypost’s picture

andypost
he/him
Primary language Russian
commented
Issue summary: View changes
Issue tags: -Needs change record updates

CR updated

catch’s picture

catch
he/him
Primary language English
commented
Title: Deporecate Url::toRenderArray() and Url::renderAccess() » Deprecate Url::toRenderArray() and Url::renderAccess()
Status: Needs review » Reviewed & tested by the community

The example in the CR is clear and covers literally every usage in contrib.

I think this is ready to go now.

rohan-sinha’s picture

rohan-sinha commented

Reviewed Patch on #10, issue has been resolved as observed, can be merged.

quietone’s picture

quietone commented
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new interdiff-10-14.txt3.61 KB
new 3342975-14.patch4.01 KB

The deprecation message for a method does not start with 'The'. And I was getting a phpcs errors. So, I updated the patch.

I also too the liberty of removing 'direct' from the deprecation message. It did not seem to be adding information.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs review » Reviewed & tested by the community

Thank you, curious why phpcs does not report it in CI

  • longwave committed 95bae2b6 on 10.1.x 
    Issue #3342975 by andypost, quietone, catch: Deprecate Url::...
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 10.1.x, thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

pcambra’s picture

pcambra
he/him
Primary language Spanish
commented

I know I'm late to the party, but why is toRenderArray deprecated? I only see the access method discussed.