Slim: Provide BC support for Slim 3's 'settings' key in Slim 4

Created on 16 Sep 2018  路  6Comments  路  Source: slimphp/Slim

In Slim 4, settings are added directly to the constructor and not via a settings sub-key.

Slim 3:

$app = new App([
    'settings' => [
        'foo' => 'bar',
    ],
]);

Slim 4:

$app = new App([
    'foo' => 'bar',
]);

Change Slim 4's App::__construct to detect:

  1. if $settings has exactly one key called 'settings', then trigger_error() with an E_USER_DEPRECATED error type.
  2. If $settings has more than one key and one key is called 'settings', then raise an RuntimeException.
discussion

Most helpful comment

Most of the time, people only use a settings key, but I'm pretty sure there are cases where the developer passes other data (i.e. additional keys) to the container being instanciated.

As it's a major version bump, I'd bend forward l0gicgate here. Such an upgrade is not a "no-brainer", it requires applying a migration procedure, and this change is quite fast and easy to make on user code.

IMHO, just doing the breaking change would be way better than adding confusion with a compatibility layer, which also is not 100% sure as it does some guessing that may be wrong (see my first sentence).

Note a similar change had been done when switching from Slim 2 to Slim 3.

Last but not least, this way you don't forbid to the user the settings key name, which may be a convenient key name for him.

All 6 comments

I agree.

Reading the docs will be required to migrate from Slim 3 to 4. I think we need to make the migration guide as clear and concise as possible. As mentioned in our Slack discussion, I dislike throwing exceptions or deprecation warnings as you have to carry that code through forever/until next major release.

Clear migration documentation is the solution here in my opinion.

Most of the time, people only use a settings key, but I'm pretty sure there are cases where the developer passes other data (i.e. additional keys) to the container being instanciated.

As it's a major version bump, I'd bend forward l0gicgate here. Such an upgrade is not a "no-brainer", it requires applying a migration procedure, and this change is quite fast and easy to make on user code.

IMHO, just doing the breaking change would be way better than adding confusion with a compatibility layer, which also is not 100% sure as it does some guessing that may be wrong (see my first sentence).

Note a similar change had been done when switching from Slim 2 to Slim 3.

Last but not least, this way you don't forbid to the user the settings key name, which may be a convenient key name for him.

As a user I would prefer not having the ambiguity of having both old and new available, and have this change clearly stated on migration docs.

I agree with @luispabon. If the user wants to use the key name, he just needs to pass the array directly. $app = new App($arr['settings']);

Thanks all. Decision made! Let's not do this.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

codeguy picture codeguy  路  3Comments

codeguy picture codeguy  路  4Comments

JamesTheHacker picture JamesTheHacker  路  3Comments

arokettu picture arokettu  路  3Comments

xymz picture xymz  路  5Comments