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

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

Closed
bilogic opened this issue May 4, 2024 · 9 comments
Labels
bug Something isn't working low priority unconfirmed

Comments

@bilogic
Copy link

bilogic commented May 4, 2024

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

@bilogic bilogic added bug Something isn't working low priority unconfirmed labels May 4, 2024
@bilogic bilogic changed the title Cannot append js after FilamentServiceProvider's FilamentAsset::register() Cannot append assets after Filament's unless using PackageServiceProvider by Spatie May 4, 2024
@danharrin
Copy link
Member

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.

@danharrin danharrin closed this as not planned Won't fix, can't repro, duplicate, stale May 4, 2024
@bilogic
Copy link
Author

bilogic commented May 4, 2024

@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
Copy link
Member

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

@bilogic
Copy link
Author

bilogic commented May 6, 2024

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
Copy link
Member

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

@bilogic
Copy link
Author

bilogic commented May 6, 2024

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
Copy link
Member

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

@danharrin
Copy link
Member

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

@bilogic
Copy link
Author

bilogic commented May 6, 2024

Yea, v4 is good enough for me. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working low priority unconfirmed
Projects
Status: Done
Development

No branches or pull requests

2 participants