search

mostlyserious / craft-promptly

Revolutionize content creation in Craft by leveraging the power of AI, giving superpowers to your content creators to help them brainstorm and edit content with an AI assistant directly within Craft CMS.
Other
0 stars 1 forks source link

Latest commit breaks installer because of declaration conflict #5

Open ddnetters opened 4 months ago

ddnetters commented 4 months ago

The latest commit aimed at fixing migrations breaks the installer: https://github.com/mostlyserious/craft-promptly/commit/3e8b370feb29be5a365a6272ce8ff428cfd3224c. When installing the plugin, the following exception gets thrown:

*** installing promptly
PHP Compile Error 'yii\base\ErrorException' with message 'Cannot declare class MostlySerious\Promptly\Migrations\Install, because the name is already in use'

in /var/www/html/vendor/mostlyserious/craft-promptly/src/migrations/Install.php:0

Stack trace:
#0 [internal function]: yii\base\ErrorHandler->handleFatalError()
#1 {main}

This happens because CraftCMS calls requires_once on the default Install migration of a plugin (https://github.com/craftcms/cms/blob/develop/src/base/Plugin.php#L317). But because the Plugin class of promptly already declares the class by requiring it, the exception gets thrown. It goes through the following flow:

  1. A user runs php craft plugin/install promptly
  2. Craft boots
  3. Craft loads the Plugin class of promptly
  4. The class requires MostlySerious\Promptly\Migrations\Install
  5. Craft class Plugin::install()
  6. In the call stack the function runs require_once "/var/www/html/vendor/mostlyserious/craft-promptly/src/migrations/Install.php"; (https://github.com/craftcms/cms/blob/develop/src/base/Plugin.php#L317)
  7. PHP compiler crashes because the class was already declared

That's also why the exception only happens on a fresh install.

A quick solution would be to just extract all code in MostlySerious\Promptly\Migrations\Install::safeUp(); to some static helper class so you can call that in your Plugin.php and Install doesn't get declared again.

corneliusio commented 4 months ago

@ddnetters Thanks for calling this out. Craft's way of handling install migrations is basically made to not make sense. I've reverted this change and made the assumption that the only reason the table wasn't installing before is that the migrations directory was capitalized and Pixel & Tonic apparently hate PSR-4 standards. ¯_(ツ)_/¯

Let me know if you have any issue with the most recent update. This did install as expected for me on a completely fresh install so I have to believe it's fixed now, ha.