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

Nova resource double loading #1928

Closed
ArroWsGM opened this issue Sep 12, 2019 · 5 comments
Closed

Nova resource double loading #1928

ArroWsGM opened this issue Sep 12, 2019 · 5 comments

Comments

@ArroWsGM
Copy link

Description

When using nova packages with customizable resource controller, placing resource in App\Nova or creating it with nova:resource command causing [vuex] duplicate getter key: error and possible sidebar menu item duplication.

Steps To Reproduce

Install laravel with nova, install any package with customizable resource:

composer require optimistdigital/nova-menu-builder
php artisan vendor:publish --tag=nova-menu-builder-config
php artisan vendor:publish --tag=nova-menu-builder-migrations
php artisan migrate

Create resource

php artisan nova:resource Menu -m \OptimistDigital\MenuBuilder\Models\Menu

Adjus configuration

//config/nova-menu.php:
use App\Nova\Menu;

return [
    'resource' => Menu::class,
];

Placing resource outside of App\Nova directory solves this problem, but breaks application structure.

@davidhemphill
Copy link
Contributor

This is an issue with the package you're using. It registers its own Nova resource in addition to the one you've created.

@ArroWsGM
Copy link
Author

@davidhemphill
Yes, it does, but I think this is an issue with nova, it includes already included file. If package allows customizing it's own resource controller, it will be loaded twice. vyuldashev/nova-permission has the same issue. They both using Nova::resources method to load resource. This resource can be overridden with config and if you place this custom file into App\Nova directory, it will be loaded twice.

@davidhemphill
Copy link
Contributor

You’re saying if a third-party package loads a resource into Nova, that we should somehow and detect that when you want to use your own? That doesn’t make sense. :-) If the package wants you to be able to extend it with your own resource, it should implement the feature itself.

@ArroWsGM
Copy link
Author

Oh my. Maybe my english even worse than I thought. I try to explain one more time. We have some 3rd-party package. It loads their own resource(s) controller from own directory. For example,
nova-menu-builder:

//MenuBuilderServiceProvider.php
...
public function boot() {
    ...
    Nova::resources([
       config('nova-menu.resource', MenuResource::class),
    ]);
}
...

nova-permission:

//NovaPermissionTool.php
...
public function boot()
{
    Nova::resources([
        //this properties you can override when you register a tool with NovaServiceProvider->tools()
        $this->roleResource,
        $this->permissionResource,
    ]);
}
...

As you can see, both packages let us use custom resource class.
If I create some class outside of App\Nova directory and do all appropriate changes to attach this class to package, they still working fine at this point, but, this class is a resource controller, and to keep application structure as clean as possible, I need to place it into App\Nova directory. And if I do that, Nova automatically add this class to $resources property, and package does the same:

[2019-09-13 10:14:45] local.DEBUG: Array
(
    [0] => App\Nova\Test
    [1] => Laravel\Nova\Actions\ActionResource
    [2] => App\Nova\Test // <- duplicated element
    [3] => App\Nova\User
)

and all you needs to do is to get rid of this duplicates:

//Nova.php
public static function resources(array $resources)
{
    static::$resources = array_unique(array_merge(static::$resources, $resources));

    return new static;
}

or maybe you can find a better solution.

@Ragash
Copy link

Ragash commented Nov 23, 2019

+1 it's annoying, and if resolution is that simple it's a shame to not have built-in.
also because it would be consistent with the current steps necessary to override standard configuration of vendor's settings. @davidhemphill

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants