Repository navigation
Support factory functions returning Closure for container variables - #318
Merged
Merged
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues block the addition of closure-valued container-variable support.
No open issues 🎉
What changed in this PR
Adds support for container-variable factories that return a Closure, preserving the returned instance across consumers while invoking the original factory only once. Class-name factory requirements remain unchanged.
Changes:
- Wraps cached closure values so later accesses do not invoke them as factories.
- Adds a regression test for shared closure identity and a single factory invocation.
- Updates the expected error when
getEnv()receives a closure instead ofstring|null.
| File | Description |
|---|---|
| tests/ContainerTest.php | Tests closure caching and updates the getEnv() type-error expectation. |
| src/Container.php | Allows closure-valued variables by wrapping cached closures. |
Review effort: Balanced
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This changeset adds support for factory functions returning a
Closurefor container variables. The factory function is still invoked only once and every consumer receives the sameClosure, just like with any other container variable:A
Closurein the container configuration is always treated as a factory function, so returning one from a factory function is the only way to provide it as a value. Until now, this was not supported because the cached result would be invoked as a factory again on the next access, which #302 turned into an explicitTypeError. The cache now wraps it in a newClosureinstead. Factory functions for class names are unchanged and must still return an object.This includes a new test case covering repeated access to the same
Closure. This has 100% code coverage and should be safe to apply.Builds on top of #302, #178, #307, #288 and others