search

webinertia / limatus

Bootstrap Laminas form elements and view helpers
BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

Wrong usage of delegators #23

Open froschdesign opened 1 year ago

froschdesign commented 1 year ago

Describe the bug

All delegators overwrite the previously created services. A delegator should be used to decorate an existing service, not to create a new service. If a new service is the desired result, use a normal factory.

To Reproduce

The menu helper should serve as an example here:

https://github.com/webinertia/limatus/blob/4ec73eeb5a18d5fa3b4c1a22421b79cf413f374f/src/ConfigProvider.php#L125-L138

The helper is created via the InvokableFactory and then a delegator is added which overwrites the previously created object:

https://github.com/webinertia/limatus/blob/4ec73eeb5a18d5fa3b4c1a22421b79cf413f374f/src/View/Helper/Navigation/Delegator/Factory/MenuFactory.php#L16-L24

Expected behavior

The delegator should not overwrite previously created services.

Additional context

Some more to read on this topic:

The forms, fieldsets and form elements are already handling options via the Laminas\Form\ElementFactory:

The delegators here handle the options differently than the ElementFactory:

if (isset($options['options'])) {
   $options = $options['options'];
}

https://github.com/laminas/laminas-form/blob/d20ba468fad89a23e68ed679b6b827543e166afa/src/ElementFactory.php#L39C9-L41C10

vs.

https://github.com/webinertia/limatus/blob/4ec73eeb5a18d5fa3b4c1a22421b79cf413f374f/src/Form/Element/Delegator/Factory/CheckboxFactory.php#L20-L22

tyrsson commented 1 year ago

Thanks for the feedback @froschdesign

Im planning on rewriting much of this. I just have to find the time. The regular 9-5 is killin me right now :( Once I have a chance to get to it if its ok I will send you a request for a review? Thanks again :)

froschdesign commented 1 year ago

I just have to find the time.

No hurry, just a hint.

The regular 9-5 is killin me right now :(

This is also true for me.

tyrsson commented 1 year ago

No hurry, just a hint.

And it's greatly appreciated. Honestly I am kinda speechless that someone as busy as you are took the time to read through the code.