#2917331: Decouple from Symfony CMF changed the return value of \Drupal\Core\Routing\RouteProvider::getAllRoutes, it no longer returns an iterable \Symfony\Cmf\Component\Routing\PagedRouteCollection object, now returns an array. This will break BC.
Previously the interface claimed the method returned an array, and there was little testing for this method, which resulted in this code change.
Example of a break with \iterator_to_array(): https://3v4l.org/PGM6Z
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | 3158708-4.patch | 1.09 KB | catch |
| #4 | 3158708-test-only.patch | 635 bytes | catch |
Comments
Comment #2
catchIs this breaking a contrib module? Or otherwise how did you spot the change?
We currently don't have a clear policy for when implementations fail to match interfaces (i.e. whether changing the interface or the implementation should be considered the bc break). The description and type hint in the interface contradict each other: https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Routing%2...
If we want to change the interface and keep the older implementation, then we should just go ahead and return an iterator again. Wrapping the array in ArrayObject is easy but does PHP have anything lighter these days?
Comment #3
dpiIt broke Entity Route Context, until things were reworked.
Comment #4
catchafaict ArrayObject is still the simplest option here. Here's a patch with test coverage.
Comment #5
amateescu commentedLooks nice and simple :)
Comment #7
andypost+1 rtbc
Comment #8
dwwRouteProviderInterfacestill says this:I thought
[]meant an array, not an iterator. How do you tell the difference in PHPDoc comments?Thanks,
-Derek
Comment #9
jibranDo we want to add BC warning here?
Comment #10
larowlanI don't think we do, because this was only in 9.1
Comment #12
larowlanCommitted c6aa9bc and pushed to 9.1.x. Thanks!
Re #8 I don't think we're consistent
E.g. We have
\Drupal\migrate\Plugin\MigrateIdMapInterface::getMessages,\Drupal\migrate\Plugin\MigrateIdMapInterface::getMessageIteratoramongst others.As this is a critical and we don't want it to ship in a tagged release, committed as is. Can we get a follow-up for #8?
Comment #13
larowlanTagging as this was flagged in the #bugsmash channel which led me here
Comment #14
dpi🕺
Comment #15
dwwRe: #12:
#3161129: [PP-1] Fix phpdoc return type for RouteProvider::getAllRoutes()
Cheers,
-Derek