Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve ease of extention #89

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

andrewnicols
Copy link
Contributor

In our usage we need to extend the ControllerInvoker to have responses normalised from a generic Payload into a standard Response.

To make this easier it would be helpful if some private methods were instead protected and the Invoker fetching the default resolver chain were separated out from the creation of the ControllerInvoker.

Fixes #105

You can see our proposed implementation of the ControllerInvoker here. With the proposed patch here, that would be vastly simplified and easier to maintain, something like:

    #[\Override]
    protected function processResponse($response) {
        return \core\di::get(response_handler::class)->standardise_response($response);
    } 

Likewise our custom Bridge implementation would be vastly simplified by breaking apart the Bridge::createControllerInvoker method from this, to:

class bridge extends \DI\Bridge\Slim\Bridge {
    #[\Override]
    protected static function createControllerInvoker(ContainerInterface $container): ControllerInvoker {
        return new controller_invoker(self::createInvoker($container));
    }
}

In our usage we need to extend the ControllerInvoker to have responses
normalised from a generic Payload into a standard Response.

To make this easier it would be helpful if some private methods were
instead protected and the Invoker fetching the default resolver chain
were separated out from the creation of the ControllerInvoker.

Fixes #105
@mnapoli
Copy link
Member

mnapoli commented Jun 20, 2024

Thanks for both PRs!

My goal with this bridge is to provide an opinionated default for Slim so that users can get started immediately.

The bridge itself is extremely simple. If you have advanced needs, I think it makes much more sense to copy the classes and edit them, they're super easy to understand.

However if we want to change the defaults I'm all ears, but #86 (comment) tells me that the bridge is currently aligned with how Slim works, so I'm not sure we want to change them.

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