Flarum\Forum\Content\Discussion overriding extension settings

[pedrorezende]

Bug Report

Current Behavior Whenever I try to extend content behavior using (new Extend\Frontend('forum'))->content(MyDispatcher::class), my callback is added before Flarum\Forum\Content\Discussion. So I can't override settings like $canonicalUrl, because they always get overwritten by the core component.

Steps to Reproduce

1. Create an extension and add a callback to (new Extend\Frontend('forum'))->content(MyDispatcher::class)
2. On the __invoke function, try to set $flarumDocument->canonicalUrl value
3. Discussion pages load with a different canonical url

Expected Behavior It would be very cool if we could prioritize the order of how extensions are executed. But for this specific issue, MyComponent@__invoke should be called after Flarum\Forum\Content\Discussion@__invoke in order to proper extend the desired functionality.

Environment

• Flarum version: Flarum 0.1.0-beta.13
• Webserver: php built-in server
• Hosting environment: localhost
• PHP version: 7.4.3
• Browser: Chrome 84 on Windows 10

[clarkwinkelmann]

I agree it's a behavior that should be changed.

Right now "global" content extenders added through Extend\Frontend::content run first, then the router content callback is added last just before running Frontend\Controller with the resulting Document.

Frontend\Frontend::content() should have a way to specify a content callback priority, and Http\RouteHandlerFactory should insert the router content callback with a neutral priority so extensions can add their content callback either before or after.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We do this to keep the amount of open issues to a manageable minimum. In any case, thanks for taking an interest in this software and contributing by opening the issue in the first place!

[askvortsov1]

As per flarum/framework#1891, we want to be able to override routes. This falls under the same category. Additionally, there should be support for removing a route. I wonder if priority-based overriding is overkill though. Anyways, the way I see this, there's several potential solutions:

1. We could subclass FastRoute\GroupCountBased adding functionality to remove a route by name, that would probably require the least number of changes on our end.
2. Instead of defining routes.php as a callback, we could have it return a list of routes (in the same format as they're stored in the current route extender), and only compile the RouteCollection after all extensions are registered.

I'm currently leaning towards (2). Any thoughts?

[clarkwinkelmann]

Based on my past experience with Fastroute while working on https://kilowhat.net/flarum/extensions/custom-paths , I found its design isn't really fit for extensibility. I was completely unable to un-register routes due to the way everything is packed into strings of regexes.

A third option would be to swap Fastroute with something else. I don't think it's too tied to anything. We could try to find a router library which allows adding, removing and replacing routes more easily.

[askvortsov1]

I like that FastRoute is quite lightweight and simple. Unfortunately it's not really maintained, but I remember that I looked at some point and didn't find significantly better alternatives. I think that adding a pre-processing step where we store routes as singletons and only register them when all extensions have booted (IE option 2) would probably be the simplest option.

[luceos]

The issue of overriding sequence of content has not been solved with the linked pr.

[SychO9]

https://github.com/flarum/framework/pull/3765