origgami / CMB2-grid

A grid system for Wordpress CMB2 library that allows columns creation
88 stars 21 forks source link

Composer autoloading issue since recent update #20

Open richardtape opened 8 years ago

richardtape commented 8 years ago

Hi!

Thanks so much for writing and maintaining this add-on. It's super useful.

We've been using it in a project for months, not as a separate plugin, but as a requirement within another plugin. Here's how we had it set up

"require": {
        "jjgrainger/wp-custom-post-type-class": "dev-master",
        "webdevstudios/cmb2": "dev-master",
        "origgami/CMB2-grid": "dev-master",
        "mustardBees/cmb-field-select2": "dev-master",
        "stomp-php/stomp-php": "4.*"
    },
    "minimum-stability": "dev",
    "autoload": {
        "psr-0": {
            "UBC": "src/",
            "UBC\\Press\\Tests": "tests/"
        },
        "files": [
            "vendor/webdevstudios/cmb2/init.php",
            "vendor/origgami/CMB2-grid/Cmb2GridPlugin.php",
            "vendor/mustardBees/cmb-field-select2/cmb-field-select2.php"
        ]
    }

However, since a recent composer update we've been getting a fatal error in the dashboard

Fatal error: Uncaught Error: Class 'Cmb2Grid\Grid\Cmb2Grid' not found in /srv/www/snip/path/wp-content/mu-plugins/ubc-press/src/UBC/Press/Metaboxes/Setup.php:206

That line (206) is $grid_layout = new \Cmb2Grid\Grid\Cmb2Grid( $section_description ); and hasn't changed.

Any ideas what has changed recently that would cause this?

jrfnl commented 8 years ago

Hi @richardtape

I'd venture a guess that this is caused by the change in PR #13 which prevents issues for people who are still running on PHP 5.2. It essentially makes the 'main' plugin file a bootstrap file which will load the 'real' plugin file. The 'real' plugin file was renamed as otherwise WP installs which have this library installed as a plugin, would break on upgrade.

I don't use Composer like that myself, but my guess would be that changing this line in your composer.json file will fix it: "vendor/origgami/CMB2-grid/Cmb2GridPlugin.php", change to: "vendor/origgami/CMB2-grid/Cmb2GridPluginLoad.php",

jrfnl commented 8 years ago

Just wondering if my suggestion solved your issue....

richardtape commented 8 years ago

Sadly it didn't, no. I tried a few different suggestions including yours and have still come up blank. I'll carry on trying and report back

jrfnl commented 8 years ago

Please do. I'd be interested to hear a solution.

Apart from the change I mentioned above, there has also been a change which implements class autoloading for the plugin. As far as I know, this shouldn't conflict with Composer, but might be worth looking into.

citelao commented 8 years ago

I am not having any problems with this.

"repositories": [
    {
        "type": "vcs",
        "url": "https://github.com/origgami/CMB2-grid"
    }
],
"require": {
    "php": ">=5.3",
    "composer/installers": "v1.0.*",
    "webdevstudios/cmb2": "^2.2",
    "origgami/CMB2-grid": "dev-master"
},
"autoload": {
    "files": [
      "vendor/webdevstudios/cmb2/init.php",
      "vendor/origgami/CMB2-grid/Cmb2GridPlugin.php",
    ]
}

Use in code:

$cmb2Grid = new \Cmb2Grid\Grid\Cmb2Grid($cmb);
                $row = $cmb2Grid->addRow();
                $row->addColumns(array($fs[0], $fs[1]));

Hopefully this helps y'all debugging.

devnix commented 8 years ago

I have the same problem on a different layout.

My plugin works perfectly in the admin, but on the login/front-end, i get this: Fatal error: Class 'Cmb2Grid\Grid\Cmb2Grid' not found in /var/www/iteaf/wp-content/plugins/unidad-movil/includes/class-ubicaciones-unidad-movil.php on line 153

My composer seems more like this:

  "autoload": {
    "classmap": ["unidad-movil.php", "includes/"],
    "psr-4" : {
      "WeDevs\\ORM\\": "vendor/tareq1988/wp-eloquent/src/"
    },
    "files": [
      "vendor/webdevstudios/cmb2/init.php",
      "vendor/origgami/CMB2-grid/Cmb2GridPlugin.php"
    ]
  },
  "repositories": [
    {
      "type": "vcs",
      "url": "https://github.com/origgami/CMB2-grid"
    }
  ],
  "require": {
    "php": ">=5.2",
    "xrstf/composer-php52": "1.*",
    "webdevstudios/cmb2": "v2.2.2.1",
    "webdevstudios/cpt-core": "^1.0",
    "origgami/CMB2-grid": "dev-master",
    "tareq1988/wp-eloquent": "dev-master"
  },
jrfnl commented 8 years ago

@DevNIX That's an unrelated issue and has to do with the Grid plugin only loading when on the back-end: https://github.com/origgami/CMB2-grid/blob/master/Cmb2GridPluginLoad.php#L19 If you think that should change (maybe with a filter to overrule the default behaviour ?), I'd suggest opening a new issue.

devnix commented 8 years ago

@jrfnl Oh LOL. Really? If CMB2 does not make that comprobation, I don't see good reasons to prevent the autoloading of the class on the frontend...

I'm going to try my own fork and make a new issue pull request. This crash really stops me from using this awesome plugin, so if the change does not get merged I'm probably going to fork the full project for my own interest.

By the way, I love the API you did there. So useful to start making a fast new admin. Thank you for your work!

jrfnl commented 8 years ago

I don't see good reasons to prevent the autoloading of the class on the frontend...

@DevNIX There are three reasons I can think off:

  1. historically - I seem to remember that CMB2 originally didn't work on the front-end either (correct me if I'm wrong).
  2. compatibility - there are quite a lot of themes which include bootstrap and loading CMB2 Grid on the front-end when one of those themes is used could lead to conflicts and unexpected layouts.
  3. lean loading - a lot gets loaded on the front-end already and only a small part or the CMB2 Grid users use it on the front-end, so always loading it on the front-end would be inefficient and would make sites slower which don't need to be.

Which is of course, why I suggested a filter.. :sunglasses:

So I could imagine that line I referred to, becoming something like:

if ( ! is_admin() && false === apply_filters( 'cmb2_grid_load_on_front', false ) )

That way you could force it load on the front-end with a simple:

add_filter( 'cmb2_grid_load_on_front', '__return_true' );

Or even better - if you need to tell CMB2 to work on the front-end via a filter (not sure, haven't look at that for a while), CMB2 Grid could just defer to that filter.

I imagine a PR like that would be very welcome (there are other open issues for the same - I just hadn't gotten around to doing it).

By the way, I love the API you did there. So useful to start making a fast new admin. Thank you for your work!

Not sure what you are referring to and if this is even meant for me, but "Thanks" ?

devnix commented 8 years ago

@jrfnl The problem is that I am not using CMB2 on the frontend. But CMB2 does not have comprobations that crash a custom plugin. Not supporting a frontend does not mean to stop autoloading the classes at all...

I highlight custom because if I decide to use CMB2-grid or not, and use it in combination with a theme using bootstrap should not be at all matter of CMB2-grid.

jrfnl commented 8 years ago

I'm not sure I completely understand your comment.

use it in combination with a theme using bootstrap should not be at all matter of CMB2-grid.

Unfortunately the reality of how themes and plugins work in WP does make it so. Saying that it shouldn't be, does not change the reality in which we work.

devnix commented 8 years ago

There is a big difference between displaying something ugly or with the wrong CSS classes and a PHP fatal error, just because we don't really know when a PHP class/namespace will be "randomly" loaded.

Unfortunately the reality of how themes and plugins work in WP does make it so. Saying that it shouldn't be, does not change the reality in which we work.

I don't get it. CMB2-grid will apply styles even if I don't invoke the code manually, for the specific case I'm developing?

The reality if you use the structure provided by WebDevStudios/generator-plugin-wp and this library is that you will get a PHP fatal error, and that seems to me like a huge bug. And I am using this library only on admin hooks...

devnix commented 8 years ago

Anyway: here is a pull request if it makes sense to the maintainers https://github.com/origgami/CMB2-grid/pull/24.

You can try my modiffication here. https://github.com/DevNIX/CMB2-grid

jrfnl commented 8 years ago

I'm still trying to understand how you are getting a fatal error if you are not using CMB2 on the front-end. Could you provide more detail so we can debug this properly ?

we don't really know when a PHP class/namespace will be "randomly" loaded.

Nothing should ever be 'randomly' loaded, that seems like a bug to me.

CMB2-grid will apply styles even if I don't invoke the code manually

CMB2-grid loads the Bootstrap styles which use quite generic CSS class names and can easily conflict with themes. See: https://github.com/origgami/CMB2-grid/blob/master/assets/css/bootstrap.css

Though to be fair - they are currently only loaded on the admin_enqueue_scripts hook.

The reality if you use the structure provided by WebDevStudios/generator-plugin-wp and this library is that you will get a PHP fatal error

I'd never come across that plugin template before, might take a look when I have some time.

devnix commented 8 years ago

I'm using CMB2-grid on the WebDevStudios Yeoman generator.

Specifically, I'm invoking it from a post type, inside the fields() method from a CPT_Core inherited class.

jrfnl commented 8 years ago

Ok, I'll take a look when I have some time.

pablo-sg-pacheco commented 8 years ago

Firstly, thank you all! I wasn't aware that so much people would be using this plugin. @jrfnl have been helping me a lot! She is really awesome. Any help from you guys is appreciated.

About the frontend issue i have some thoughts. Originally, CMB2 had only one filter for creating metabox. And it was 'cmb_init'. It loaded things on both back and frontend. So i've created my tests on that time i made this plugin using this filter and a conditional !is_admin() because I didn't think the possibility to load it on frontend. It would be a waste.

Now things changed. CMB2 now has a 'cmb2_admin_init' filter. So i think we don't need the conditional anymore. But I must confess i need to read the code again to remember.

richardtape commented 8 years ago

OK, I came back to this after a while. I managed to find out what was going on. As I'm including CMB2-Grid within another plugin which was also getting instantiated on plugins_loaded without a priority then the Cmb2Grid\init() function wasn't being called in time.

Two possible solutions to this; for now I've adjusted the priority of when I'm instantiating my bits and pieces, which is fine.

A second option is to have a custom define for this plugin. i.e.

add_action( 'plugins_loaded', '\\' . __NAMESPACE__ . '\init' ); would become something like

add_action( 'plugins_loaded', '\\' . __NAMESPACE__ . '\init', ( ( defined( 'CMB2_GRID_INIT_PRIORITY' ) ) ? CMB2_GRID_INIT_PRIORITY : '10' ); (completely untested but you get the idea). -- does a couple of things, makes it plainly obvious where things are getting instantiated and also adds a little flexibility.

Thoughts?

citelao commented 8 years ago

@richardtape I think the second option is what CMB2 does by default. You might want to make the number closer to CMB2's number (I've used something like -100) in case someone depends on CMB2. The solution's really icky IMO, but I think it's probably the best one.