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

Adds Route support in App->route() #239

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Adds Route support in App->route() #239

wants to merge 1 commit into from

Conversation

emersion
Copy link

@emersion emersion commented Oct 6, 2014

This PR allows to provide a Route object as $path in order to be able to build more complex endpoints, such as:

$app->route(new Route('/{any}', array(), array('any' => '.*')), $httpServer);

(this redirects all URLs to the controller)

@cboden
Copy link
Member

cboden commented Nov 8, 2014

I'm on the fence about this approach. On the plus side it maintains some ideal configuration for the server security. On the minus side it's breaking both the App and Route abstractions.

The RouteCollection has been exposed as a public property $routes on the App class. Is that open enough to fit your more complex endpoints?

@emersion
Copy link
Author

The problem is that we have to set properties such as _controller, Origin and call setHost manually. That's not very convenient, do you have another idea?

@saidmoya12
Copy link

For security reasons you cannot implement this directy on the bundle but..

1 you can override the class, (extends)

  1. You can install a bundle or library for cors domain

You don't must implement that directly on a bundle becouse that is an specific situation

@mikegioia
Copy link

Has there been any update on this? I think the only difference between @emersion 's pull request and the suggestion from @cboden is that the PR sort of collects what I'd have to do anyway with the public $routes collection.

The PR is more of a convenience method in accepting a Route object instead of a path string imo. However, what's painful about manually hitting $routes is that I have to emulate all of the decorations Ratchet adds when creating $decorated. I'll end up copying out more code than if I just extended Ratchet\App with my own class :(

@sudoanand
Copy link

sudoanand commented Oct 9, 2018

This is a real requirement and would love to see this feature.

Please have a look at this related PR: #689

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.

None yet

6 participants