Allow Multiple Config Classes for Each Type

[chrisguitarguy]

So something like this might be in a composer.json:

{
    "extra": { "aura": {
        "type": "project",
        "config": {
            "common": [
                "Acme\\Example\\_Config\\AuraSql",
                "Acme\\Example\\_Config\\Twig"
            ],
            "dev": "Acme\\Example\\_Config\\Dev"
        }
    } }
}

A bigger application might want to separate configuration into several
different classes. Normal strings still work, but arrays may be used as
well.

[harikt]

Hey @chrisguitarguy ,

Thank you for the PR . I was discussing with @pmjones over auraphp/Aura.Intl@0afd245#commitcomment-7231183 .

May be I should have opened an issue.

One thing I noticed working on it is the type. I feel the type should be an array.

"type": {
    "project": {
        "config": {
            "common": [
                "Acme\\Example\\_Config\\AuraSql",
                "Acme\\Example\\_Config\\Twig"
            ],
            "dev": "Acme\\Example\\_Config\\Dev"
        }
    },    
    "library": {
        "config": {
            "common": [
                "Acme\\Example\\_Config\\AuraSql",
                "Acme\\Example\\_Config\\Twig"
            ],
            "dev": "Acme\\Example\\_Config\\Dev"
        }
    }
}

The idea of making type as an array is the library, bundle etc have different priority.

Thank you.

[chrisguitarguy]

That doesn't make much sense to me. Actually, the concept of having a type doesn't make much sense to me based on the current code. If something needs to be configured, it will probably be configured the same way regardless of whether its a library or project. To do otherwise seems to violate the principle of least astonishment.

I like this a lot:

if (is_string($config->common)) {
    $this->config_classes[$type][] = $config->common;
} else {
    $values = array_values((array) $config->common);
    $this->config_classes[$type] = array_merge($this->config_classes[$type], $values);
}

Much better solution than what I wrote. More flexible.

[harikt]

@chrisguitarguy I don't know whether you get me on this.

The library , bundle, kenel, project need to be loaded in the order. Else the problem is when you have defined config/Common.php you may not be having the necessary dependencies defined. Or you need to define the order or set a priority.

Let us look into an example. Take https://github.com/harikt/Aura.Asset_Bundle/blob/master/config/Common.php#L15-L22 .

$di->params['Aura\Asset_Bundle\AssetResponder'] = array(
    'response' => $di->lazyGet('web_response'),
);

$di->params['Aura\Asset_Bundle\AssetAction'] = array(
    'domain' => $di->lazyNew('Aura\Asset_Bundle\AssetService'),
    'responder' => $di->lazyNew('Aura\Asset_Bundle\AssetResponder'),
);

You can see the call for $di->lazyGet('web_response') . If Aura.Web config is not loaded, then the call breaks. That is the idea of the type.

But I may be wrong to understand the Principle of least astonishment

[chrisguitarguy]

The library , bundle, kenel, project need to be loaded in the order. Else the problem is when you have defined config/Common.php you may not be having the necessary dependencies defined. Or you need to define the order or set a priority.

Gotcha, that makes more sense. This is a super clever way to accomplish that, just didn't pick up on the reasoning behind it until you mentioned something.

Would there ever be a call for more than one type?

[harikt]

Would there ever be a call for more than one type?

I didn't get this :( . Could you please expand ?

[chrisguitarguy]

In your example:

{ "type": {
    "project": {
        "config": {
            "common": [
                "Acme\\Example\\_Config\\AuraSql",
                "Acme\\Example\\_Config\\Twig"
            ],
            "dev": "Acme\\Example\\_Config\\Dev"
        }
    },    
    "library": {
        "config": {
            "common": [
                "Acme\\Example\\_Config\\AuraSql",
                "Acme\\Example\\_Config\\Twig"
            ],
            "dev": "Acme\\Example\\_Config\\Dev"
        }
    }
} }

It seems like the benefit of using the more complex structure is that a package could have more than one type. If that's the case: how does the Project object know which one to load? Is there ever really a call to have more than one type?

[harikt]

It seems like the benefit of using the more complex structure is that a package could have more than one type

ok, thanks for letting me know. No only one type. What I wanted was if we are doing it on the https://github.com/auraphp/Aura.Web_Project/blob/master/composer.json#L31-L37 . See Web_Project and not on library or bundle or anything. Only on the root composer.json .

The reason behind it is if some libraries may have missed the type https://github.com/auraphp/Aura.Input/blob/master/composer.json#L43-L47 we could add in the project.

And you should note that , aura/input is a library. And it was just a suggestion though. Not a super important stuff.

[harikt]

And all the changes in the PR and the one mentioned will only happen at

Aura.Project_Kernel/src/Project.php

Line 186 in f26dec4

$this->addConfigClasses($this->composer);

. Not all places is needed.

[chrisguitarguy]

The reason behind it is if some libraries missed to have the type we could add in the project.

Ah, I see what you mean, I think. The idea of allowing multiple "types" in a root project would be to allow adding subprojects. Like Aura\Input which doesn't have the composer stuff defined. Let's you correct the resolution order of dependencies. Right?

Seems like something to be resolved in another pull request or issue?

[harikt]

I would love @pmjones to look into this before we do anything further.

That may take quite some time depending on @pmjones availability though ;) .

[pmjones]

I like it! @chrisguitarguy thanks for the PR and the collegial work, and of course many thanks to @harikt for giving this such competent attention. Well done guys.

[harikt]

@pmjones there are still somethings that are not done. Eg multiple types. What are your thoughts on that ? Only for the root project it will be helpful.

[pmjones]

@harikt You mention earlier that multiples types per package seems unwise, and I agree with that assessment.

[harikt]

multiple types per package seems unwise

Yes, but for the project it is nice to have. I mean inside the json of Aura.Web_Project if there is a way to add multiple types it seems nice. Do you get me ?

Eg : When Aura.Input v1 is not getting the config/Common in the composer.json .

Thanks

[pmjones]

I think I get you, but I don't think "multiple types" is needed. At the project level, you override the settings from the underlying levels. You can still override, e.g., Aura.Input when you want to.

[harikt]

You can still override, e.g., Aura.Input when you want to.

ok. May be I am not aware how to do then other than adding in the project/config/Common.php even if the aura/input/config/Common.php exists for it will not be loaded via project kernel.

[harikt]

Anyway it is not necessary :) .