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

AOT factory creation CLI command for ReflectionBasedAbstractFactory mapped factories #171

Closed
boesing opened this issue Jan 26, 2023 · 4 comments · Fixed by #180
Closed
Milestone

Comments

@boesing
Copy link
Member

boesing commented Jan 26, 2023

Feature Request

Q A
New Feature yes
RFC kinda
BC Break no

Summary

Currently, there are CLI commands around where it is possible to create mezzio handler factories and stuff by using reflection. I was wondering if it would also be possible to provide a CLI command which takes runtime config from container and creates factories (along with a mapping-file such as mezzio-tools) using the same logic as ReflectionBasedAbstractFactory.

This would enable projects to use a more "generic" runtime factory during development while enabling devs to run that AoT CLI command in CI builds to auto-generate factories. This will remove the reflection stuff during runtime and increases performance with just running a CLI command (and maybe a bit of configuration, i.e. to provide different other PluginManager configuration keys such as validators, etc. - which can also be injected to the config from within our own components).

WDYT?

@boesing
Copy link
Member Author

boesing commented Feb 7, 2023

Seems that the ReflectionBasedAbstractFactory is supporting config parameter while the Factory creation tool is not. Might need to be extended to properly support the AOT command.

@froschdesign
Copy link
Member

The documentation says:

A parameter named $config typehinted as an array will receive the application "config" service (i.e., the merged configuration).

https://docs.laminas.dev/laminas-servicemanager/reflection-abstract-factory/

But should the entire configuration be used here? That just seems too much.

@boesing
Copy link
Member Author

boesing commented Feb 7, 2023

Thats not to be discussed in this issue.
This issue simply suggest providing an AOT CLI command which can be executed during CI which detects services explicitly using ReflectionBasedAbstractFactory as their factory.

The reflection based abstract factory provides that feature and thus the AOT command should reflect the same logic.
That it is not a good idea to use the whole config in a service in general is another topic.

So for example:

return [
     'dependencies' => [
        'factories' => [
            'WhateverService' => ReflectionBasedAbstractFactory::class, // factory should be generated and configuration should be replaced with the generated factory
        ],
        'abstract_factories' => [
             ReflectionBasedAbstractFactory::class, // ignored
        ],
    ],
];

@boesing boesing linked a pull request Mar 5, 2023 that will close this issue
@boesing boesing added this to the 4.0.0 milestone May 2, 2023
@boesing
Copy link
Member Author

boesing commented May 2, 2023

Implemented with #180

@boesing boesing closed this as completed May 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants