Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Committer notes: This was worked on by kim.pepper and dashaforbes together so please include him in the commit credits.
Problem/Motivation
As a part of #1744302: [meta] Resolve known performance regressions in Drupal 8 we want to reduce the number of getContainer() calls in LinkGenerator. We can do this by avoiding calling it for external links.
Proposed resolution
Remaining tasks
Task | Novice task? | Contributor instructions | Complete? |
---|---|---|---|
Update the issue summary noting if allowed during the beta | Instructions | ||
Review patch to ensure that it fixes the issue, stays within scope, is properly documented, and follows coding standards | Instructions |
User interface changes
API changes
Comment | File | Size | Author |
---|---|---|---|
#20 | 2421197-16-reroll.patch | 5.15 KB | joelpittet |
#16 | 2421197-16.patch | 5.15 KB | joelpittet |
#14 | 2421197-14.patch | 5.5 KB | joelpittet |
| |||
#9 | interdiff.txt | 2.17 KB | kim.pepper |
#9 | 2421197-link-generator-container-9.patch | 6.32 KB | kim.pepper |
Comments
Comment #1
kim.pepperThis injects Renderer and UnroutedUrlAssembler into LinkGenerator to reduce the number of calls to getContainer().
This also adds a new test to ensure the renderer is called when $text is an array.
Comment #2
larowlanThis can be wrapped in a
not needed
Comment #3
kim.pepperFixes for #2
Comment #6
dawehnerWhat about using @covers?
Note: You can do the same with ->willReturn('/test-route-7') and similar.
Comment #9
kim.pepperFixes for #6 as well as a reroll for missing constructor args in FieldPluginBaseTest.
Comment #11
YesCT CreditAttribution: YesCT commentedneeds a beta evaluation in the issue summary (especially since this is a normal task).
Comment #14
joelpittetRe-rolled this, doesn't need beta evaluation anymore.
Comment #16
joelpittetAdded a duplicate renderer property by accident.
Comment #18
dawehnerI believe this is clearly not a bug, so it just applied to 8.3.x. In that case though there is no BC problem involved.
Comment #20
joelpittetReroll