Repository navigation
Avoid loading internal classes from PSR-11 container - #319
Merged
Merged
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues were identified for the change isolating framework internals from custom PSR-11 autowiring.
No open issues 🎉
What changed in this PR
Prevents custom PSR-11 autowiring containers from incorrectly creating Framework X internals, while preserving delegation for application classes and configuration.
Changes:
- Creates the container self-reference, router, and default CLI HTTP runner without consulting the PSR-11 adapter for those classes.
- Preserves explicit custom runner selection through
X_EXPERIMENTAL_RUNNER. - Adds regression tests and documents the adapter behavior.
| File | Description |
|---|---|
| tests/ContainerTest.php | Tests internal-class isolation and explicit runner selection. |
| tests/AppTest.php | Verifies default runner creation and router use of the supplied container. |
| src/Container.php | Bypasses PSR-11 lookup for selected internal classes and the default CLI runner. |
| docs/best-practices/controllers.md | Explains custom-container delegation and explicit HTTP runner configuration. |
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 updates the
Containerto avoid loading its internal classes from a custom PSR-11 container. Previously, autowiring containers such as PHP-DI would report every instantiable class as available, so X would ask them for its ownContainer,RouteHandlerandHttpServerRunner. With PHP-DI,new App(new Container(new DI\Container()))would throw in CLI because it can not create theHttpServerRunner, and the router would load controllers and middleware from a freshContainerinstead of the given one. X now creates these itself by default, while your controllers, middleware and configuration are still loaded from the given container:These regressions from #292 and #291 have not been released yet, v0.17.0 is not affected. If you want to configure the HTTP server runner, such as for a larger request body limit via #311, you can still define it in your custom container and load it by setting
X_EXPERIMENTAL_RUNNERto its class name.The built-in container works exactly as before. This includes new test cases covering the PSR-11 adapter, so this has 100% code coverage and should be safe to apply.
Builds on top of #292, #291, #290, #295, #311, #289, #294, #163 and others