hard coupling to zend/diactoros and slim

[ppetermann]

the composer.json requires zend-diactoros, and require-dev's slim
PhpDebugBarMiddleware.php actually directly uses zend-diactoros, and makes use in an instanceof comparison of Slims Uri class.

• the uses of zend-diactoros seem to be about building the response, this should be done through a Psr-17 factory, it should then also be moved to the require-dev's
• the use of slim framework seems to be to deal with an incompatibility (which should go away with slim/http) which would be better worked with if the slim uri was wrapped into psr-7 compatible class, which would do the translation, and only used when used within a slim context.

[spotman]

I agree with @ppetermann , hard coupling creates more issues than solves.
I'm using a new version of zend/diactoros (2.0) and I can not install this package with error:

Your requirements could not be resolved to an installable set of packages.

We can collaborate on this issue and make a PR for it. @snapshotpl will you merge that PR?

[snapshotpl]

@spotman no problem man :) However first of all I prefer to release fix with support zend-diactoros 1 and 2. Your PR will be very welcome!

[snapshotpl]

@spotman you can install zend-diactoros v2 now by #25 . psr-17 will come in near future.

[spotman]

@snapshotpl Thanks for quick response! I need some time to check your lib and to dive in the code. I'll let you know when I'll be ready to start working on this issue.

[snapshotpl]

@ppetermann I think slim dependency isn't a problem since is in require-dev. I don't see any benefit for end-user to create wrapper for slim users. My goal was create package working out-of-the-box.

[ppetermann]

I'm sorry, but I have no idea why having slim in require-dev would fix the hard coupling issue your package has.

You use Slim here, aliasing it as SlimUri
https://github.com/php-middleware/phpdebugbar/blob/master/src/PhpDebugBarMiddleware.php#L13-L17

You use the aliased SlimUri here:
https://github.com/php-middleware/phpdebugbar/blob/master/src/PhpDebugBarMiddleware.php#L132-L142

which is used here:
https://github.com/php-middleware/phpdebugbar/blob/master/src/PhpDebugBarMiddleware.php#L110

which is called here:
https://github.com/php-middleware/phpdebugbar/blob/master/src/PhpDebugBarMiddleware.php#L40

which is basically your main function, being called from __invoke.

thus, you've created a dependency which means slim must be installed in order for your lib to work. Which means:
everywhere else where your code is run, but slim is not explicitly installed, your lib will fail hard.

Which means:

• slim being in require-dev is a mistake, as it definitely needs slim during runtime
• everyone who is not using slim needs to have slim installed just to use your middleware.

[snapshotpl]

slim dependency is define only in require-dev because I need it in tests. However if your code base missing slim, there is no problem because instanceof doesn't need autoloading (more here http://php.net/manual/en/language.operators.type.php)

[ppetermann]

apologies, I wasn't aware that use and instanceof both don't need the class to be existing.

However I'd still consider abstracting it away.

[snapshotpl]

@ppetermann :)

Can you make PR for that? Also look at #27

[ppetermann]

@snapshotpl #27 seems a great improvement 👍

I'll see if i can prepare a PR as soon as i have some spare time for that, but its not on the highest priority looking at all the things I got on my plate right now - sorry for that.

[snapshotpl]

since we support psr-17 and slim is not required to install to lib work I think that we can close this issue