Skip to content

Conversation

@joeforjoomla
Copy link
Contributor

@joeforjoomla joeforjoomla commented Oct 22, 2018

Pull Request for Issue #20339 .

Summary of Changes

Keep support for components that implement the new J4 format but don't implement the RouterServiceInterface

As of now the classic router.php is no more working and executed. Having a component in the new J4 format that doesn't implement the RouterServiceInterface is fully broken and executed without any component router inclusion. It simply creates a RouterLegacy dummy class.

@joeforjoomla joeforjoomla changed the title Update SiteRouter.php [4.0]Update SiteRouter.php Oct 22, 2018
{
$this->componentRouters[$component] = $componentInstance->createRouter($this->app, $this->menu);
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Remove tabs on lines 619, 625, 630, 637, and 641.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@laoneo
Copy link
Member

laoneo commented Oct 23, 2018

Why you want to support the old behavior in the new component structure?

@joeforjoomla
Copy link
Contributor Author

If using the new component structure, why should i be forced to have a file joomla40/administrator/components/com_content/Extension/ContentComponent.php that extend MVCComponent and implements RouterServiceInterface?

Before this PR the new component structure was more flexible and all of this was not required. I was able to have a working component having a instance of $component = new MVCComponent in the service provider

@laoneo
Copy link
Member

laoneo commented Oct 24, 2018

If your only concern is not to have your own extension class, then it would be better to introduce a base class in core, similar to the MVCComponent, which components can use. The idea behind this service provider story is that we have one way to access these services.

Before this PR the new component structure was more flexible

Actually the opposite was the case. You had to place a specific file into the root of your component with a specific class name. Otherwise it would not work. If somebody needs the router on a different place, then the same code, to load it, had to be duplicated. Really, we should start getting rid of this magical file and class loading and work with interfaces and standard ways to access services of a component.

@joeforjoomla
Copy link
Contributor Author

joeforjoomla commented Oct 24, 2018

Well long story short story i refactored my extensions in order to use this structure. Basically i would prefer a core architecture that doesn't force developers to have a component extension class if needed only for the router service and nothing else. So a base concrete class of MVCComponent that implements BootableExtensionInterface, RouterServiceInterface, etc as your proposal could be a good idea.

@laoneo
Copy link
Member

laoneo commented Oct 24, 2018

I would go instead of with a new class which extends MVCComponent as it is very likely that a component which has a router will also have MVC.

Just for the record, you can also write in your services/provider.php

$component = new class ($container->get(ComponentDispatcherFactoryInterface::class)) extends MVCComponent implements RouterServiceInterface { use RouterServiceTrait; };

@joeforjoomla
Copy link
Contributor Author

Yes correct, an anonymous class there would do the trick as well.

@laoneo
Copy link
Member

laoneo commented Oct 26, 2018

I'm going to close this one. If you want to come up with a new base class, please open a new pr.

@laoneo laoneo closed this Oct 26, 2018
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 this pull request may close these issues.

4 participants