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

Basepath concatenation could be improved #154

Open
KernelDeimos opened this issue Oct 23, 2017 · 3 comments
Open

Basepath concatenation could be improved #154

KernelDeimos opened this issue Oct 23, 2017 · 3 comments

Comments

@KernelDeimos
Copy link

Currently, basepath is simply concatenated with the path regex used in a route.

This can lead to confusing results when, say, basepath is websites/ and the route is /testpage, since Aura.Router currently will concatenate this to websites//testpage.

I have implemented a fix for this on my fork as follows:

// Trim leading and following slashes from basepath
$basePart = trim($this->basepath, '/');

// Trim leading slash from route path
$routePart = ltrim($this->route->path, '/');

// Join basepath and route with forwardslash
$this->regex = '/' . implode('/', array($basePart, $routePart));

This works great, but I found out it's too lenient. When I ran phpunit I noticed it breaks 12 test cases :/

I'll try to figure out a fix that doesn't cause this issues, but in the meantime I'll post this issue in case I forget.

@KernelDeimos
Copy link
Author

KernelDeimos commented Oct 24, 2017

OH.... This is very complicated to do because the route regex may or may not match a leading slash depending on the route the user enters.

@ncou
Copy link

ncou commented Jun 22, 2019

some router use this code on the basePath :
$this->basePath = rtrim($basePath, '/');

@harikt
Copy link
Member

harikt commented Jan 26, 2022

Hi,

I am sorry for being late. Just a quick question is this still a problem or have a fix ?

We are in the process of making 4.x . In case this is an issue please report back. Else do close it.

Thank you.

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

No branches or pull requests

3 participants