Getting rid of the factory would also mean that we set a router in the component directly and do always return the same instance on $app->bootComponent('com_content')->createRouter(); (we should rename then the function to getRouter).

and do always return the same instance on $app->bootComponent('com_content')->createRouter();

yes but you wrap it in an anonymous function so it's still lazy loaded

When you create the ContentComponent then you need an instance no callback. Have a look here, where we pass now the factory. I would love to do it without a factory, I tried different things but none worked to have it in a clean way.

I need to get this one merged to finish the service conversion of com_contact, com_newsfeeds and com_categories.

This is going to give b/c breaks because you're going to be loading every components extension interface on every frontend page. For example this will cause JHtml issues etc (https://github.com/joomla/joomla-cms/blob/4.0-dev/administrator/components/com_content/Extension/ContentComponent.php#L106-L107) for the global JHtml stuff.

I think with this one we have to go to the container rather than going through the extension interface. Either that or we can't load JHtml on every boot component call or something.

I'm assuming that we have to have the factory now because otherwise we can't really distinguish JCategories instances?

// cc @mbabker for thoughts

you're going to be loading every components extension interface on every frontend page

Why? Don't get you.

Why? Don't get you.

Because as I recall the router works by running through every extension router until it finds a match?

Because as I recall the router works by running through every extension router until it finds a match?

^^ That

TBH it probably only needs the menu, especially for building routes across
applications. Not staring at code but I don’t remember the application
being too heavily used in component routers.

On Thu, Aug 30, 2018 at 5:54 PM George Wilson notifications@github.com
wrote:

@wilsonge commented on this pull request.

In components/com_content/Router/RouterFactory.php
#20339 (comment):

• {

• $this->category = $category;
  

• $this->db       = $db;
  

• }
• /**
• • Creates a router.
• • @param CMSApplicationInterface $application The application
• • @param AbstractMenu $menu The menu object to work with
• • @return RouterInterface
• • @SInCE DEPLOY_VERSION
• */
• public function createRouter(CMSApplicationInterface $application, AbstractMenu $menu): RouterInterface

If we have a SiteApplication here (we are aborting if we don't) - why
can't we use it's getMenu method? Is there ever a time we need any other
instance?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#20339 (review),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAWfoQuvfLPVzDqbwQONfFo6NV9_vzVvks5uWG07gaJpZM4T52Bw
.

--

• Michael Please pardon any errors, this message was sent from my iPhone.

Any more feedback?

Thankyou!