Skip to content

Conversation

@guanguans
Copy link
Contributor

Hello, I really like the features of this project. However, it requires manually registering the routing service provider. This commit solves that problem.

@andrey-helldar andrey-helldar added the major Breaking changes label Apr 30, 2025
Copy link
Member

@andrey-helldar andrey-helldar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is also worth considering that the 1st version of the project works with older versions of Laravel, starting from 7.0.

The booting method did not exist in that version.

I see two ways of events development:

  1. modify your code so that it supports older versions of Laravel and release a new version as part of the 1st - 1.7.0.
  2. remove support for those versions of Laravel where the booting method does not exist in the service provider and leave your code. Release the new version as a major change - 2.0.0.

Based on the situation and current time, the best solution for me is the second option, so I suggest:

  1. update the code from the main branch to your project;
  2. detect those Laravel versions that do not have the booting method and remove them from composer.json, .github/workflows/phpunit.xml;
  3. Raise the minimum allowed version of PHP to 8.2.

@andrey-helldar andrey-helldar changed the title Extend router and remove RoutingServiceProvider Removed support for older versions of Laravel Apr 30, 2025
@guanguans
Copy link
Contributor Author

guanguans commented Apr 30, 2025

It is also worth considering that the 1st version of the project works with older versions of Laravel, starting from 7.0.

The booting method did not exist in that version.

I see two ways of events development:

  1. modify your code so that it supports older versions of Laravel and release a new version as part of the 1st - 1.7.0.
  2. remove support for those versions of Laravel where the booting method does not exist in the service provider and leave your code. Release the new version as a major change - 2.0.0.

Based on the situation and current time, the best solution for me is the second option, so I suggest:

  1. update the code from the main branch to your project;
  2. detect those Laravel versions that do not have the booting method and remove them from composer.json, .github/workflows/phpunit.xml;
  3. Raise the minimum allowed version of PHP to 8.2.

@andrey-helldar

Sorry. I used ServiceProvider::booting() because I wanted to extend the Router as early as possible, but I didn't realize that Laravel 7 doesn't have this method. Now I using Application::booting() instead, and the tests have already passed.

Related Update

https://github.com/guanguans/laravel-route-names/blob/main/src/ServiceProvider.php#L46

    protected function extendRouter(): void
    {
-       $this->booting(function (): void {
+       $this->app->booting(function (): void {
            $this->app->extend(
                'router',
                static fn(
                    BaseRouter $router,
                    $app
                ): Router => !$router instanceof Router ? new Router($app['events'], $app) : $router
            );
        });
    }

…iceProvider

- Remove the `RoutingServiceProvider` class to simplify the codebase and centralize routing logic.
- Extend the router directly in `ServiceProvider` to ensure custom `Router` instantiation when necessary.
- This change reduces redundancy and improves maintainability of the routing setup.
@andrey-helldar andrey-helldar changed the title Removed support for older versions of Laravel Extend router and remove RoutingServiceProvider Apr 30, 2025
@andrey-helldar andrey-helldar added minor Non-critical changes to the project structure and removed major Breaking changes labels Apr 30, 2025
Copy link
Member

@andrey-helldar andrey-helldar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@andrey-helldar andrey-helldar merged commit aa6390c into TheDragonCode:main Apr 30, 2025
22 of 23 checks passed
@guanguans
Copy link
Contributor Author

guanguans commented Apr 30, 2025

@andrey-helldar

Sorry, I just found out that there is a problem with this PR. When I updated to the latest version(1.7.0), all requests returned a 404 response. I am looking into the cause...

@andrey-helldar
Copy link
Member

@guanguans I stopped using this project a long time ago in my applications, so I can only rely on tests.

I'm currently working on removing support for older versions of Laravel (only L10+ will be supported) and will also add Workspace to make it easier to test changes. This will be available in the coming hours.

I have also canceled the releases of versions 1.8.0 and 1.7.0.

@andrey-helldar
Copy link
Member

@guanguans Corrected a mistake. Still this service provider is needed and it should be loaded at application startup.

For correct work it is necessary to replace the initialization of class Application. I have separated this notification into a more noticeable block on the README page.

https://github.com/TheDragonCode/laravel-route-names/releases/tag/2.0.0

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

minor Non-critical changes to the project structure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants