Support Attributes #194
Support Attributes #194
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #194 +/- ##
============================================
- Coverage 97.57% 95.26% -2.32%
+ Complexity 139 137 -2
============================================
Files 9 9
Lines 413 422 +9
============================================
- Hits 403 402 -1
- Misses 10 20 +10
☔ View full report in Codecov by Sentry. |
samdark
added
type:enhancement
|
Need docs and additional tests. |
|
|
||
| Only two properties are required: `methods` and `pattern`. The rest properties are optional. | ||
|
|
||
| > Parsing attributes is up to user. The package does not provide a solution for parsing and constructing routes tree. |
There was a problem hiding this comment.
Need to link to the guide on how to achieve colleccting attributes or to describe it right here.
There was a problem hiding this comment.
We don't have any attributes collectors. I'll add it later if we merge this example
|
Overall I like it 👍 |
| } | ||
|
|
||
| private function buildDispatcher( | ||
| ?MiddlewareDispatcher $dispatcher, |
There was a problem hiding this comment.
| ?MiddlewareDispatcher $dispatcher, | |
| MiddlewareDispatcher $dispatcher, |
There was a problem hiding this comment.
The dispatcher is nullable
| if ($dispatcher === null) { | ||
| throw new RuntimeException(sprintf('There is no dispatcher in the route %s.', $route->getData('name'))); | ||
| } | ||
|
|
There was a problem hiding this comment.
| if ($dispatcher === null) { | |
| throw new RuntimeException(sprintf('There is no dispatcher in the route %s.', $route->getData('name'))); | |
| } |
| if ($this->dispatcher !== null && !$this->route->getData('hasDispatcher')) { | ||
| $this->route->injectDispatcher($this->dispatcher); | ||
| if ($dispatcher->hasMiddlewares()) { | ||
| return $dispatcher; |
There was a problem hiding this comment.
Router pass middlware dispatcher to MatchingResult and this dispatcher almost always contain middlewares. In this case route middlewares will not work.
There was a problem hiding this comment.
I don't understand what you are saying
|
See #196 |
|
Closed in favor of #196 |
Uh oh!
There was an error while loading. Please reload this page.