I would not deprecate it, Web APP still should have it, like it have a Document.
Just add ServiceProvider, and if someone need a specific Router he will pick it from there (like in Route::link()).

Another thing, introducing SiteRouterInterface looks a bit not-intuitive to me, what purpose of it?
I would expect then AdministratorRouterInterface, ApiRouterInterface, CliRouterInterface etc

I would not deprecate it, Web APP still should have it, like it have a Document. Just add ServiceProvider, and if someone need a specific Router he will pick it from there (like in Route::link()).

I agree. Just like Document, Session, etc for consistency it should not be deprecated. Otherwise everything should be deprecated. App still should have everything as a shortcut to the container.

The app doesn't have a router instance, all it does it forwards the request to Router::getInstance. There is no reason to have a delegating function in the app. So all of this should be deprecated and the router should be injected wherever possible. On a next step the Route class should be deprecated and routing should actually go over the router only which is the available in all views.

I Made only the SiteRouter interface for now as this is what actually mathers if we need other's then we can do them on demand.

I agree. Just like Document, Session, etc for consistency it should not be deprecated. Otherwise everything should be deprecated. App still should have everything as a shortcut to the container.

The app should not be a god object with tons of references. Modern apps inject the stuff which are needed. Especially for the router where the router has a reference to the app but not vice versa. And the container should not be abused as a service locator in the application. This is an anti pattern.

I have tested this item ✅ successfully on 51cfca4

I have tested this item ✅ successfully on f6908c0

rtc

@laoneo This requires another test after the new commit.

@BPBlueprint Could you redo your test again?

@roland-d all I did was adding a try/catch block around, nothing more f6908c0

Thanks everybody.

Pinging @laoneo

Actually this PR is not fully b/c and reliable, given than:

Factory::getContainer()->get('SiteRouter'):

is not the same than:

$this->app::getRouter('Site');

I've noticed that they are 2 separate and different instances of the SiteRouter class.

So given for example that the Joomla core uses the container: Factory::getContainer()->get('SiteRouter'):
but a system plugin still uses $this->app::getRouter('Site'); to attach some build/parse rules:

$router->attachParseRule ( array (
		$this,
		'preProcessParseRule'
), \Joomla\CMS\Router\Router::PROCESS_BEFORE );

The result will be that the preProcessParseRule function of this plugin will never be called.

This implication has potential drawbacks, not sure how it can be solved other than update the code of all extensions to the new container to work on the same instance of the router class.

There is an alias missing for the old class names. Can you give #37381 test?