Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork5.2k
[Routing] Document the Route attribute#14230
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
routing.rst Outdated
| class BlogController | ||
| { | ||
| #[Route('/blog/{page}', name: 'blog_index', defaults: ['page' => 1, 'title' => 'Hello world!'])] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
💡 I've kept this attribute single-line in order to remain consistent with the existing annotation block. But I think, the example would be more readable with we defined the attribute/annotation multi-line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I prefer multiline too, please adjust the examples. If you like here or in a separate PR first, your choice 😃
7c4d81e toa9723e7CompareThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thanks a lot, not only for single-handly making Symfony PHP 8 compatible, but also documenting all great new features!
When a code block illustrates the logic inside a controller action and the annotation is only used to provide context, I think it would create too much noise to show the attribute and the annotation way. In routing.rst, I have replaced the annotation with an equal attribute in such cases. I think it would be a good idea to do the same with the rest of the documentation, but I'd like to discuss this with you before I change all those spots.
I agree that having both would create too much noise. I however think we should stick to using annotations in these places. Examples are often used to copy/paste by beginners and I want everyone to be able to use and understand our code examples (as long as they're on a Symfony-supported PHP version). So I think our code examples should always base on the lower bound of Symfony's PHP requirements, which at this moment is 7.2.5 for Symfony 5.2.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
routing.rst Outdated
| .. versionadded:: 5.2 | ||
| The ability to use PHP attributes to configure routes was introduced in | ||
| Symfony 5.2. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I think this should be moved after thecomposer require ... example + we should reword that paragraph and the next one. It is no longer required to install doctrine annotations if you're using PHP 8 attributes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I agree. I was thinking, we should also change the recipes. Ifdoctrine/annotations is an optional dependency now, the routes snippet that enables attributes/annotations should not be attached to thedoctrine/annotations package anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
That's exactly what I was thinking. The big question is then: what should be the trigger for this recipe? Maybe, we should make it the default routing config when installing in a PHP 8 application? (I don't think that's a feature of Flex atm)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
We could add the file to thesymfony/routing recipe for 5.2. If we're on PHP 7, that file should have no effect. 🤔
a9723e7 toaff430fCompare
That would likely mean we cannot switch the code examples to attributes before Symfony 7.0 in 2023. I'm rolling back the examples in question, but we might want to reconsider this rule in a year or so. |
7628b3e to35a1c49Compare
Yeah, and maybe the other doc team members have another opinion than me :). Btw, I think it's also good to consider PHP 8 as lower bound for Symfony 6, given that it does bring significant new features (like attributes) and is released a year before Symfony 6.0. |
Uh oh!
There was an error while loading.Please reload this page.
routing.rst Outdated
| class BlogController | ||
| { | ||
| #[Route('/blog/{page}', name: 'blog_index', defaults: ['page' => 1, 'title' => 'Hello world!'])] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I prefer multiline too, please adjust the examples. If you like here or in a separate PR first, your choice 😃
@derrabus I mergedOskarStark/doctor-rst#758, can you please rebase to get the latest DOCtor-RST build? Thanks 🙏 |
35a1c49 toeade19cCompare
Done! |
eade19c tod261a29CompareSorry for the delay. The blocks are swapped now. |
d261a29 to4ca794eCompareThanks again@derrabus! |
This PR was merged into the 5.x branch.Discussion----------Use updated dependenciesCurrently I fail to build the docs in 5.x. They complain on php-attributes.This PR make sure to use@derrabus PRfabpot/sphinx-php#49 released in 2.0.2.This change was missing from#14230Commits-------3c0372a Use updated dependencies
This PR documents the new
#[Route]attribute that was introduced withsymfony/symfony#37474 and thuscloses#14188.For the routing configuration via Doctrine Annotations as well as PHP attributes, we use the very same class,
Symfony\Component\Routing\Annotation\Route. This means that both mechanisms are equally powerful. Right now, there's nothing you can do with@Routethat is impossible with#[Route]and vice versa. The main difference is that you don't need an external library for attributes because they're a native language feature.Because of that, I'd like to shift the general recommendation to use annotation towards attributes and recommend annotations as the fallback for projects that need to remain compatible with PHP 7.
Right now, the
@Routeannotation is used throughout the documentation in two different ways:In this case, I've added an additional code block that shows the same configuration for attributes. I've consistently placed the attribute block before the annotation block.
When a code block illustrates the logic inside a controller action and the annotation is only used to provide context, I think it would create too much noise to show the attributeand the annotation way. In
routing.rst, I have replaced the annotation with an equal attribute in such cases. I think it would be a good idea to do the same with the rest of the documentation, but I'd like to discuss this with you before I change all those spots.