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

Slim 4 support #46

Closed
wants to merge 4 commits into from
Closed

Slim 4 support #46

wants to merge 4 commits into from

Conversation

fritsvt
Copy link
Contributor

@fritsvt fritsvt commented Aug 4, 2019

Did some hacking today and managed to get it working in Slim 4.

I still need to clean things up and add/change tests but it all seems to work so far.

I had to change the way the App gets created because this seemed to have changed in Slim 4.

A basic example of booting up an app would now look like this:

<?php

use DI\Container;
use Slim\Factory\AppFactory;
use DI\Bridge\Slim\App;

require __DIR__ . '/../vendor/autoload.php';

// Create Container using PHP-DI
$container = new Container();

// Set container to create App with on AppFactory
AppFactory::setContainer($container);

// boot Slim4 app with DI
$app = App::boot($container);

Will continue to work on this in the coming days to hopefully get it all working.

EDIT: Changed the behaviour to still be the same as before. So the DI-Bridge App would accept the same parameters as the Appfactory::create method does. Only changed it so the container becomes the first parameter because that's the one people would use most.

Example would now look like:

<?php

use DI\Bridge\Slim\App;
use Psr\Http\Message\ServerRequestInterface as Request;
use Slim\Psr7\Response;

require __DIR__ . '/../vendor/autoload.php';

$app = new App;

$app->get('/{name}', function ($name, Request $request, Response $response) {
    $response->getBody()->write("Hello, $name");
    return $response;
});

$app->run();

So this would create a DI container for you

I removed the ErrorTest and ContainerTest because I think that behaviour changed. But please let me know if I'm wrong there.

I can't seem to fix one of the Travis checks. So if someone knows why that's failing please let me know.

@mnapoli when you have time, please let me know what you think :)

resolves #22

@fritsvt fritsvt changed the title WIP: Slim 4 support Slim 4 support Aug 5, 2019
@mnapoli
Copy link
Member

mnapoli commented Aug 5, 2019

Thanks! I'm sorry I saw the PR yesterday and worked today on fixing the tests. It seems we have both fixed the test, sorry about that!

I managed to include some of your commit into that branch, I'll push a PR. I decided to mimic the behavior of the AppFactory from Slim, instead of extending App. I think that will be more in line with what's documented in Slim.

@mnapoli mnapoli mentioned this pull request Aug 5, 2019
2 tasks
@fritsvt
Copy link
Contributor Author

fritsvt commented Aug 6, 2019

Alright, I wasn't sure what to do with the behaviour so I decided to keep it the same in the end. But I agree that keeping it similar to the framework is better. New PR looks good 👍

@fritsvt
Copy link
Contributor Author

fritsvt commented Aug 7, 2019

Closing this so v4 can move to #47

@fritsvt fritsvt closed this Aug 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Slim 4 support
2 participants