filamentphp / filament

A collection of beautiful full-stack components for Laravel. The perfect starting point for your next app. Using Livewire, Alpine.js and Tailwind CSS.
https://filamentphp.com
MIT License
17.4k stars 2.72k forks source link

Cannot append assets after Filament's unless using `PackageServiceProvider` by Spatie #12615

Closed bilogic closed 4 months ago

bilogic commented 4 months ago

Package

filament/filament

Package Version

v3.2.73

Laravel Version

10.48.10

Livewire Version

v3.4.12

PHP Version

8.1.28

Problem description

At the core of it, the problem is:

Why does the filamentphp call goes to the static register() which does not follow Laravel's convention? This leads to a very confusing code path that one cannot follow by just reading code

This surfaced because I needed to override some CSS of easyMDE, so wanted to insert them after filament's by registering later, but was unable to do so despite trying several methods e.g $this->booted(function() {...});, static::resolved(function (AssetManager $assetManager)...

Expected behavior

There is probably some reason why \Filament\Support\Facades\FilamentAsset::register() exists, but it does not seem to follow Laravel convention. Thus, in my view:

  1. \Filament\Support\Facades\FilamentAsset::register() should not exist
  2. In FilamentServiceProvider::packageBooted(), \Filament\Support\Facades\FilamentAsset::register() should call AssetManager::register()

Or it would greatly help if someone could clarify why things work as described in my problem description

Steps to reproduce

It happens in every repository that uses filament, just load any filament page after setting XDebug breakpoints at

  1. https://github.com/filamentphp/support/blob/49637c225f9eb72380a3dc54e9fd951563955770/src/Facades/FilamentAsset.php#L37
  2. https://github.com/filamentphp/support/blob/49637c225f9eb72380a3dc54e9fd951563955770/src/Assets/AssetManager.php#L46

Reproduction repository

n/a

Relevant log output

No response

danharrin commented 4 months ago

We need to do that otherwise calls made to the facade BEFORE the singleton is registered get lost, since they are overridden by the singleton registration.

bilogic commented 4 months ago

@danharrin

Thanks, but I'm still confused as to why

  1. my ServiceProvider ran first and led to AssetManager::register() (meaning the singleton has been registered)
  2. FilamentServiceProvider ran next but still called the FilamentAsset facade's static register()

Do you know what can be causing this? Is there a way to inject CSS after filament's? I have exhausted all means and the only way was to include it in my custom field's blade.

danharrin commented 4 months ago

So you have no problem injecting before Filament's, is there a reason you want it after? Specificity?

bilogic commented 4 months ago

Alright, there are 2 issues here

  1. Yes, CSS specificity and I have found a workaround by using @assets
  1. my ServiceProvider ran first and led to AssetManager::register() (meaning the singleton has been registered)
  2. FilamentServiceProvider ran next but still called the FilamentAsset facade's static register()
  1. This is the bigger problem, as it does not follow Laravel convention and debugging it was exasperating. For whatever reason, FilamentServiceProvider decided to call the static method even though the singleton has already been registered. I still have no answer as to how and why that happened.
danharrin commented 4 months ago

Will probably need some extra checks to see if the singleton has been resolved before registering another resolving() callback

bilogic commented 4 months ago

If I may, I do not think filament needs to have static methods in a facade to achieve its aims, removing them will make behavior more standardized and easier to debug.

danharrin commented 4 months ago

Removing them will cause breaking changes for asset registration in register().

danharrin commented 4 months ago

It's something we will look at dealing with in v4 but its not possible in v3.

bilogic commented 4 months ago

Yea, v4 is good enough for me. Thanks!