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.Problem/Motivation
Drupal\Core\Utility is filled with classes that have all static methods, most of which have no external dependencies.
LinkGenerator is a service.
We've made a distinction between Services and Utility classes, we should keep that.
This also applies to #2346361: Add a UnroutedUrlAssembler and put it into the container
Proposed resolution
Move LinkGenerator to a new namespace: Drupal\Core\Render\LinkGenerator
Remaining tasks
| Task | Novice task? | Contributor instructions | Complete? |
|---|---|---|---|
| Create a patch | Instructions |
User interface changes
API changes
Beta phase evaluation
| Issue category | Task because it functions normally without the change (not a bug) |
|---|---|
| Issue priority | Minor because nothing is currently broken. |
| Unfrozen changes | It's frozen because it changes namespaces |
| Prioritized changes | Possibly a prioritized DX improvement. |
| Disruption | Disruptive for current patches in core which touch LinkGenerator. Other code will be using the service, which will be somewhat more namespace-agnostic. Patches which use the service will need to double-check their @var documentation. |
| Comment | File | Size | Author |
|---|---|---|---|
| #32 | 2346431_32.patch | 17.83 KB | Mile23 |
| |||
| #24 | interdiff-14-23.txt | 10.51 KB | tadityar |
| #23 | 2346431-23.patch | 19.73 KB | tadityar |
| #22 | 2346431-22.patch | 61.88 KB | tadityar |
| #20 | 2346431-19.patch | 37.73 KB | tadityar |
Comments
Comment #1
dawehnerI don't think this is novice as we originally had big issues finding a better namespace.
Comment #2
tim.plunkettRight, deciding the namespace isn't novice, but performing the move will be.
Comment #3
Palashvijay4O CreditAttribution: Palashvijay4O commentedSubmitting a patch .
Comment #4
tim.plunkett@Palashvijay4 I think this was the wrong patch?
Comment #5
jibranNW for #4
Comment #6
Palashvijay4O CreditAttribution: Palashvijay4O commentedSir @tim.plunkett why the patch is not correct ?
Comment #7
tim.plunkettAll the patch does is remove the LinkGeneratorTest, and change unrelated CSS.
It does not touch LinkGenerator at all.
Comment #8
alimac CreditAttribution: alimac commentedComment #9
rpayanmany progress @Palashvijay4O?
Comment #10
dawehnerWe should not assign issues anyway these days.
Comment #11
rpayanmThen, which is the new namespace for LinkGenerator? I like work on this!
Comment #12
dawehnerWell, that is the point of the issue, find a good one.
Do you have a good suggestion?
Comment #13
mpdonadioIt generates HTML, so Drupal\Core\Render would potentially be valid, and this is where the render service landed.
It's related to routing, so Drupal\Core\Routing may make sense. Same could be said for Drupal\Core\Path.
It could go in a new namespace in case there are any other link related activities in the future, Drupal\Core\Link
Of those, since the output of this service is HTML, then Drupal\Core\Render is most appropriate.
Comment #14
borisson_I agree that Drupal\Core\Render makes the most sense, I've added a patch that implements this change.
Comment #15
tadityar CreditAttribution: tadityar commentedstatus change so bot will test.
Comment #17
RavindraSingh CreditAttribution: RavindraSingh commentedIf Linkgenerator is moving out from unitlity so LinkGenratorInterface and LinkGeneratorTest should moved out
use Drupal\Core\Utility\LinkGeneratorInterface;
use Drupal\Tests\Core\Utility\LinkGeneratorTest;
Should be
use Drupal\Core\render\LinkGeneratorInterface;
use Drupal\Tests\render\Utility\LinkGeneratorTest;
Comment #18
tadityar CreditAttribution: tadityar commentedTrying. I also moved LinkGeneratorInterface and the test to Render. Please correct me if I'm wrong.
Comment #20
tadityar CreditAttribution: tadityar commentedForgot to change the namespace in LinkGeneratorTest.php . Changed now
Comment #21
shivanshuag CreditAttribution: shivanshuag commented@tadityar The patch should also remove LinkGenerator.php and LinkGeneratorInterface.php from core/tests/Drupal/Tests/Core/Utility/ and LinkGeneratorTest.php from core/tests/Drupal/Tests/Core/Utility/
Comment #22
tadityar CreditAttribution: tadityar commented@shivanshu: oops, updated!
Comment #23
tadityar CreditAttribution: tadityar commentedI realized that if do the method in patch #22 It'll remove the history of the file. So this time I used
git diff -C --stagedinstead.Comment #24
tadityar CreditAttribution: tadityar commentedAdding interdiff.
Comment #25
dawehnerSomeone should update the issue summary and add a beta evalulation to explain how useful this is, or not.
Comment #26
tadityar CreditAttribution: tadityar commentedAdded beta evaluation and priority change. Please correct me if I'm wrong.
Comment #27
tadityar CreditAttribution: tadityar commentedComment #28
dawehnerThis itself looks fine, thank you!
What is your oppinion here, do you think its worth broken existing people's code due to the renaming of the namespace?
Comment #29
tadityar CreditAttribution: tadityar commented@dawehner I think that it depends on how often this class is used, but since I don't know how often it is I'm not sure.
Comment #30
dawehnerWell, that classed is used quite often, given that it will be used to create links, actually it is one of the most often used ones.
Comment #31
tadityar CreditAttribution: tadityar commentedSo, will we move this at all? Maybe it can be closed(won't fix)?
Comment #32
Mile23Reroll.