lara-zeus / sky

CMS for your website. it include posts, pages, tags, and categories. with a frontend scaffolding ready to use
https://larazeus.com/sky
MIT License
136 stars 25 forks source link

[Bug]: Cannot use Closure for fields with `itemType()` #196

Closed mallardduck closed 6 months ago

mallardduck commented 6 months ago

What happened?

I cannot currently use a Closure as the fields value when calling SkyPlugin::make()->itemType(). There are many reasons someone might want to customize a Form field using a closure as documented in filament docs. When I attempt doing this I get the following error:

Filament\Forms\Components\Component::getChildComponents(): Return value must be of type array, Closure returned

Flareapp trace of error: https://flareapp.io/share/x5MW6QoP

How to reproduce the bug

When I register SkyPlugin using custom itemType with a closure for fields (which Filament supports):

                SkyPlugin::make()
                    ->itemType(
                        name: "Test",
                        fields: function(): array {
                            return [
                                Select::make('test')
                                    ->label("Test")
                                    ->options([
                                        '' => "placeholder",
                                        'test' => "test"
                                    ])
                                    ->default('')
                                    ->selectablePlaceholder(false),
                            ];
                        },
                        slug: 'test'
                    ),

Package Version

3.X

PHP Version

8.1

Laravel Version

10.x

Which operating systems does with happen with?

macOS, Windows, Linux

Notes

Cannot sort out yet if this is a filament issue, or a Sky issue. Leaning towards sky issue related to how we're reusing the Filament Form Group - but could totally be something else upstream in filament I guess.

atmonshi commented 6 months ago

ya I think I am allowing to pass a closure but never evaluate it

can you test this for me see if works? in the Configuration class

public function itemType(string $name, array | Closure $fields, ?string $slug = null): static
    {
        $this->itemTypes[$slug ?? Str::slug($name)] = [
            'name' => $name,
            'fields' => $this->evaluate($fields),
        ];

        return $this;
    }

also dont remember if there is other places need to change too

mallardduck commented 6 months ago

so I gave this a try and while it did fix the error, it executes at a different time than expected. for instance, the specific new "itemType" i'm trying to add is "Local Route" - one that lists public (only having web middleware) routes not needing route parameters to create links from.

Initially I was hacking this up as a PR to submit to sky and it seemed to be working as I expected there. But then I realized that was overkill since the itemType method existed, so started to port it into my project.

the gist is that when I had the code in Sky it gets executed a lot "closer" to when the user clicks the "Add Link" button - so it properly can see all the apps routes. But when I moved it into itemType via my project it seems to run "at Panel configuration" time which runs before all the routes are registered.

mallardduck commented 6 months ago

Oh wait - I got it now. Instead of evaluating the fields "at configure time", we just do that "at call time" within getItemTypes and get the same fix with more of the app state online when evaluated. Edit: on third look, you already cover other instance of this in HandlesNavigationBuilder::class, so that is most appropriate for consistency.