Skip to content

[4.x] Improve container extensibility#61

Closed
voronkovich wants to merge 3 commits intojoomla-framework:4.x-devfrom
PopArtDesign:improve-extensibility
Closed

[4.x] Improve container extensibility#61
voronkovich wants to merge 3 commits intojoomla-framework:4.x-devfrom
PopArtDesign:improve-extensibility

Conversation

@voronkovich
Copy link
Contributor

@voronkovich voronkovich commented Mar 13, 2025

The current design of the Container class lacks extensibility. For example, it's not possible to add a new type of service (e.g. lazy services) without dirty hacks or BC breaks.

Summary of Changes

This PR replaces the use of $mode, $shared and $protected parameters with a single plain $options array.

$container->set(
    'foo',
    fn () => new \stdClass(),
    [ 'shared' => true, 'protected' => true ]
);

$container->share(
    'foo',
    fn () => new \stdClass(),
    [ 'protected' => true ]
);

$container->protect(
    'foo',
    fn () => new \stdClass(),
    [ 'shared' => true ]
);

It allows us to easily add new types of services and options in the future:

$container->set(
    'foo',
    fn () => new \stdClass(),
    [ 'shared' => true, 'protected' => true, 'lazy' => true ]
);

Testing Instructions

Launch phpunit and ensure that the tests are passed.

Documentation Changes Required

Yes

@voronkovich voronkovich force-pushed the improve-extensibility branch from faca5ca to a350581 Compare March 13, 2025 01:43
@HLeithner
Copy link
Contributor

I'm not really happy with the direction, you can't really test this in you own component if you have a typo or not (at least it's harder) also static code analyzers need much more support for this. It's also a hard b/c break, I'm pretty sure that this will not happen in this way.

@voronkovich
Copy link
Contributor Author

@HLeithner, thanks for your review!

I've added checking for allowed options. Now, if an unknown option is provided, an exception is thrown.

Unknown option "unshare" given. Allowed options: shared, protected.

Also, I've implemented a simple BC layer to avoid breaking existing code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants