nystudio107 / craft-autocomplete

Provides Twig template IDE autocomplete of Craft CMS & plugin variables
MIT License
42 stars 4 forks source link

Fix “Twig instantiated” error #12

Closed bencroker closed 1 year ago

bencroker commented 1 year ago

Fixes an issue in which the plugin was instantiating Twig before Craft was fully initialized, causing errors in logs and console commands.

Screenshot 2023-07-02 at 08 29 54
khalwat commented 1 year ago

Looks good, but should the event be specific to the CraftWebApp? I'm thinking it should be generic to any CraftApp (console, too)?

khalwat commented 1 year ago

eh, I'll just merge it in and fix it.

khalwat commented 1 year ago

thx!

khalwat commented 1 year ago

Posting here for completeness.

In the plugin scaffolding from the Craft generator it does this:

        // Defer most setup tasks until Craft is fully initialized
        Craft::$app->onInit(function () {
            $this->attachEventHandlers();
            // ...
        });

which does this:

    public function onInit(callable $callback): void
    {
        if ($this->_isInitialized) {
            $callback();
        } else {
            $this->on(WebApplication::EVENT_INIT, function() use ($callback) {
                $callback();
            });
        }
    }
``` but doesn't this mean that for console requests, this setup will just never happen if the app isn't fully initialized yet? Because it only ever triggers that event if it's a `WebApplication` (from the trait): ```php
        // Fire an 'init' event
        if ($this->hasEventHandlers(WebApplication::EVENT_INIT)) {
            $this->trigger(WebApplication::EVENT_INIT);
        }

I'm asking because we have an auto-bootstrapping Yii2 module that's trying to do stuff after Craft is fully initialized & ready to go... but it also has a console command aspect to it, and it's making me think this will not work as intended in that case.

khalwat commented 1 year ago

So as it turns out...

        // Fire an 'init' event
        if ($this->hasEventHandlers(WebApplication::EVENT_INIT)) {
            $this->trigger(WebApplication::EVENT_INIT);
        }

in the ApplicationTrait will trigger the event when when $this is a craft\console\Application as well. That's because events are just constants, and you can add whatever events you want from whatever class, whether they are defined there or not.

khalwat commented 1 year ago

Even though I understand now why it doesn't make a practical difference, it would have made more logical sense to me if the event constant was declared in the Trait (yes, I know, not until PHP 8.2) or if there was a craft\base\Application instead of (or in addition to) the craft\base\ApplicationTrait where the constant (and some other code from the Trait) could live.

But whatever ¯_(ツ)_/¯