Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Add Route Annotation : explicit_defaults#28633

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

Closed
chs2 wants to merge1 commit intosymfony:masterfromchs2:master
Closed

Add Route Annotation : explicit_defaults#28633

chs2 wants to merge1 commit intosymfony:masterfromchs2:master

Conversation

@chs2
Copy link
Contributor

Allows generator to add default values to generated URL
in order to get a canonical one

QA
Branch?master
Bug fix?no
New feature?yes
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed tickets
LicenseMIT
Doc PR

If a route has a parameter with default value when it gets generated this parameter is not included

news_export:  path: '/exports/news.{_format}  requirements:    _format: xml|json|rss  defaults:    _format: xml$uri = $router -> generate('news_export')

$uri is/exports/news which is identical, from the router point of view, to/export/news.xml.

URIs should be unique to avoid duplicate content, which is bad for SEO.

Though it is possible to always add default value to parameters ($router -> generate('news_export', ['_format' => 'xml', ])), the added "explicit_defaults" ensures the route will always generated to a canonical form.

Allows generator to add default values to generated URLin order to get a canonical one
@ro0NL
Copy link
Contributor

ro0NL commentedSep 29, 2018
edited
Loading

what about a new path syntax to do this:

path: '/exports/news.{>_format?xml}' (note the?xml is already possible, it's about> here).

path: '/exports/news.{{_format}}' could also do the trick.

@nicolas-grekas
Copy link
Member

I like@ro0NL's idea to deal with this at the param syntax level.
I'd suggest using an! for that:/foo.{!bar} => this would enforce bar to have a default - and would always use it when generating URLs.
WDYT?

ro0NL reacted with thumbs up emoji

@nicolas-grekasnicolas-grekas added this to thenext milestoneSep 29, 2018
@chs2
Copy link
ContributorAuthor

My original idea was to have a canonical URL generated.
Here the example has only one parameter but there could be more :

the_route:  path: /{section}.{_format}  requirements:    section: (index|news|authors)    _format: (html|xml|json)  defaults:    section: index    _format: html

Having the option on the parameter level gives more flexibility but IMO defeats the purpose of the canonical URL.

(BTW, the next step I had in mind was having the Router sending a Redirect when it matches a route on a non-canonical form.)

@ro0NL
Copy link
Contributor

ro0NL commentedSep 29, 2018
edited
Loading

we can debate if redirecting non-canonical to canonical URLs is something that belongs to core 🤔

if so, that might also be triggered by e.g.canonical: true, which IMHO is a better semantic thenexplicit_defaults. In that case it would create a redirect with a default value for all parameters.

@nicolas-grekas
Copy link
Member

The generated URL is already a canonical one to me. I get you'd like it to have the suffix in it instead. It's not about canonical 1/0 to me, but about suffix 1/0, isn't it?

@nicolas-grekas
Copy link
Member

@chs2 WDYT about these proposals? Would you like to implement it?

@chs2
Copy link
ContributorAuthor

I'd like to but I can't get my head around the param syntax especially with the XML/XSD.
I could go with "canonical: true" proposed by@ro0NL (instead of my "explicit_defaults").

@ro0NL
Copy link
Contributor

ro0NL commentedOct 22, 2018
edited
Loading

@chs2 i dont think we need canonical:true|false, as@nicolas-grekas mentioned each configured route is/should be already a canonical one. So the options are:

  • Handle the duplicate content redirect in code / at server level (htaccess) or another router
news_export:path:'/exports/news.{_format}'defaults:_format:xml
  • Handle the duplicate content by another route definition
news_export_non_canonical:path:'/export/news'controller:Symfony\Bundle\FrameworkBundle\Controller\RedirectController::redirectActiondefaults:route:news_export_xmlnews_export_xml:path:'/exports/news.xml'

These are explicit configurations to show we dont needcanonical: true as a feature. The user should be responsible here.

If we look at option 2, we see our missing step: it cant specify the{_format} placeholder to be required (so it would need a route per format).

That's what we should solve by the proposed{!_format} syntax, so we can do

news_export_non_canonical:path:'/export/news'controller:Symfony\Bundle\FrameworkBundle\Controller\RedirectController::redirectActiondefaults:route:news_exportnews_export:path:'/exports/news.{!_format}'defaults:_format:xml

Due! syntax we would always require a format (given or default).

@nicolas-grekas
Copy link
Member

Thanks@chs2 for raising the point. I think the! syntax is what we should implement.
I created#29593 to keep track of the idea.
Closing here as this is quite different.
Help wanted to implement that!

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Projects

None yet

Milestone

4.3

Development

Successfully merging this pull request may close these issues.

4 participants

@chs2@ro0NL@nicolas-grekas@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp