FriendsOfSymfony1 / symfony1

[DEPRECATED -- Use Symfony instead] Fork of symfony 1.4 with DIC, form enhancements, latest Swiftmailer, better performance, composer compatible and PHP 8 support
https://symfony.com/legacy
MIT License
337 stars 175 forks source link

[WIP][RFC] improve plugin loading with `symfony1-plugin` typed composer packages #342

Open connorhu opened 3 months ago

connorhu commented 3 months ago

For Issue #341, I looked into what is needed to put sfDoctrinePlugin in a separate package and not have it shipped with the base framework (e.g. propel orm doesn't need this plugin). I don't really like the solution of the other fos1 plugin packages. What happens there is that composer-installer 1.0 knows about symfony1-plugin and copies the installed package into the appropriate plugins directory. This is not very practical to do in the composer era. Let's take a look at what to do:

This change would get rid of package codes (which don't work anyway: https://github.com/orgs/FriendsOfSymfony1/discussions/333#discussion-6326793 ), pear codes, and should be default the composer installation way of the framework and the plugins.

What other improvements could be considered?

Files to remove

Deprecated commands/tasks

Deprecated methods

alquerci commented 3 months ago

Good idea.

Another idea without composer dependency. But using ReflectionClass.

It avoids plugin duplication

we should not allow composer/installers 1.0 to be in the dependencies, because this will duplicate the plugins

If a plugin is installed with composer, it is also autoloaded. Then

class ProjectConfiguration extends sfProjectConfiguration
{
    public function setup()
    {
        $this->enablePlugins([
            \Some\Vendor\SomePluginConfiguration::class,
        ]);
    }
}

where

    public function getPluginPaths()
    {
        if (!isset($this->pluginPaths[''])) {
            $pluginPaths = $this->getAllPluginPaths();

            $this->pluginPaths[''] = [];
            foreach ($this->getPlugins() as $plugin) {
                if (isset($pluginPaths[$plugin])) {
                    $this->pluginPaths[''][] = $pluginPaths[$plugin];
                } else {
                    // START new code
                    try {
                        $pluginConfigurationClass = $plugin;
                        $pluginReflection = new ReflectionClass($pluginConfigurationClass);

                        $classFile = $pluginReflection->getFileName();

                        // to be compatible with loadPlugins()
                        // require_once '${pluginPath}/config/${pluginName}Configuration.class.php'; be noop
                        $pluginName = $this->extractBaseClassWithoutConfigurationSuffix($pluginConfigurationClass);
                        $pluginPath = $this->extractPluginDirectory($classFile);

                        $this->pluginPaths[''][] = $pluginPath;
                        $this->setPluginPath($pluginName, $pluginPath);
                    } catch (ReflectionException) {
                         throw new InvalidArgumentException(sprintf('The plugin "%s" does not exist.', $plugin));
                    }
                    // END new code
                }
            }
        }

        return $this->pluginPaths[''];
    }
connorhu commented 3 months ago

I have nothing against relying heavily on the composer. It has become a quasi-industry standard. Currently, the code relies heavily on an outdated package management system (pear). This code won't live to see composer become obsolete, so integrating it won't be a problem.