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

4.x - Get base path within a route callback #2837

Closed
adriansuter opened this issue Sep 16, 2019 · 10 comments
Closed

4.x - Get base path within a route callback #2837

adriansuter opened this issue Sep 16, 2019 · 10 comments

Comments

@adriansuter
Copy link
Contributor

Say you have a static file for which you would like to build the url. So the url is not a route. As there might be a base path present, we can't just write /static-file.html. How do we get the base path or even the base url from within a route callback?

Of course we could inject the base path to the class containing the route callback. But there should be a better solution.

Something like

$absoluteUrl = RouteContext::fromRequest($request)->getRouteParser()->basePath() . 'static-file.html';
$fullQualifiedUrl = RouteContext::fromRequest($request)->getRouteParser()->fullBasePath($uri) . 'static-file.html';

I am not sure if that belongs to the route parser though. If it does, we could maybe instead of the above create two methods that are similar to urlFor and fullUrlFor but instead of using a route name, we could pass a path (relative to the application end point).

public function url(string $path, array $queryParams = []): string;
public function fullUrl(UriInterface $uri, string $path, array $queryParams = []): string;

So

RouteContext::fromRequest($request)->getRouteParser()->url('static-file.html');
RouteContext::fromRequest($request)->getRouteParser()->fullUrl($uri, 'static-file.html');

What do you think?

@adriansuter
Copy link
Contributor Author

Any other ideas or opinions on that?

@odan
Copy link
Contributor

odan commented Sep 21, 2019

@adriansuter What would be the difference compared to the existing Uri::getBaseUrl?

https://github.com/slimphp/Slim-Http#decorated-uri-object-methods

The decorated UriInterface provides the following additional methods:
Uri::getBaseUrl()
Returns the fully qualified base URL of the underlying uri object.

I think first we should get a better overview of what we have now (and where) and point out the differences and uses cases for each of that.

@adriansuter
Copy link
Contributor Author

@odan if your app lives in a subdirectory, say app, that is you would access your root route / with http://example.com/app, then the base path is /app. Say I have a file script.js, how can we build a path inside a route callback that works even if we change the base path later on? Would that work with the http decorator Uri?

@l0gicgate
Copy link
Member

Any other ideas or opinions on that?

@adriansuter I think we could maybe provide the basepath via middleware when routing is done? Then maybe add onto the RouteContext object?

@odan
Copy link
Contributor

odan commented Sep 26, 2019

@adriansuter The currently possible way would be to define a "name" for the / route, e.g. "root" and use this name to determine the url with the basePath.

$app->setBasePath('/app');

$app->get('/', \App\Action\Home\HomeAction::class)->setName('root');

$app->get('/users', \App\Action\User\ListUserAction::class)->setName('list-user');

Then you can fetch a url path (with the baseBath) like this:

// Result: /app
$url = RouteContext::fromRequest($request)->getRouteParser()->urlFor('root');

// Result: /app/users
$url = RouteContext::fromRequest($request)->getRouteParser()->urlFor('list-user');

Using the route name is also better in terms of maintainability, because there is only one "source of truth" for the route.

Edit: This works fine for existing routes, but we might want to create a dynamic route. In this case @l0gicgate idea with a basePath middeware / attribute would be nice.

@piotr-cz
Copy link
Contributor

I think RouteParser should implement basePath method.

IMHO RouteParser class is good place for it as

  • it contains other related methods (relativeUrlFor, urlFor, fullUrlFor)
  • it can be available in route attribute (and indirectly in middleware) when using App::addRoutingMiddleware

However I'd suggest method with no arguments to keep it as simple as possible.

class RouteParser
{
    // ...

    public function basePath(): string
    {
        $this->routeCollector()->getBasePath();
    }
}

One can always use sprintf('%s/static-file.html', $this->routeParser->basePath()), extend class or use composition in view.

Another helpful method would be baseUrl which could be used for example in <base> html element:

<html>
  <head>
    <base href="<?= $this->routeParser->baseUrl() ?>/" />
  </head>
  <body>
    <a href="<?= $this->routeParser->relativeUrlFor('about-us') ?>">about us</a>
    <img src="./image.png" />
  </body>
</html>

@adriansuter
Copy link
Contributor Author

@piotr-cz Keep it simple is good. The thing is, if you are developing an app and always forget to prepend basePath() to those urls (say you develop in an environment without base path and do not test these cases), then your app would not work once someone tries to install it in an environment with a base path.

My idea was to kinda force the developer to use the url method in order to get a valid url to a static file. So whenever the developer needs a url to a route or a static file then either urlFor or url should be used.

This does not mean, that we should not have a method to get the base path or the base url. In fact, I would even suggest to implement all four methods. What do you think @l0gicgate? Too much?

@piotr-cz
Copy link
Contributor

piotr-cz commented Oct 1, 2019

@adriansuter I get your point.
However scenarios you are describing are specific to application which renders html responses.

In that case it's more convenient to use one of the view libraries (PHP-View or Twig-View) and implement these methods in them (or in extending classes).

But to do so, there must bu a way to retrieve basePath.

I think that one thing we both agree on is that there should be a method to get basePath similarly like we get routes. I suggest that once such implementation lands in codebase we can discuss other methods.

@adriansuter
Copy link
Contributor Author

How about

RouteContext::fromRequest($request)->getBasePath() : string

As RouteContext is final and the constructor is private, we can add a fourth argument without a BC.

    // \Slim\Routing\RouteContext
    private function __construct(
        ?RouteInterface $route,
        RouteParserInterface $routeParser,
        RoutingResults $routingResults,
        string $basePath
    ) {

But to be able to develop this method, we have to bring the base path into the \Slim\Middleware\RoutingMiddleware, as it is this middleware that actually writes the corresponding attributes to the $request object. Has anyone an idea how to do that without BC? We can't just add a third argument to the constructor, can we?

    // \Slim\Middleware\RoutingMiddleware
    public function __construct(RouteResolverInterface $routeResolver, RouteParserInterface $routeParser, string $basePath)

@l0gicgate How about using the \Slim\Routing\RouteRunner to add the base path to the $request object? As the RouteRunner gets constructed explicitely in the App constructor, we might pass the app (as route collector) to the route runner and then in the handle() method of the route runner we could write the basePath attribute into the $request object. That is a BC, but only if someone is instantiating or extending the route runner.

@l0gicgate
Copy link
Member

Need to add docs for this on Slim-Website repo

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 a pull request may close this issue.

4 participants