Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Premature resolving of UrlGenerator breaks other extensions that register routes #1

Closed
clarkwinkelmann opened this issue Nov 4, 2019 · 1 comment · Fixed by #2
Closed

Comments

@clarkwinkelmann
Copy link
Contributor

Hello,

Following the issue reported here https://discuss.flarum.org/d/7026-linguist-customize-translations-with-ease/57 I investigated why your extension might break compatibility with Linguist.

I noticed that you resolve the UrlGenerator in the constructor of an event listener here

public function __construct(SettingsRepositoryInterface $settings, Application $application, UrlGenerator $url)

Because event listeners are instantiated right when they are registered, anything resolved in its constructor will be immediately resolved early in the forum lifecycle.

This means that the UrlGenerator is resolved at the time time your extend.php is read, which in turn resolves the route collection, which in turn prevents any extension that's read after yours to register new routes.

You should instead resolve the UrlGenerator, and possibly all other classes used in the constructor directly in the sendWebPushNotification method using $url = app(UrlGenerator::class).

This will also improve performance because those will be resolved only when your event triggers, instead of every time Flarum code runs.

@NikoVonLas
Copy link
Owner

NikoVonLas commented Nov 23, 2019

Thank you for this fix!

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 a pull request may close this issue.

2 participants