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

Bug: There are cases where the name of a named route is not determined. #8039

Closed
tomomo opened this issue Oct 13, 2023 · 3 comments · Fixed by #8040
Closed

Bug: There are cases where the name of a named route is not determined. #8039

tomomo opened this issue Oct 13, 2023 · 3 comments · Fixed by #8040
Labels
bug Verified issues on the current code behavior or pull requests that will fix them

Comments

@tomomo
Copy link

tomomo commented Oct 13, 2023

PHP Version

8.2

CodeIgniter4 Version

4.4.1

CodeIgniter4 Installation Method

Composer (using codeigniter4/appstarter)

Which operating systems have you tested for this bug?

macOS

Which server did you use?

apache

Database

No response

What happened?

There are cases where the name of a named route is not determined.
Unnamed or duplicate.

# app/Config/Routes.php
$routes->get('login', [AuthController::class, 'showLogin'], ['as' => 'loginShow']);
$routes->post('login', [AuthController::class, 'login'], ['as' => 'login']);
$routes->get('logout', [AuthController::class, 'logout'], ['as' => 'logout']);
php spark routes

CodeIgniter v4.4.1 Command Line Tool - Server Time: 2023-10-13 02:07:54 UTC+00:00

+--------+--------+-----------+--------------------------------------------+----------------+---------------+
| Method | Route  | Name      | Handler                                    | Before Filters | After Filters |
+--------+--------+-----------+--------------------------------------------+----------------+---------------+
| GET    | /      | »         | \App\Controllers\Home::index               |                | toolbar       |
| GET    | login  | loginShow | \App\Controllers\AuthController::showLogin |                | toolbar       |
| GET    | logout | »         | \App\Controllers\AuthController::logout    |                | toolbar       |
| POST   | login  | loginShow | \App\Controllers\AuthController::login     |                | toolbar       |
+--------+--------+-----------+--------------------------------------------+----------------+---------------+

"logout" cannot be named.
"login(GET or POST") has the same name.(Is this the specification?)

Steps to Reproduce

Add the following to app/Config/Routes.php

# app/Config/Routes.php
$routes->get('login', [AuthController::class, 'showLogin'], ['as' => 'loginShow']);
$routes->post('login', [AuthController::class, 'login'], ['as' => 'login']);
$routes->get('logout', [AuthController::class, 'logout'], ['as' => 'logout']);

Expected Output

Check with php spark routes etc.

php spark routes

CodeIgniter v4.4.1 Command Line Tool - Server Time: 2023-10-13 02:07:54 UTC+00:00

+--------+--------+-----------+--------------------------------------------+----------------+---------------+
| Method | Route  | Name      | Handler                                    | Before Filters | After Filters |
+--------+--------+-----------+--------------------------------------------+----------------+---------------+
| GET    | /      | »         | \App\Controllers\Home::index               |                | toolbar       |
| GET    | login  | loginShow | \App\Controllers\AuthController::showLogin |                | toolbar       |
| GET    | logout | »         | \App\Controllers\AuthController::logout    |                | toolbar       |
| POST   | login  | loginShow | \App\Controllers\AuthController::login     |                | toolbar       |
+--------+--------+-----------+--------------------------------------------+----------------+---------------+

"logout" cannot be named.
"login(GET or POST") has the same name.(Is this the specification?)

Anything else?

No response

@tomomo tomomo added the bug Verified issues on the current code behavior or pull requests that will fix them label Oct 13, 2023
@kenjis
Copy link
Member

kenjis commented Oct 13, 2023

Thank you for reporting. This seems to be a bug in the spark routes command.

Try:

--- a/system/Router/DefinedRouteCollector.php
+++ b/system/Router/DefinedRouteCollector.php
@@ -57,7 +57,7 @@ final class DefinedRouteCollector
                         $handler = $view ? '(View) ' . $view : '(Closure)';
                     }
 
-                    $routeName = $this->routeCollection->getRoutesOptions($route)['as'] ?? $route;
+                    $routeName = $this->routeCollection->getRoutesOptions($route, $method)['as'] ?? $route;
 
                     yield [
                         'method'  => $method,

@tomomo
Copy link
Author

tomomo commented Oct 13, 2023

Thank you for your prompt reply.
When I tried replacing it, I got the following result.

+--------+--------+-----------+--------------------------------------------+----------------+---------------+
| Method | Route  | Name      | Handler                                    | Before Filters | After Filters |
+--------+--------+-----------+--------------------------------------------+----------------+---------------+
| GET    | /      | »         | \App\Controllers\Home::index               |                | toolbar       |
| GET    | login  | loginShow | \App\Controllers\AuthController::showLogin |                | toolbar       |
| GET    | logout | »         | \App\Controllers\AuthController::logout    |                | toolbar       |
| POST   | login  | »         | \App\Controllers\AuthController::login     |                | toolbar       |
+--------+--------+-----------+--------------------------------------------+----------------+---------------+

I confirmed that there was a change in login (POST).
I also found out that if Route and Name are the same, the name will be the "»" symbol.
I think it's working well.

@kenjis
Copy link
Member

kenjis commented Oct 13, 2023

Thank you for checking.
I sent a PR to fix: #8040

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Verified issues on the current code behavior or pull requests that will fix them
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants