Skip to content

Conversation

@clue
Copy link
Owner

@clue clue commented Dec 28, 2025

This changeset adds support for injecting the internal RouteHandler into the App explicitly or default to loading it from the Container. This is only used internally at the moment, but may be exposed as part of our public API in a follow-up. This also allows us to clean up the tests and avoid a number of reflection calls in our tests.

Builds on top of #290, #289, #282, #94, #42, and others

@clue clue added this to the v0.18.0 milestone Dec 28, 2025
@clue clue requested a review from Copilot December 28, 2025 18:29
@clue clue added maintenance new feature New feature or request labels Dec 28, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds support for explicitly injecting a RouteHandler into the App constructor or loading it from the Container. The primary goal is to make the framework's internal architecture more flexible while cleaning up tests by avoiding reflection calls.

Key changes:

  • RouteHandler constructor now requires a Container parameter (previously optional)
  • Container can now automatically instantiate RouteHandler with itself as dependency
  • App constructor accepts RouteHandler instances or class names, with validation that it must be last in the middleware chain

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/Io/RouteHandler.php Made Container parameter mandatory in constructor (breaking change for internal use only)
src/Container.php Added fallback to auto-instantiate RouteHandler with self-reference, similar to existing Container self-reference pattern
src/App.php Added logic to accept explicit RouteHandler instances/class names and validate they appear last in middleware chain
tests/Io/RouteHandlerTest.php Updated all tests to pass Container instance to RouteHandler constructor
tests/ContainerTest.php Added tests verifying RouteHandler can be loaded from container with proper container injection
tests/AppTest.php Simplified tests by injecting mocked RouteHandler directly instead of using reflection; added tests for new RouteHandler injection feature
tests/AppMiddlewareTest.php Simplified all HTTP method tests by injecting mocked RouteHandler instead of using reflection

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@clue clue merged commit e52bde8 into clue:main Dec 28, 2025
74 checks passed
@clue clue deleted the router-inject branch December 28, 2025 20:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintenance new feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant