Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removed duplicate ConfigProvider from config/config.php #166

Open
wants to merge 2 commits into
base: 3.16.x
Choose a base branch
from

Conversation

bidi47
Copy link

@bidi47 bidi47 commented Jan 23, 2025

Q A
Documentation no
Bugfix yes
BC Break no
New Feature no
RFC no
QA no

Description

This PR fixes a bug in issue #162 by removing the duplicate ConfigProvider in config/config.php

@froschdesign
Copy link
Member

@bidi47

Added ConfigProvider with full namespace for Mezzio\Helper…

The duplication is still present because the component installer automatically installs and injects the helpers:

"extra": {
"laminas": {
"component-whitelist": [
"mezzio/mezzio",
"mezzio/mezzio-helpers",
"mezzio/mezzio-router",
"laminas/laminas-httphandlerrunner"
]
}
},

See: https://docs.laminas.dev/laminas-component-installer/#marking-packages-to-auto-install
(component-whitelist is also supported)

@froschdesign
Copy link
Member

Please remove the entry from config.php and then please test it. Unfortunately, I can't test it at the moment. (Ignore the unit test for now.)

@bidi47
Copy link
Author

bidi47 commented Jan 23, 2025

@froschdesign, I see what you mean about the injections based on component-whitelist, but I tested locally on my end and found nothing to be duplicated. Apparently, the injection ignores \Mezzio\Helper\ConfigProvider::class if it's added to the aggregator with full namespace. Before, it was added with a use.

What I did to test:

  • I ran composer create-project mezzio/mezzio-skeleton . in a new folder
  • Before submitting anything, I edited config/config.php with the update I commited here
  • I went through the installation process which completed successfully
  • I ran composer serve to see the homepage

This is my config/config.php after the installation

<?php

declare(strict_types=1);

use Laminas\ConfigAggregator\ArrayProvider;
use Laminas\ConfigAggregator\ConfigAggregator;
use Laminas\ConfigAggregator\PhpFileProvider;

// To enable or disable caching, set the `ConfigAggregator::ENABLE_CACHE` boolean in
// `config/autoload/local.php`.
$cacheConfig = [
    'config_cache_path' => 'data/cache/config-cache.php',
];

$aggregator = new ConfigAggregator([
    \Mezzio\Twig\ConfigProvider::class,
    \Mezzio\Tooling\ConfigProvider::class,
    \Mezzio\Router\FastRouteRouter\ConfigProvider::class,
    \Laminas\HttpHandlerRunner\ConfigProvider::class,
    // Include cache configuration
    new ArrayProvider($cacheConfig),
    \Mezzio\Helper\ConfigProvider::class,
    \Mezzio\ConfigProvider::class,
    \Mezzio\Router\ConfigProvider::class,
    \Laminas\Diactoros\ConfigProvider::class,

    // Swoole config to overwrite some services (if installed)
    class_exists(\Mezzio\Swoole\ConfigProvider::class)
        ? \Mezzio\Swoole\ConfigProvider::class
        : function (): array {
            return [];
        },

    // Default App module config
    App\ConfigProvider::class,

    // Load application config in a pre-defined order in such a way that local settings
    // overwrite global settings. (Loaded as first to last):
    //   - `global.php`
    //   - `*.global.php`
    //   - `local.php`
    //   - `*.local.php`
    new PhpFileProvider(realpath(__DIR__) . '/autoload/{{,*.}global,{,*.}local}.php'),

    // Load development config if it exists
    new PhpFileProvider(realpath(__DIR__) . '/development.config.php'),
], $cacheConfig['config_cache_path']);

return $aggregator->getMergedConfig();

@froschdesign
Copy link
Member

@bidi47
Thanks for testing! 👍🏻

In the meantime, I have also found the corresponding code location: https://github.com/laminas/laminas-component-installer/blob/5a2ebadbc6ecf81b38d84222170dbd773fc00d7f/src/Injector/AbstractInjector.php#L370-L379

@arhimede arhimede changed the base branch from 3.16.x to 3.15.x January 24, 2025 11:09
@arhimede arhimede added this to the 3.15.1 milestone Jan 24, 2025
@arhimede arhimede changed the base branch from 3.15.x to 3.16.x January 24, 2025 11:09
@arhimede arhimede removed this from the 3.15.1 milestone Jan 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants