Updated: Comment #2
Problem/Motivation
The content of $module.routing.yml is has a high WTF/line measure since it tries to use a raw symfony format for all the keys and forces us to use a leading "_" for no reason and to group or name keys according to the pure symfony parsing.
Among the worse parts is trying to use a non-permission access check. Even with permissions I have to remember it's under "requirements" not "access".
Proposed resolution
Provide an alternate parsing/translation of Drupal-specific keys that make it delightful to accomplish common developer tasks in terms of defning how the route content, access, etc are handled.
node.add_page:
path: '/node/add'
defaults:
_title: 'Add page'
_content: '\Drupal\node\Controller\NodeController::addPage'
requirements:
_permission: 'administer content types'
_node_add_access: 'node'
node.revision_show:
path: '/node/{node}/revisions/{node_revision}/view'
defaults:
_title: 'Revisions'
_content: '\Drupal\node\Controller\NodeController::revisionShow'
requirements:
_access_node_revision: 'view'
Could be something more like: this, where e.g. access services are named directly rather than having to grep for the tag and matching that to a service.
node.add_page:
path: '/node/add'
properties:
title: 'Add page'
content: '\Drupal\node\Controller\NodeController::addPage'
access:
- 'administer content types'
- access_check.node.add: 'node'
node.revision_show:
path: '/node/{node}/revisions/{node_revision}/view'
properties:
title: 'Revisions'
content: '\Drupal\node\Controller\NodeController::revisionShow'
access:
- access_check.node.revision: 'view'
Remaining tasks
Define the desired format
Make sure we can parse either this Druaplized version or the exisintg raw symfony format
User interface changes
none
API changes
None- should still be compatible with existing API
Comments
Comment #1
alexpottTagging
Comment #1.0
pwolanin CreditAttribution: pwolanin commentedUpdated issue summary.
Comment #2
dawehnerThis for sure makes it way easier to learn what the different entries actually mean, especially removing the magic underscores.
Not sure whether properties is a good name. Another short idea: Move access one level up.
If we decide to basically hide symfony from people the question is whether telling people that learning Drupal 8 will allow them to apply the knowledge to other systems is still true.
Comment #3
pwolanin CreditAttribution: pwolanin commented@dawehner - yeah, this was a really quick first pass. However, I was trying to suggest that we need to avoid potential conflict with symfony keys that might happen if we put them all at the top level.
I don't think most module developers also aspire to be symfony routing experts, and the thought would be to leave the current parsing in place, as an "expert" mode.
Comment #4
sunRemarks:
Comment #5
dawehnerJust linking to the original title issue to explain why titles on routes are a thing which are needed.
https://drupal.org/node/2032535
Comment #6
webchick#4 actually looks like a pretty promising start to me.
Comment #7
fubhy CreditAttribution: fubhy commentedExcept that the "access" part is short-sighted and won't work like that. The way we currently use it is short sighted too. That reminds me... I gotta open an issue for that. It's currently impossible to use entity access checks more than once on a single route... You can't currently check access for two entities in the route argument simultaneously. That's a limitation we will eventually run into. Access checks should be declared as lists or generally support lists internally. I guess I'd prefer the first option (making access checks a list in general).
Comment #8
Crell CreditAttribution: Crell commentedIssues with #4:
1) Any format we use that's not consistent with Symfony's means more things we need to document ourselves. We won't be able to rely on Symfony documentation (and it's another speed bump for devs moving between Drupal and Symfony.)
2) By design, there's a lot more than just "controller" that can be specified. Currently we have _controller, _content, _form, _entity_view, _entity_form, and probably one or two others I'm forgetting. Supporting multiple varieties of "thing that handles the route" is a feature we cannot lose.
3) Title also has title callback.
4) As fubhy notes, access control is a bit more involved than that.
5) We are using the "requirements" parameters already for their regex filtering. We have to continue supporting that, even if we move our access check stuff out of that block.
6) There's also method / requirements: _method, format / requirements: _format, scheme, and domain restrictions we currently support. Those cannot be lost either just because they're not the common case. (Symfony has most of those start as _-prefixed requirements, and some eventually migrate to top-level items with BC support. Just as both path and pattern are currently supported.)
7) We still need to support "defaults", as that's a rather critical feature. You can't have optional parameters otherwise.
8) Just a note, even if we do change the YAML format we cannot really change the Route object structure. That means anyone making a dynamic route will be exposed to the "native" structure of the object, which maps 1:1 to the current YAML file. That is, defining routes dynamically will be *completely different* in terms of what values you need to set than defining them in YAML. That would be a major WTF/DX Fail.
Comment #9
damiankloip CreditAttribution: damiankloip commentedYes, #8.8 is a really good point. So route objects will still need options, defaults, and requirements. So they would still have to fit into one of those anyway. Which it currently does.
Otherwise, E.g. the (strange) access array above would still live in requirements? under an access key or something? This will make it much more difficult to find anything, plus we'll just get this mangled up data.
Comment #9.0
damiankloip CreditAttribution: damiankloip commentedupdate summary
Comment #10
tim.plunkettThe structure of *.routing.yml maps 1:1 with the constructor of the \Symfony\Component\Routing\Route class.
Here is the code that uses the parsed YAML:
In #2145041: Allow dynamic routes to be defined via a callback we just worked to increase the similarity between more advanced routing needs and static YAML files.
For example, the current SearchPluginRoutes does:
If we change one, we need to change the other. Dealing with this data in a different form will be very confusing.
Comment #11
dawehnerIf we change that here we also run into the problem that everything internal would be different as well. There are many places like Access checkers,
breadcrumb builders, theme negotiators where we deal with potential route objects. Yes these examples are drupal internal, but at some point we basically move away 100% from the architecture of symfony + CMF, which makes us less interobable and people have to understand kind of both syntaxes.
Comment #12
sunHm. That issue title change was a bit premature. There's not really a reason for having to override Symfony's Route object with a custom one. The schema, structure, and format of route declarations in routing.yml files is decoupled from how the data is parsed and how the parsed data is being processed. There's no technical reason for replacing the Route object with a custom one.
Likewise, I do not think that anyone ever proposed that it should not be possible to use Symfony's native declaration syntax for routes. Yes, that is well-documented and yes, Symfony developers will be more familiar with it. There's absolutely no reason for why you shouldn't be able to use it. I do not think that anyone disrespects the point that is being made. And there is no reason to think in black and white.
But at the same time, Drupal is not a framework, but a product that uses a framework. A product always comes with vendor-specific things to learn, which have been optimized for the product's use-case. The underlying framework still supports the original framework concepts as-is. But there's no reason to cripple the common product experience for the ordinary and primary product consumers, just because the underlying framework is more complex and abstract (as it cannot make any assumptions).
Let's face reality: The vast majority of modules integrate with Drupal\Core\Entity, Drupal\Core\Form, and Drupal\Core\Access.
Because you see "Core" in all of that, and not "Component", all of these concepts are specific to Drupal. As a consequence, the implementations are specific to Drupal, too. Within these implementations, we're working very hard to improve the DX so as to make their usage as simple as possible.
But yet, when it comes to registering these implementations to be accessible on certain routes, the DX ball is suddenly dropped, and you're forced to learn how the underlying, use-case-agnostic framework wants the data to be declared. Not only the declarative syntax is immensely abstract, but on top, the entire nomenclature being used does not map to any of the concepts that you're familiar with (in your actual code/implementation).
As a product, we can optimize the declarative syntax for our concrete use-case in order to improve DX for our primary consumers, while retaining support for the original syntax in parallel. Let's stop thinking in black and white, please.
Comment #13
tim.plunkettThat sounds more like an argument for Drupal\Core\Routing\Route than anything I wrote.
Not getting into an edit war just yet, but I don't think we can change the routing YAML without fixing the route callbacks and alters.
Comment #14
sun@tim.plunkett: Could you elaborate more (as much as possible) on these two sentences? It sounds like you're having some more concrete concerns in mind, but the rest of us (1) doesn't know what those are, (2) why they are important, and (3) whether they actually are as important as you see and interpret them. That would be terrific. :)
Comment #15
tim.plunkettHere is some D7 code:
And here is some D8 code:
So the *.routing.yml has path, defaults, requirements, and options. And it points to this callback, which creates a new Route object, which has the parameters of path, defaults, requirements, and options.
As you can see from the D7 example, this code did not need to be separate. The same goes for if/foreach D7 code.
While knowing that we couldn't have the same cohesiveness as D7, we still tried to keep these two as similar as possible.
If you propose to change the keys of the *.routing.yml, any semblance of similarity will be gone. In order to write some routes, they'll need to learn our proprietary Drupal syntax, and all documentation they find about Symfony routes will be wrong. Until they need to write that one conditional or loop, in which case they would nee to learn the Symfony syntax after all.
That is unacceptable.
Either we keep with the current syntax (won't fix), or we have to change the class we use (retitle the issue).
Comment #16
dawehnerEven if we also change the syntax in our dynamic route subscribers at some point we basically have to decide between getting of the island and accept what other people had in mind vs. building a solution which works at least for the usecase of core as good as possible.
There are a lot of good symfony docs/people which drupal can profit on, but yeah this is a way more meta question which has to be answered as well.
Let's assume we have optimized for the solution of core, people still have to learn the way how it internally looks like for probably 50% or more usecases. Writing access checkers/breadcrumb builders or even accessing the route object in your controller is quite a common usecase if you look at drupal itself. Now you end up with two syntaxes which are hard to map for the user.
Instead of babysitting people like that we could also think of improving tools for the users. Create good scaffolding tools, talk with IDE people
and finally provide easy c&p-able examples.
Comment #17
webchickWell, I think the general idea would be that all of those would be written in a similar way (aka, with definition language geared towards Drupal's specific use vs. Symfony's abstraction/genericism) but it's definitely a fair point that such a decision has far-reaching consequences.
Comment #18
chx CreditAttribution: chx commentedAlternative approach here #2177015: Create a saner routing YAML format
Comment #19
Wim LeersStrongly related: #2092647: Consolidate _form, _controller, _entity_form, etc. into just _controller?.
Comment #20
tim.plunkettThe ship has sailed for D8, and I think we made the right choice to keep Symfony's structure. This could be reopened for D9 if desired.