outl1ne / nova-settings

A Laravel Nova tool for editing custom settings using native Nova fields.
MIT License
273 stars 97 forks source link

Nova packages break when registering fields #60

Closed pepijn-vanvlaanderen closed 3 years ago

pepijn-vanvlaanderen commented 3 years ago

I ran into an issue when I updated Nova from v3.14.0 to v3.25.0 in combination with this plugin. The issue results in all scripts / styles from the nova-api route are not loading anymore (https://github.com/laravel/nova-issues/issues/3390).

It happens when registering these fields as array:

// using https://github.com/64robots/nova-fields
use R64\NovaFields\Row;
use R64\NovaFields\Text as R64Text;

\OptimistDigital\NovaSettings\NovaSettings::addSettingsFields([
    Row::make('Test row', [
        R64Text::make('Test field')
    ]),
]);

When using a callable the issue does not take place:

// using https://github.com/64robots/nova-fields
use R64\NovaFields\Row;
use R64\NovaFields\Text as R64Text;

\OptimistDigital\NovaSettings\NovaSettings::addSettingsFields(function () {
    return [
        Row::make('Test row', [
            R64Text::make('Test field'),
        ]),
    ];
});

Also when I remove the Text field inside the Row it also works, but I need the nesting in the row for my settings. The callable works fine for me as workaround, so not sure if it is a bug in this plugin or not.

Tarpsvo commented 3 years ago

I am able to reproduce this error, but I have no idea where this error comes from. It seems that the $request that reaches the ScriptsController@show method no longer has any route params (so the $this->script) is empty.

I tried my best to find the cause, but all I could conclude is that with the R64Text inside a Row inside Settings would cause the script parameter to be empty when binding NovaRequest instance in NovaCoreServiceProvider.

Here:

$this->app->afterResolving(NovaRequest::class, function ($request, $app) {
  if (!$app->bound(NovaRequest::class)) {
    // Here $request->script is empty for some reason?
    // If we remove the R64Text field, it will be defined as expected
    $app->instance(NovaRequest::class, $request);
  }
});

If the R64Text is removed, the $request->script is defined as expected and will work as intended when the @show method resolves its dependency on NovaRequest. So it basically comes down to NovaRequest being created from a Illuminate\Request instance that does not have the ->script parameter.

Anyhow, the callable workaround seems to work indeed and I think it's a valid workaround. It's more efficient to initialize the fields inside a callable anyway. :) Thanks!