orchidsoftware / platform

Orchid is a @laravel package that allows for rapid application development of back-office applications, admin/user panels, and dashboards.
https://orchid.software
MIT License
4.26k stars 631 forks source link

Usage of facade aliases causes exceptions when aliases are forbidden in project. #2723

Open swayok opened 8 months ago

swayok commented 8 months ago

Describe the bug In blade templates you use aliases of facades like {{ Auth::check() }} that cause exceptions like Class "Auth" not found when project discourages usage of aliases.

To Reproduce Steps to reproduce the behavior:

  1. In config/app.php remove everything from aliases: 'aliases' => []
  2. Open /admin
  3. Get exception Class "Auth" not found

Expected behavior No exception.

Additional context In most cases aliases can be replaced by app(class) or fully qualified name of the facade. This way platform will be more independent from the project's configs and restrictions.

tabuna commented 8 months ago

Hi @swayok

Facades, such as Auth, are an important part of the Laravel framework, and it is recommended to include them to ensure a consistent user experience and guarantee functionality.

If you unintentionally removed these facades and are experiencing their absence, it is recommended to check your project's configuration file to ensure it aligns with the framework's recommendations. You can review the configuration file at this link: link to configuration file. It is recommended to keep the facades enabled.

However, if you deliberately removed the facade registration in accordance with your organization's policies or personal preferences, you can dynamically register the facades, for example, within a middleware, to make them work only in your Orchid application.

The current use of facades does not pose any problems for me or for the majority of users.

However, abandoning the use of facades would impose limitations on my role as a maintainer. Are there any significant reasons to refrain from using facades in this package? This will help me better understand the context and make an informed decision.

swayok commented 8 months ago

HI. I did not mean to remove usage of facedes. The problem is usage of aliases of facades that Laravel declares in config/app.php -> aliases. This is literally a magic flavored with magic which is very bad from the design point of view. The less magic you have in project - the more it is maintainable and compatible with other projects. In the case of a library it is very important to use less magic so the library is compatible with more projects. Most people do not remove aliases and use them but it is better to use facades directly, avoiding aliases.

I've forked a platform and managed to run it without facade aliases - all you need to change is replace \Auth:: by auth()-> helper (it was actually designed for use in templates and you can see this in Laravel docs). Laravel won't remove this helper because it is in core, so it is safe to use it instead of app('auth')-> which would be the the most direct option.

Also I've seen a problem with arguments typization. For example: you have Orchid\Access\Impersonation->loginAs(User $user) instead of Orchid\Access\Impersonation->loginAs(Autheticatable $user). The main point here is usage of the most abstract entity (class/interface) so that developer can pass any instance of class that implements interface or extends base class. I don't like mixing users and admins in same table (separation of concerns is very important here) so, I needed to pass Admin model but method does not allow this.