yiisoft / docs

Various Yii 3.0 related documentation
http://www.yiiframework.com
BSD 3-Clause "New" or "Revised" License
232 stars 70 forks source link

About hiqdev/composer-config-plugin and configs #20

Closed pgaultier closed 4 years ago

pgaultier commented 6 years ago

Hi, I'm starting to use yii 3.0 to make some proof of concept and to learn new features from Yii3.0 before it is released.

I have a question about all the config files created in yiisoft/yii-core and other libraries/modules:

The use of https://github.com/hiqdev/composer-config-plugin is really great but I don't understand why some parameters are defined in https://github.com/yiisoft/yii-core/blob/master/config/params.php and used in https://github.com/yiisoft/yii-core/blob/master/config/common.php#L27.

This force the final developper to override keys defined in https://github.com/yiisoft/yii-core/blob/master/config/params.php in its own params.php file which will be cluttered.

I don't think it's a good idea as those values are already hardcoded in yiisoft/yii-core

For example

right now we have :

https://github.com/yiisoft/yii-core/blob/master/config/params.php

return [
    'app.id' => 'core',
    'app.name' => 'Core',
    'app.language' => 'en',
];

and in https://github.com/yiisoft/yii-core/blob/master/config/common.php

...
'app' => [
        'id' => $params['app.id'],
        'name' => $params['app.name'],
        'language' => $params['app.language'],
        'aliases' => array_merge($aliases, [
            '@root'     => YII_ROOT,
            '@vendor'   => '@root/vendor',
            '@public'   => '@root/public',
            '@runtime'  => '@root/runtime',
            '@bower'    => '@vendor/bower-asset',
            '@npm'      => '@vendor/npm-asset',
        ]),
        'params' => $params,
    ],
...

Why do not leave https://github.com/yiisoft/yii-core/blob/master/config/params.php empty

return [
];

and define hardcoded values in https://github.com/yiisoft/yii-core/blob/master/config/common.php

...
'app' => [
        'id' => 'core',
        'name' => 'Core',
        'language' => 'en',
        'aliases' => array_merge($aliases, [
            '@root'     => YII_ROOT,
            '@vendor'   => '@root/vendor',
            '@public'   => '@root/public',
            '@runtime'  => '@root/runtime',
            '@bower'    => '@vendor/bower-asset',
            '@npm'      => '@vendor/npm-asset',
        ]),
        'params' => $params,
    ],
...

This way, app configuration will be done by the developper as it was before :

in app/params.php

return [
    'myparam1' => 'value1',
];

and app/common.php

return [
    'app' => [
        'id' => 'my-app',
        'name' => 'My own Application',
        'language' => 'fr',
    ],
]

and this will leave params.php dedicated to application parameters which will be more clear.

hiqsol commented 6 years ago

We are using composer-config-plugin for quite some time and developed good habits with it. It's very convenient to have own params.php in every extension because:

In many cases developer may not even need to define any configuration, it could be enough to set only params.

leave params.php dedicated to application parameters which will be more clear

IMHO it's sad to use such a good mechanism for application parameters only. Actually we almostly don't use application params (I consider it a bad style), but we use lots of such pass of parameters to configs, these makes configurations very obvious and ... um ... configurable.

in its own params.php file which will be cluttered

Vice-versa. Thanks to distributed nature of configuration the main configs become tiny. We have huge applications (I have reasons to think them to be among biggest Yii applications) and it's main params.php has about 20 parameters and common.php is just empty.

pgaultier commented 6 years ago

Well, I understand what your are explaining (and I also run really big applications on Yii2), but in that case why have you defined only 3 parameters for application configuration ?

Right now we have :

return [
    'app.id' => 'core',
    'app.name' => 'Core',
    'app.language' => 'en',
];

by the way, the app.language does not match the language defined in Application and the app.name does not match the name defined in Application

but if I follow what you are explaining, we should have :

return [
    'app.id' => 'core',
    'app.name' => 'Core',
    'app.language' => 'en-US',
    'app.layout' => 'main',
    'app.controllerNamespace' => 'app\\controllers',
    'app.controllerMap' => [],
    'app.defaultRoute' => 'default',
    'app.charset' => 'UTF-8',
    'app.sourceLanguage' => 'en-US',
    'app.bootstrap' => [],
];

which are all the values we can configure for the yiisoft/yii-core Application

My "problem" here is that you defined only the params based on your projects while other developpers may override other params.

Right now, in my case, if I follow what you are telling, I need to define : params.php

return [
    'app.id' => 'my-app',
    'app.name' => 'MyApplication',
    'app.language' => 'fr-FR',
];

and web.php

return [
    'app' => [
        'sourceLanguage' => 'fr-FR',
        'layout' => 'front',
        'controllerNamespace' => 'web\\controllers',
    ],
]

Which is not easy to use.

Right now, I would prefer to have :

params.php

return [
];

and web.php

return [
    'app' => [
        'id' => 'my-app',
        'name' => 'MyApplication',
        'language' => 'fr-FR',
        'sourceLanguage' => 'fr-FR',
        'layout' => 'front',
        'controllerNamespace' => 'web\\controllers',
    ],
]

OR params.php

return [
    'app.id' => 'my-app',
    'app.name' => 'MyApplication',
    'app.language' => 'fr-FR',
    'app.sourceLanguage' => 'fr-FR',
    'app.controllerNamespace' => 'web\\controllers',
    'app.layout' => 'front',
];

and web.php

return [
]

As I explained before I prefer the former which show clearly which properties are populated while the latter just move some information from original "object" configuration to a params.php file.

hiqsol commented 6 years ago

Ok. You can have it exactly as you prefer: empty params.php and web.php with everything set. When you have values set in web.php they will not be touched by params. Of course you don't need to copy same values to params and configs that would be violation of basic SPOT principle.

So in short. If you prefer to have your config more directly - it's up to you, put all config in web.php. I prefer (and recommend based on my experience) to use standard configuration and tune it with params or change it when it's necessary.

Some people even prefer to have all their config completely explicit without any merging. Or merge it in some other way. Fine, let a thousand flowers bloom.

What I'm trying to achieve is the configuration that is:

pgaultier commented 6 years ago

I totally understand you point but let me rephrase my question :

  1. Why don't you put all parameters in params.php ? a. If you think this is better for "doc", all core params should be available don't you think ? b. Why the values you put in the https://github.com/yiisoft/yii-core/blob/master/config/params.php are not consistent with the default defined in the https://github.com/yiisoft/yii-core/blob/master/src/base/Application.php ?

  2. Is there a way to force $app->params to be empty or have only my values and keep the use of the config builder which is great ?

Because when I do that in web.php :

return [
    'app' => [
        'basePath' => dirname(dirname(__DIR__)).'/web',
        'controllerNamespace' => web\controllers::class,
        'params' => []
    ],
];

the params are still populated with the default values sets in every "module" which is the expected behavior as parameters are merged.

hiqsol commented 6 years ago
  1. That's all simply because it's far from finished yet :) I only began and there is still lots to improve.
  2. This one is harder. Those little params make you troubles so much? :) No simple solution right now. I think I need to add support for UnsetArrayValue, ReplaceArrayValue in composer-config-plugin. Please see https://www.yiiframework.com/doc/api/2.0/yii-helpers-replacearrayvalue
pgaultier commented 6 years ago

This would be great ;-)

In fact, the problems with stuff in $params is that right now everything "seems" clear for everyone, but in some time, when a lot of people will create extensions and use this technique, we will have a $params with a lot of parameters inherited from all modules and we will probably have some collisions between some params we are using and others which are defined by some applications.

In that case, to avoid collisions I would see some guidelines to "protect" variables in the $params.

For exemple, everything inherited from yii could be put in a separate "namespace" :

return [
    '__yii' => [
        'app.id' => 'my-app',
        'app.name' => 'MyApplication',
        'app.language' => 'fr-FR',
    ]
];

and define some guidelines for modules authors to not clutter the params and ask them to add values like this :

return [
    '__moduleName' => [
        'module.var1' => 'value1',
    ],
];

or anything else while it stays in their "namespace"

This way I would be less disturbed ;-)

hiqsol commented 6 years ago

We've tried different variants. IMHO simple plain structure module.option.suboption turned out to be very convenient to use and distinctive enough.

It doesn't seem very difficult to avoid collisions - one can open vendor/hiqdev/composer-config-plugin-output/params.php and see which params are already used. But you are right we have to think and work out recommendations for params naming. It is similar story to i18n messages category and it looks like there are no recommendations for good category naming.

pgaultier commented 6 years ago

I agree with you about the module.option.suboption but I would just encapsulate the options with the __module prefix. Let's says, for example, I have an application with my own param :

return [
    'mail.address' => 'xxx',
];

and a few time later, I add a new mail module from a third party, the probability to have a collision with the module params is high.

If a rule says the module developper should encapsulate his options there won't be collision.

In this case the new module called mail would export :

return [
    '__mail' => [
        'address' => 'xxx',
        'transport.secure' => true,
    ],
];

and everything would be ok.

In fact, in my idea, the developper of the end application shouldn't have to bother about third parties modules params and he should be sure he can do whatever he wants with his options naming. Exactly like how it was done for the di when the core team chose to move class to __class .

Anyway thank you for your answers.

hiqsol commented 6 years ago

I didn't catch what is the principal difference between:

The question is how to select distinctive enough identifier 'mail' and this is what should the recommendations tell about.

My general idea is: the deeper the code - the shorter the identifiers (same is true for i18n categories). So inside of yii we have very short params like app.id. While third party extensions should have longer param names maybe including their vendor name like: hiqdev.mail.address.

pgaultier commented 6 years ago

Sorry, I was not clear about what I had in mind and I think your idea about the vendor name is important.

let says, right now we will have in the params generated by the different modules:

return [
    // generated by yiisoft yii-core module
    'app.id' => 'core',
    'app.name' => 'Core',
    'app.language' => 'en',
    // generated by hiqdev mail module
    'mail.address' => 'no-reply@address.com',
    'mail.transport.secure' => true,
];

I would create something like this :

return [
    // generated by yiisoft yii-core module
    '__yiisoft' => [
        'app.id' => 'core',
        'app.name' => 'Core',
        'app.language' => 'en',
    ],
    // generated by hiqdev mail module
    '__hiqdev' => [
        'mail.address' => 'no-reply@address.com',
        'mail.transport.secure' => true,
    ],
];

This way I can add whatever property in the params.php, it will never collide with other modules and the rules will be identical for yiisoft vendor and others

let's say my params.php is :

return [
    'mail.address' => 'contact@company.com'
];

in the former, I will obtain in merged params :

return [
    // generated by yiisoft yii-core module
    'app.id' => 'core',
    'app.name' => 'Core',
    'app.language' => 'en',
    // generated by hiqdev mail module
    'mail.transport.secure' => true,
    // overriden by my config
    'mail.address' => 'contact@company.com'
];

While in the latter, I will obtain :

return [
    // generated by yiisoft yii-core module
    '__yiisoft' => [
        'app.id' => 'core',
        'app.name' => 'Core',
        'app.language' => 'en',
    ],
    // generated by hiqdev mail module
    '__hiqdev' => [
        // original config won't be overriden as expected
        'mail.address' => 'no-reply@address.com',
        'mail.transport.secure' => true,
    ],
    // my own parameter will be OK
    'mail.address' => 'contact@company.com'
];

And if I want, I can specifically override module parameters :

return [
    '__hiqdev' => [
        'mail.address' => 'no-reply@company.com',
    ],
    'mail.address' => 'contact@company.com'
];

which will give me the expected result :

return [
    // generated by yiisoft yii-core module
    '__yiisoft' => [
        'app.id' => 'core',
        'app.name' => 'Core',
        'app.language' => 'en',
    ],
    // generated by hiqdev mail module
    '__hiqdev' => [
        // this key will be overriden as expected
        'mail.address' => 'no-reply@company.com',
        'mail.transport.secure' => true,
    ],
    // my own parameter will be OK
    'mail.address' => 'contact@company.com'
];

Am I more clear ?

hiqsol commented 6 years ago

Yes. I got it. But my argument remains intact. That's the question of good prefixes. And to my experience it is easier to have param names plain without subarrays.

So in your example I would recommend to have params like this:

return [
    'app.id' => 'core',
    'app.name' => 'Core',
    'hiqdev.mail.address' => '...',
    'hiqdev.other.param' => '...',
    'mail.address' => '...',
];

And compare usage:

return [
    \hiqdev\MailerInterface::class => [
        '__class' => \hiqdev\Mailer::class,
        'address' => $params['hiqdev.mailer.address'],
        'address' => $params['__hiqdev']['mailer.address], 
    ],
];

And of course you are free to use subarrays - the system supports it. For me it's a question of convenience.

Maybe other yii members (@samdark, @SilverFire) have something to say.

samdark commented 6 years ago

I've used both approaches and they were OK. From my experience flat dot-separated names are fine to use so I have no objections.

pgaultier commented 6 years ago

Well, my idea was just to have a rule which can make sure we won't have collisions between keys.

let me give you an example :

return [
    // variable to make sure I have my speed level 
    'speedy.level' => 2,
];

What's going on if I have to use later a plugin from vendor speedy who already exists (https://packagist.org/packages/speedy/) ?

That's why I would prefer a rule for modules creators to not create bad parameters and create a guideline to prefix their vendor name with __.

based on your proposal, it would be

return [
    // variable to make sure I have my speed level 
    'speedy.level' => 2,
    '__speedy.param1' => 'value for speedy parameter 1', // from vendor speedy
];

or based on my proposal, it would be

return [
    // variable to make sure I have my speed level 
    'speedy.level' => 2,
    '__speedy' => [ // from vendor speedy
        'param1' => 'value for speedy module', 
    ],
];

Defining in the guidelines that the __ must be used will avoid a lot of problems.

For the second part which is to use subarrays for the vendor it is just a convenience which would allow grouping. Anyway this is not something which bother me while, IMHO, the absence of __ to prefix and protect params is a problem.

PS: just one think I forgot, I think it's important to create a real guideline which explain how the parameters should be defined in a module (subarray, or dot syntax). This will make something consistent in Yii ecosystem.

samdark commented 6 years ago

Agree. Having a convention would help.

Aradiv commented 6 years ago

From reading the issue it may be also a good idea to create a guidline what should be in the config files and what should be in params.php

for example with the authclient component

currently we have

'components' => [
    'authClientCollection' => [
        'class' => 'yii\authclient\Collection',
        'clients' => [
            'google' => [
                'class' => 'yii\authclient\clients\Google',
                'clientId' => 'google_client_id',
                'clientSecret' => 'google_client_secret',
                'scopes' => 'google_required_scopes'
            ]
        ]    
    ]
]

While clientId and clientSecret have to be different in every installation scopes are required to stay the same unless the code of the application changes.

So a good rule of thumb would be something like: when you install the application you only have to change params.php when you extend / modify the application you change the config files.

hiqsol commented 6 years ago

So a good rule of thumb would be something like: when you install the application you only have to change params.php when you extend/modify the application you change the config files.

Yes. That's the idea. In most cases, the default configuration is quite enough and it is only needed to set params.

Configurations and params are far from finished at the moment. There will be more configurations and params before release. And of course, lots of documentation about to be added.

tomaszkane commented 4 years ago

Hi @hiqsol any updates here? I found https://github.com/hiqdev/composer-config-plugin#usage How about full usage example with yii2-app-advanced? (common, console, backend, params, enviromnets (dev, prod...) etc.

samdark commented 4 years ago

https://github.com/yiisoft/docs/blob/master/guide/en/concept/configuration.md