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

REQUEST: Changing array_merge() into array_merge_recursive() in \Illuminate\Support\ServiceProvider\mergeConfigFrom() #13108

Closed
aschwin opened this issue Apr 11, 2016 · 4 comments

Comments

@aschwin
Copy link

aschwin commented Apr 11, 2016

Hi,

Is there any objection to change array_merge() to array_merge_recursive in mentioned method?

array_merge() will only merge on a shallow level, while the recursive one will do it properly (as far as I can see).

Any thoughts?

@crynobone
Copy link
Member

http://jontai.me/blog/2011/12/array_merge_recursive-vs-array_replace_recursive/

array_replace_recursive were once merged in but had was reverted due to breaking changes AFAIK. See #1225

@GrahamCampbell
Copy link
Member

We very intentionally don't do this recursively.

@GrahamCampbell
Copy link
Member

Otherwise, how could you override a config entry with an empty array if it had other stuff there?

@Levivb
Copy link

Levivb commented Nov 10, 2016

Although this request is closed I'd like to show how it would be possible.

For a bit of background
I'm using pingpong modules for a project and needed to overrule certain nested module-config values in the app.

Every module has a [moduleName]ServiceProvider (ex. PageServiceProvider) which extends a custom ModuleServiceProvider which in turn extends Illuminate\Support\ServiceProvider.

the mergeConfigFrom in ModuleServiceProvider contains the following:

protected function mergeConfigFrom($path, $key)
    {
        //Get module app config
        $config = $this->app['config']->get($key, []);
        //Get module config
        $module_config = require $path;
        //recursive replace (also merges). Prefer app-specific config
        $combined_config = array_replace_recursive($module_config, $config);

        //Recursive closure which removes null from the config
        $filter_nulls = null;
        $filter_nulls = function ($input) use (&$filter_nulls) {
            if (is_array($input)) {
                foreach ($input as &$value) {
                    // is_callable check just for IDE error supression, since the function itself should be callable
                    if (is_array($value) && is_callable($filter_nulls)) {
                        $value = $filter_nulls($value);
                    }
                }
            }
            // Keep not-null values
            return array_filter($input, function ($v) {
                return !is_null($v);
            });
        };
        // Apply filter
        $filtered_config = $filter_nulls($combined_config);
        // And set
        $this->app['config']->set($key, $filtered_config);
    }

In short, it recursively removes null values from the config. It might not be backwards compatible nor Laravel worthy. But for whoever stumbles across this thread, it might be useful. On the downside... you can't have null values in your config.

As for an example:

$moduleConfig = [
    'first_setting' => [
        'inner_setting_1' => true,
        'inner_setting_2' => [
            'nested_inner_setting_2' => 'some_value',
        ],
    ],
    'redundant_array' => [
        'redundant_inner_setting_1' => true,
        'redundant_inner_setting_2' => false,
    ],
];

$appConfig = [
    'new_setting' => 'customization',
    'first_setting' => [
        'inner_setting_1' => false
    ],
    'redundant_array' => [
        'redundant_inner_setting_1' => null,
        'redundant_inner_setting_2' => null,
    ],
];

Results in:

$moduleConfig = [
    'new_setting' => 'customization',
    'first_setting' => [
        'inner_setting_1' => false,
        'inner_setting_2' => [
            'nested_inner_setting_2' => 'some_value',
        ],
    ],
    'redundant_array' => [
    ],
];

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

No branches or pull requests

4 participants