-
Notifications
You must be signed in to change notification settings - Fork 346
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
v1.8.2 caused package/config/routes.php to stop registering with the router automatically #2142
Comments
Packages are backend code, they should not contain routes? |
Fair, totally understand if this doesn't deserve attention. It's been a while... either the docs matured or, more likely, my understanding of Fuel was just incomplete at the time. If anyone else out there painted outside the lines and your package's routes break - the workaround above is a cheap fix, otherwise convert it to a module. |
Any idea what exactly broke it? And what it the definition of "broken"? |
The package in use here was to add onelogin/php-saml capability to our project. The package's routes were anonymous functions that executed an adapter class method. Previously fuelphp would automatically load the config/routes.php from the package routes and add them to the application. The change caused those routes to no longer get registered with the router. Hence breaking our SAML integration. |
Related to 7fc6286#diff-abf8fc3534cc67ebd9af340c3e9b36d0 ? |
Correct, previous version was 1.8.1.6. I applied just these changes to fuel.php and the routes in a the package stopped automatically loading. |
Ok. Understandable, this moves loading the routes to before packages are autoloaded. |
sry, for beeing late to the issue but don't mess with the router logic in the core! so, if you need any router logic and still separate the code from the main app, put it into a module! reference: https://fuelphp.com/forums/discussion/15217 |
Then you did it wrong. Packages are for backend extensions, modules are for frontend extensions. From which follows that packages should never contain routes. |
We've been lagging behind in updating, so I apologize for the tardiness. The following commit 7fc6286 causes any packages that were previously using a
config/routes.php
to create routes to break.Reading the docs, perhaps this wasn't an intended feature in the first place?
I'm mostly making a comment here to make the maintainers aware of the change and leave a trail for others that may have the same issue.
Our current workaround is to use the package's bootstrap.php file to load the config and manually add it's routes to the Fuel Router:
This does change the order that the routes are registered (package routes came after app routes before), which may which pattern matches first in the router?
The text was updated successfully, but these errors were encountered: