craftcms / cloud-extension-yii2

0 stars 1 forks source link

[Cloud]: Issue with `craftcms/cloud` and Twig global deprecation #46

Closed khalwat closed 3 months ago

khalwat commented 3 months ago

What happened?

Description

This change adds a deprecator to the isCraftCloud global, which on the surface makes sense, since you're moving it elsewhere.

The problem is that the function is accessed immediately whenever the Twig environment is initialized:

    public function getGlobals(): array
    {
        return [
            'cloud' => new CloudVariable(),
            'isCraftCloud' => $this->isCraftCloud(),
        ];
    }

...which then invokes the deprecator:

    /**
     * @deprecated in 1.4.8
     */
    public function isCraftCloud(): bool
    {
        Craft::$app->getDeprecator()->log(
            __METHOD__,
            'The `isCraftCloud` Twig global has been deprecated. Use `cloud.isCraftCloud` instead.',
        );

        return Helper::isCraftCloud();
    }

So you're going to get a deprecation notice every time, whether your templates or other code ever access the isCraftCloud global variable.

This is relatively minor (although incorrect/unintended), except that if you have this in your config/app.php:

        'deprecator' => [
            'throwExceptions' => App::env('DEV_MODE'),
        ],

...and in that case, any time anything uses Twig, it's going to throw an exception. The only recourse is to uninstall the craftcms/cloud package, or disable deprecator.throwExceptions

Sidenote:

I don't know where else to report this, because if I click the link here on GitHub to report an issue for Craft Cloud, I'm sent to a generic contact form which doesn't seem appropriate for a technical issue specifically with the cloud-extension-yii2 (but maybe it is?)

Also, the craftcms/cloud extension is oddly aliased to the craftcms/cloud-extension-yii2, and neither of them has an Issues tab enabled, so I can't submit an issue there either (and there is no CHANGELOG.md to see the changes from version to version, which can be helpful).

Finally, the documentation still says to use isCraftCloud and should probably be updated.

Steps to reproduce

  1. Have craftcms/cloud installed
  2. See that a deprecation notice is logged for every request, regardless of whether the isCraftCloud Twig global is accessed from any templates

Worser:

  1. Have craftcms/cloud installed
  2. Have deprecator.throwExceptions = true
  3. See that an exception is thrown any time Twig is used for anything, rendering the CP and frontend unusable

Expected behavior

Deprecation notices/exceptions would only be logged/thrown when the global is actually accessed.

Actual behavior

Deprecation notices/exceptions are logged/thrown on every request, regardless of whether isCraftCloud is accessed from a Twig template.

Craft CMS version

4.x, 5.x

PHP version

n/a

Operating system and version

n/a

Database type and version

n/a

Image driver and version

n/a

Installed plugins and versions

timkelty commented 3 months ago

Thanks @khalwat – For now I've removed the deprecator log, just leaving the @deprecated comment.

Updated in 2.3.2 and 1.49.2

I don't know where else to report this…

I've enabled issues on https://github.com/craftcms/cloud-extension-yii2 and we will update issue templates to direct there.

and there is no CHANGELOG.md to see the changes from version to version, which can be helpful

For now, release notes is the best place to find that info, but that will likely change once Cloud in in GA.

Finally, the documentation still says to use isCraftCloud and should probably be updated.

Done!

linear[bot] commented 3 months ago

PT-1784 [Cloud]: Issue with `craftcms/cloud` and Twig global deprecation