Problem/Motivation

In \Drupal\Core\Routing\UrlGenerator::__construct()

$this->provider = $provider;

Is a dynamic declaration

Proposed resolution

Declare and document the property

Remaining tasks

write patch

User interface changes

API changes

none

Data model changes

none

CommentFileSizeAuthor
#5 2809801-5.patch507 bytesziomizar
#5 interdiff-2809801-2-5.txt550 bytesziomizar
#2 drupal-2809801.patch562 bytesziomizar
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

pwolanin created an issue. See original summary.

ziomizar’s picture

ziomizar CreditAttribution: ziomizar at East Atlantic Engineering for East Atlantic Engineering commented
FileSize
drupal-2809801.patch562 bytes
ziomizar’s picture

ziomizar CreditAttribution: ziomizar at East Atlantic Engineering for East Atlantic Engineering commented
Status: Active » Needs review
pwolanin’s picture

pwolanin CreditAttribution: pwolanin as a volunteer commented
Status: Needs review » Needs work

Looks mostly right, but I don't think we need to type hint both interfaces, only RouteProviderInterface becuase PreloadableRouteProviderInterface extends RouteProviderInterface and we never call the extra method \Drupal\Core\Routing\PreloadableRouteProviderInterface::preLoadRoutes() in this context.

ziomizar’s picture

ziomizar CreditAttribution: ziomizar at East Atlantic Engineering for East Atlantic Engineering commented
FileSize
interdiff-2809801-2-5.txt550 bytes
2809801-5.patch507 bytes

Ohhh, I was looking at class RoutePreloader to see an example of that, in that class is used PreloadableRouteProviderInterface.

ziomizar’s picture

ziomizar CreditAttribution: ziomizar at East Atlantic Engineering for East Atlantic Engineering commented
Status: Needs work » Needs review
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner at Chapter Three commented
Status: Needs review » Reviewed & tested by the community

Looks perfect

  • catch committed 182f8fd on 8.3.x 
    Issue #2809801 by ziomizar: provider property is dynamically declared in...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Version: 8.2.x-dev » 8.3.x-dev
Category: Bug report » Task

I added a note to the bc policy for public properties: https://www.drupal.org/node/2562903/revisions/view/10149671/10168259

Committed/pushed to 8.3.x, thanks!

This doesn't fix a bug, so not putting in 8.2.x

pwolanin’s picture

pwolanin CreditAttribution: pwolanin as a volunteer commented
Status: Reviewed & tested by the community » Fixed

ok, so fixed then

Status: Fixed » Closed (fixed)

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