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

[RFC]: Removal of Application#bootstrap and Application#init #151

Open
boesing opened this issue Mar 15, 2023 · 4 comments
Open

[RFC]: Removal of Application#bootstrap and Application#init #151

boesing opened this issue Mar 15, 2023 · 4 comments
Labels

Comments

@boesing
Copy link
Member

boesing commented Mar 15, 2023

RFC

Q A
Proposed Version(s) 4.0.0
BC Break? Yes

Goal

Removal of the method Application#bootstrap and the method Application#init.

This will automatically also supersede the Bootstrap MVC event.
Whatever is being done in those event listeners should be moved to delegator factories. If something has to be configured in the application, the Application can be delegated, etc.

This will also remove the ability to have (whyever) listeners being registered to the EventManager.
These listeners can be registered via a delegator factory for the EventManagerInterface being injected to Application::__construct as well.

Due to the latest changes being made to the skeleton, we suggest projects to provide config/container.php as we do in mezzio. Thats awesome as that will actually move plenty of stuff to that config/container.php.

I am aware of projects (even in our company) which are already migrating a away from modulemanager to config-aggregator and thus, no modules are registered anymore since they provide ConfigProvider instead (and due to the laminas-config-aggregator-modulemanager, we are also providing full support to modules being provided as ConfigProvider).

So by having a full container up and running via config/container.php (which for laminas-mvc-skeleton currently calls Application#init($config)->getServiceManager()), the Application is already fetched via $container->get(Application::class) and thus, all the magic bootstrap/whatever logic can be added as delegator factories.

If a project hardly depends on that bootstrapping logic, we can provide a DelegatorFactory which can be registered to the $container->get(Application::class) which then triggers the event. I am open for ideas here.

Background

Due to the huge amount of possibilities to configure the MVC application, etc. there is almost no way to catch all scenarios in laminas-cli. Thats all because too much logic is executed in public/index.php instead of having the container configure that stuff.

Considerations

Users might want to migrate their MvcEvent#BOOTSTRAP_EVENT to delegator factories of Application.
We do have to migrate our stuff to delegator factories (which is now being done in both init and bootstrap method).
We do have to provide a proper factory for the Application due to the lack of Application#init factory method).

Proposal(s)

  • Remove both methods
  • Provide Application factory to be registered to the container
  • Retrieve Application from container (already prepared laminas/laminas-mvc-skeleton@1a242e4)
  • Move logic from init and bootstrap to delegator factories

Appendix

Ref: laminas/laminas-cli#106
Ref: laminas/laminas-cli#92
Ref. laminas/laminas-cli#85

@internalsystemerror
Copy link
Member

internalsystemerror commented Mar 15, 2023

+1 for this. Does it have to be delegator factories though (iirc these aren't the most performant)?

If we're firing the bootstrap event from the Application factory, would there be any need to register further factories via delegators (honestly no idea on that hence why I ask)?

We too have completely converted to using the config-aggregator, but I'm sure that there are many that haven't, so I'd be hesitant to get rid of module manager completely and see that part of this as much more long term.

@boesing
Copy link
Member Author

boesing commented May 8, 2023

Does it have to be delegator factories though (iirc these aren't the most performant)?

Not sure what "not the most performant" actually means. Can you clarify?
Delegator factories are only applied once. I'd say that having delegator factories over event listeners (as delegator factories are service-specific and in case the service is not used, the delegator has no TODO) will be the more performant way? Or do I miss something here?

If we're firing the bootstrap event from the Application factory, would there be any need to register further factories via delegators (honestly no idea on that hence why I ask)?

Can you elaborate? What do you mean with "firing bootstrap event from application factory"? This RFC targets the removal of those events, I do not really want to move these events to another location.

@internalsystemerror
Copy link
Member

Delegators vs regular factories, I was told around v2 time that they should be avoided where possible for performance reasons. Maybe I misunderstood/misremembered or thats no longer the case. Still I typically avoid them so my suggestion was probably to just combine everything from the delegators into a single factory if thats possible? As I say, maybe thats not an issue anymore or never was.

Reading again its obvious the events are to be removed, not moved so not sure why I thought otherwise.

@boesing
Copy link
Member Author

boesing commented May 9, 2023

Maybe you have initializers in mind? initializers do have direct performance impact as they are applied for any service. Delegators are service-specific and thus only trigger for those services which are requested.

Since most delegators are optional (especially those I am talking about), I don't see direct performance implication.
So we either have:

Application#bootstrap and Application#init which is called once per request or we have dedicated delegators which are executed once per request. The amount of code is the same while the latter is at least optional and won't fire anything in case projects do not depend on those events. Thats at least the idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants