Contrib.social

There are too many ways to generate URLs and links

Open on Drupal.org →
Created on 20 May 2015, over 9 years ago
Updated 15 March 2023, almost 2 years ago

Detailed analysis of the existing usages

  • t('foo bar baz', \Drupal::url()) (e.g. /ViewsBlockBase.php:1)
  • test assertions (e.g. core/modules/user/src/Tests/UserPasswordResetTest.php:224)
  • hook_help() (e.g. \user_help())
  • redirects (e.g. new RedirectResponse(\Drupal::url('', [], ['absolute' => TRUE])); -> solveable
  • twig variables (e.g. $variables['book_url'] = \Drupal::url('entity.node.canonical', array('node' => $book_link['bid']));)
  • #theme => item_list's #items should support Url (e.g. core/authorize.php:136)
  • hook_help() (e.g. block_help())
  • test assertions (e.g. core/modules/book/src/Tests/BookTest.php:231)
  • twig variables (e.g. $variables['title'] = \Drupal::l($comment->getSubject(), new Url(''));, core/modules/comment/comment.module:670)
  • links printing the URL or path as link text (e.g. core/modules/views_ui/src/ViewListBuilder.php:268)

Should be deprecated, implicitly already is, we just need to make it explicit.

Should be internal.

Is already deprecated.

  • test assertions (e.g. \Drupal\system\Tests\Common\fully_qualified_local_url) -> add $this->url() & $this->link() to TestBase, those helpers would NOT take cacheability metadata into account
  • t(…) -> should NOT do the manual ->toString() call, that should be taken care of by t() automatically
  • redirects (e.g. return new RedirectResponse($url->toString());); -> solveable, see the point in the conclusion about RedirectResponse
  • HTML HEAD link/feed (e.g. core/modules/content_translation/content_translation.module:561, \Drupal\views\Plugin\views\style\build) -> special case the 'href' attribute in _drupal_add_html_head_link() to only accept Url objects
  • links printing the URL or path as link text (e.g. core/modules/link/src/Plugin/Field/FieldFormatter/LinkFormatter.php:188)
  • entity API validation (e.g. core/modules/link/src/Plugin/Validation/Constraint/LinkNotExistingInternalConstraintValidator.php:51) -> does not need any cacheability metadata
  • Any #theme callback/template that accepts a URL (e.g. #theme => feed_icon's #url, concrete example: core/modules/views/src/Plugin/views/style/Opml.php:46) should pass the Url object, and not the string; TwigExtension::renderVar() already automatically casts that to a string and therefore bubbles cacheability metadata
  • Advanced use cases, for example in Views, where we assign a Url to an object that is passed to a template, whose preprocess function extracts the object property into a seprate variable, causing Twig to render it individually, and hence it is safe: core/modules/views/src/Plugin/views/row/RssFields.php:149, another example of this can be found in Search: \Drupal\search_extra_type\Plugin\Search\SearchExtraTypeSearch::execute()
  • statistics_node_view() and views_views_pre_render() are passing a URL to JavaScript code via drupalSettings; hence it technically needs to associate cacheability metadata, but *actually* they should not be sent at all; the needed URLs can be generated on the client side already using Drupal.url()
  • Views' custom "active link" handling in template_preprocess_views_view_summary()
  • $form['#action'] should support Url objects (e.g. core/modules/views/src/Form/ViewsExposedForm.php:119)

Remove the sole usage & mark as deprecated.

Should be internal.

Should be internal.

  • t(…)
  • $this->l() that should actually be a render array (#type => link …), e.g. core/modules/book/src/Controller/BookController.php:94, \Drupal\dblog\Controller\DbLogController::eventDetails, etc.
  • #theme => item_list's #items should support Url (e.g. \Drupal\help\Controller\HelpController::helpLinksAsList)
  • t(…)
  • $this->url() called to generate a destination query string (e.g. core/modules/comment/src/CommentManager.php:170, core/modules/block_content/src/Plugin/Menu/LocalAction/BlockContentAddLocalAction.php:31)
  • $form['#action'] (e.g. \Drupal\comment\CommentForm::form)
  • redirect

They should either be deprecated and trigger an error

……………

The original reason for introducing \Drupal::url()
See https://www.drupal.org/node/2078285#comment-7819847 →
- * t('Visit the content types page', array('url' => \Drupal::urlGenerator()->generate('node_overview_types')));
+ * t('Visit the content types page', array('@url' => \Drupal::url('node_overview_types')));

Proposed:
- * t('Visit the content types page', array('@url' => \Drupal::url('node_overview_types')));
+ * t('Visit the content types page', array('@url' => Url::fromRoute('node_overview_types'))); 1

Conclusions/notes
-> we can provide our own RedirectResponse class that does accept Url objects
-> It's either painful to fix now, or painful for the entire D8 cycle.

Key problems:

  1. far too many ways to generate URLs and links
  2. all of them need to support the gathering/bubbling of cacheability metadata (see #2335661: Outbound path & route processors must specify cacheability metadata → )

Proposed resolution

  • Use URL objects as much as possible
  • Ensure that all our APIs support URL objects
  • Mark the following bits as internal:

    UrlGenerator->generateFromRoute()
    LinkGenerator->generate(), LinkGenerator->generateFromUrl()

  • Mark the following bits as deprecated:
    UrlGenerator::generate()
    Entity->url()
    Entity->link()
    and throw notices
  • We throw an exception in ::generate() in case we return a string in a inside a render context.
  • For link objects add toString() so its renderable in twig
  • Add Link::toRenderArray() to support using Link objects

Remaining tasks

TBD

User interface changes

None.

API changes

Less APIs!

🌱 Plan
Status

Fixed

Version

10.1 ✨

Component
Base  →

Last updated 2 days ago

Created by

🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

Live updates comments and jobs are added and updated live.
  • Needs issue summary update

    Issue summaries save everyone time if they are kept up-to-date. See Update issue summary task instructions.

Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024