Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Routes should be just imported, not mounted #6612

Closed
wants to merge 1 commit into from
Closed

Routes should be just imported, not mounted #6612

wants to merge 1 commit into from

Conversation

OndraM
Copy link
Contributor

@OndraM OndraM commented May 25, 2016

It just causes me headache to figure out why the routes behave so weirdly even if you follow the cookbook 1:1 :-).

When calling mount() on already imported routes (what is stated in the cookbook), the router behaves a bit weirdly and mounts the routes in this way:

-------------------------- -------- -------- ------ --------------------------------------------- 
  Name                       Method   Scheme   Host   Path                                         
 -------------------------- -------- -------- ------ --------------------------------------------- 
  _wdt                       ANY      ANY      ANY    /_wdt/_wdt/{token}                           
  _profiler_home             ANY      ANY      ANY    /_profiler/_profiler/                        
  _profiler_search           ANY      ANY      ANY    /_profiler/_profiler/search                  
  _profiler_search_bar       ANY      ANY      ANY    /_profiler/_profiler/search_bar              
...

Basically the prefixes are doubled, what is definitely not intended. While the behavior of the RouteCollectionBuilder is kinda weird, I rather think this is a just misuse of the mount() method - which is not needed and is probably not expected to be called on already imported and mounted routes.

Moreover, the import of annotation routes works just by coincidence - the second parameter of import() is prefix, not type, so the routes are added with prefix "/annotation" and only because the mount is used, they appears as mounted to "/" prefix.

Cc @weaverryan

@stof
Copy link
Member

stof commented May 25, 2016

the doc is wrong because it was written based on an earlier version of the RouteCollectionBuilder which was not auto-mounting routes when importing them

@wouterj
Copy link
Member

wouterj commented Jul 2, 2016

Hi @OndraM! It looks like your completely correct here. I'm sorry we didn't do anything with this PR. I rebased your commit and created #6708. You'll still get all credits.

Thanks!

@wouterj wouterj closed this Jul 2, 2016
wouterj added a commit that referenced this pull request Jul 4, 2016
This PR was merged into the master branch.

Discussion
----------

Routes should be just imported, not mounted

Finishes #6612

Commits
-------

34a3166 Routes should be just imported, not mounted
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants