artkonekt / appshell

AppShell is an Laravel Application boilerplate with UI, ACL, Users & Customers
https://konekt.dev/appshell
MIT License
80 stars 10 forks source link

Unknown error for ExtendsIconThemes #24

Closed huiyang closed 3 years ago

huiyang commented 3 years ago

After update vanilo/framework and other vanilo packages, it show this error whenever i run php artisan:

Error Class name must be a valid object or a string at C:\wamp64\www\ant\celectric\vendor\konekt\appshell\src\Traits\ExtendsIconThemes.php:26

https://github.com/artkonekt/appshell/blob/e8b53f5fe75cb81bf2bc9ebcbbcc171f31ddab18/src/Traits/ExtendsIconThemes.php#L26

1 C:\wamp64\www\ant\celectric\vendor\vanilo\framework\src\Providers\ModuleServiceProvider.php:123 Vanilo\Framework\Providers\ModuleServiceProvider::registerIconExtensions()

2 C:\wamp64\www\ant\celectric\vendor\laravel\framework\src\Illuminate\Container\BoundMethod.php:36 Vanilo\Framework\Providers\ModuleServiceProvider::boot()

Debug

First, i tried to clear all caching, but the error is still there.

I dd() the: Value for $class, it is null, causing the error Value for $id, it is "zmdi"

I tried to add a throw new Exception(''); inside Konekt\AppShell\IconThemes::add() but no exception is thrown. So, I have no idea where is those icons is added/registered?

Should i fix the issue by simply add a isset check? haha

//  Konekt\AppShell\Traits\ExtendsIconThemes
if (isset($class)) {
    $class::extend($abstract, $concrete);
}

Packages version:

vanilo/cart 2.1.1 vanilo/category 2.1.0 vanilo/channel 2.1.0 vanilo/checkout dev-master 8a60e58 vanilo/contracts 2.1.0 vanilo/framework dev-master 9b7a824 vanilo/order 2.2.0 vanilo/payment 2.1.0 vanilo/product 2.1.0 vanilo/properties 2.1.1 vanilo/shipping dev-shipping-method ab883d2 vanilo/support 2.1.1

konekt/acl 1.5.0 konekt/address 2.1.0 konekt/appshell 2.1.0 konekt/concord 1.10.0 konekt/customer 2.1.0 konekt/enum 3.1.0 konekt/enum-eloquent 1.7.0 konekt/gears 1.7.0 konekt/laravel-migration-compatibility 1.4.1 konekt/menu 1.8.0 konekt/user 2.3.0

fulopattila122 commented 3 years ago

It looks like the IconThemes don't get registered. Does this: method get called? https://github.com/artkonekt/appshell/blob/e8b53f5fe75cb81bf2bc9ebcbbcc171f31ddab18/src/Providers/UiServiceProvider.php#L39 ?

What's the output of

php artisan concord:modules -a

?

huiyang commented 3 years ago
  1. It don't enter the register() of appshell/src/Providers/UiServiceProvider.php
  2. Here is the output for php artisan concord:modules -a

image

fulopattila122 commented 3 years ago

Got it, AppShell needs to be added to Concord.

Add it to config/concord.php, before Vanilo, for an example see: https://vanilo.io/docs/2.x/installation#with-composer

<?php

return [
    'modules' => [
        Konekt\AppShell\Providers\ModuleServiceProvider::class => [
            'ui' => [
                'name' => 'Vanilo',
                'url' => '/admin/product'
            ]
        ],
        Vanilo\Framework\Providers\ModuleServiceProvider::class
    ]
];
fulopattila122 commented 3 years ago

:information_source: Side note: Concord was created back in the days of the Laravel 5.4 era, when package auto-discovery wasn't yet present. We plan to clean up Concord a bit and utilize Laravel Package auto-discovery in the next major version of Concord. But that's not going to happen sooner than 2021 Q3-Q4

huiyang commented 3 years ago

Oh i see, but actually last time I disabled it because it conflict with caffeinated/menus package (as both of them used app('menu') ). I used vanilo backend but didn't use vanilo interface/ui, I used fusioncms/cms for interface/ui.

But anyway, never mind, i will temporarily fix the issue using solution i mentioned above. adding isset($class) checking, haha.

Thanks anyway.

fulopattila122 commented 3 years ago

This works for the moment, but it makes your app undeployable. As a proper solution, I suggest to remove vanilo/framework and add all the vanilo modules to composer & to concord one by one

First remove Framework and AppShell from config/concord.php

One liner:

composer remove vanilo/framework konekt/appshell --no-update && composer require vanilo/contracts vanilo/support vanilo/category vanilo/product vanilo/cart vanilo/checkout vanilo/order vanilo/properties vanilo/payment

Then add the modules to config/concord.php:

'modules' => [
        Vanilo\Category\Providers\ModuleServiceProvider::class   => [],
        Vanilo\Product\Providers\ModuleServiceProvider::class    => [],
        Vanilo\Properties\Providers\ModuleServiceProvider::class => [],
        Vanilo\Cart\Providers\ModuleServiceProvider::class       => [],
        Vanilo\Checkout\Providers\ModuleServiceProvider::class   => [],
        Vanilo\Order\Providers\ModuleServiceProvider::class      => [],
        Vanilo\Payment\Providers\ModuleServiceProvider::class    => [],
    ],

You may want to use the model extensions of the framework, so check them: https://github.com/vanilophp/framework/tree/2.1.1/src/Models

If any of these extensions are of your interest, simply copy them to your app (change the namespace) and override the model in your AppServiceProvider class, similarly to this: https://github.com/vanilophp/framework/blob/2.1.1/src/Providers/ModuleServiceProvider.php#L105

That's the proper approach, otherwise, the Framework and the admin module will be bothering you even in the future.

Please let me know if it worked properly (I tested the steps, but it's just "my machine" :wink: )

huiyang commented 3 years ago

Thanks! Let me try.