Framework: [5.4] Regression with loading of nested config files

Created on 18 Jan 2017  路  6Comments  路  Source: laravel/framework

  • Laravel Version: 5.4
  • PHP Version: 7.0.14
  • Database Driver & Version: N/A

Description:

Due to changes to src/Illuminate/Foundation/Bootstrap/LoadConfiguration.php within a00b601, nested config files are no longer loaded correctly.

The bug was introduced with the removal of the getConfigurationNesting method, and the alteration to getConfigurationFiles.
The issue is that a config file within a folder, such as config/package/general.php, will not be able to be read with config('package.general'), but instead only with config('general') because the nesting data is discarded.

A secondary issue is that when two separate nested config files with the same name are loaded (such as config/a/general.php and config/b/general.php), the file read last will overwrite the previously loaded one. Finder seems to order folders-first, alphabetically - so in this case config/b/general.php can be read with config('general'), but /a/ is lost.

Steps To Reproduce:

$ laravel new config-regression --dev
$ cd config-regression && \
  mkdir config/a && \
  echo "<?php return ['somekey' => 'somevalue'];" > config/a/general.php
$ php artisan tinker
>>> config('a')
=> null
>>> config('general')
=> [
     "somekey" => "somevalue",
   ]

As a comparison (5.3)

$ laravel new everything-is-fine
$ cd everything-is-fine && \
  mkdir config/a && \
  echo "<?php return ['somekey' => 'somevalue'];" > config/a/general.php
$ php artisan tinker
>>> config('a')
=> [
     "general" => [
       "somekey" => "somevalue",
     ],
   ]
>>> config('general')
=> null

Most helpful comment

All 6 comments

Nested configuration files are not going to be supported in 5.4.

I'm curious why you needed a folder nested within the config directory?

I'm of the opinion of @robmilward

I'm curious why you needed a folder nested within the config directory?

@taylorotwell For starters it allows package developers to better organise the published config files from their own packages. I've done it and still do, but i do realise not a lot of people do it.

Let's imagine i have a couple of packages under the same vendor and all of these packages have their set of configuration files (1 or many)

What looks better

config/my-vendor/package-a/config.php
config/my-vendor/package-a/config-second.php
config/my-vendor/package-b/config.php
config/my-vendor/package-b/config-second.php
config/my-vendor/package-c/config.php
config/my-vendor/package-d/config.php
config/my-vendor/package-e/config.php

Or

config/my-vendor-package-a-config.php
config/my-vendor-package-a-config-second.php
config/my-vendor-package-b-config.php
config/my-vendor-package-b-config-second.php
config/my-vendor-package-c-config.php
config/my-vendor-package-d-config.php
config/my-vendor-package-e-config.php

If you'll ask me, i would say option 1 without a blink as it keeps the config folder more cleaner.
If we can organise controllers, models etc.. with our own way, why can't we organise the config folder the same way?

Just leaving my opinion and by the end of the day, it is what it is :)

I'm curious why you needed a folder nested within the config directory?

@taylorotwell - @brunogaspar has pretty much exactly described the reason

I'm beginning to build a set of interconnected packages but wanted to keep their configuration completely separated from the system config, but still grouped. The ability to nest is a good way of namespacing them so I don't litter the config folder with:

package-general.php
package-something.php
package-another.php

It also ties in nicely when merging configs in the package providers:

// In vendor/package-general's provider
$this->mergeConfigFrom(
    '/path/to/config/general.php', 'package.general'
);

// In vendor/package-something's provider
$this->mergeConfigFrom(
    '/path/to/config/something.php', 'package.something'
);

// In vendor/package-another's provider
$this->mergeConfigFrom(
    '/path/to/config/another.php', 'package.another'
);

I'm having @themsaid take a look at bringing it into 5.4.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

felixsanz picture felixsanz  路  3Comments

iivanov2 picture iivanov2  路  3Comments

RomainSauvaire picture RomainSauvaire  路  3Comments

shopblocks picture shopblocks  路  3Comments

fideloper picture fideloper  路  3Comments