Move 'nyholm/psr7' from require to suggest & require-dev

[Doqnach]

Would it be possible to move the composer requirement for nyholm/psr7 to require-dev and suggest?

Right now the bundle orders an implementer to use nyholm/psr7 as their PSR-7 implementation, even if they might want to use something else (like guzzlehttp/psr7).

The bundle should work with both implementations right? There is no hard requirement for it being nyholm/psr7?

[Doqnach]

@qpautrat any chance you might have a look into this one as well?

[qpautrat]

Hello @Doqnach 👋 ,

Right now the bundle orders an implementer to use nyholm/psr7 as their PSR-7 implementation, even if they might want to use something else (like guzzlehttp/psr7).

Yes you are right.

The bundle should work with both implementations right? There is no hard requirement for it being nyholm/psr7?

Unfortunately it is.

So it is a bit more complicated than

Would it be possible to move the composer requirement for nyholm/psr7 to require-dev and suggest?

In this case we need to inject a ResponseFactoryInterface inside the JsonApiFactory.
Then use the ResponseFactory to create a Response.
The only thing I don't know here is how to provide a concrete implementation to the bundle ?
We can suggest as you said but it does not make the bundle fully operational.
Do you know what to do in this situation ?

[Doqnach]

The bundle should be able to work based on purely thepsr/http-message and psr/http-factory provided interfaces, given the assumption the bundle is actually meant to be Psr compliant. The bundle should not have to worry about implementation, that is up to the implementer of the bundle.

So you could require-dev nyholm/psr7 if you have any tests that require you to provide an actual implementation, and you can put it under suggest to tell implementers: "hey, you need a psr7 & psr17 implementation, here are ones that work*!, *) other options available".

By having the bundle require the psr/http-message and psr/http-factory it should tell the implementer that working with it will mean having to provide an implementation for it.

That concrete implementation should then be provided as a dependency-injection (e.g. the implementer can choose Nyholm\Psr7\Factory\ Psr17Factory) somewhere based on the ResponseFactoryInterface... perhaps simply in the JsonApiFactory::__construct() along with the other factories?

[qpautrat]

That is basically what I said. ;)
The only question that remains is as a bundle developer, should I provide a fully functional library ?

I red a bit about it:

A post from Matthias Noback.
And more Symfony based documentation.

It suggets that I should not provide concrete implementation, but what happens if autowire is disabled ? I still must inject a ResponseFactory inside the JsonApiFactory.

[qpautrat]

Because of this I'm confused.

[qpautrat]

We could force users to provide an alias in the configuration if autowiring is disabled or if they have multiple implementation of http-message-factory. 🤔

[Doqnach]

The bundle should not depend on autowiring internally, yeah. So anything that is used internally in method calls needs to be provided from the outside. So if JsonApiFactory requires an implementation of a ResponseFactoryInterface, this should be provided somewhere higher-up the chain by the implementer.

(I didn't trace the code yet to see how & where yet, above is just the general stance how I see it)

[qpautrat]

symfony/psr-http-message-bridge already suggest nyholm/psr7

But in the same time woohoolabs/yin requires psr/http-message-implementation.
It means that if someone wants to use the library it must require an implementation of psr/http-message by installing a package which provides this implementation.

So it confuses me.
Should we explicitly ask for psr/http-factory-implementation and let composer install fails ?
Or should we just let the DI of the framework fails by saying that it does not succeed to provide an implementation of Psr\Http\Message\ResponseFactoryInterface ?

What is your thought about it @Doqnach ?

I'm doing a PR pushing the second option and I will put you in review.

[Doqnach]

Should we explicitly ask for psr/http-factory-implementation and let composer install fails ?
Or should we just let the DI of the framework fails by saying that it does not succeed to provide an implementation of Psr\Http\Message\ResponseFactoryInterface ?

Going by the article you mentioned before:
I would say to not require psr/http-factory-implementation, but to let DI fail. This makes it easier for the implementer to use alternative sources to provide the implementation (like the article mentions, e.g. php extensions or the application itself).

[qpautrat]

I completely agree with you.
But it appears that woohoolabs/yin has not the same point of view

[Doqnach]

Is the composer update ran against dev environment? Then adding nyholm/psr7 in require-dev would satisfy the requirements for psr/http-factory-implementation?

When an implementer requires qpautrat/woohoolabs-yin-bundle, it would be up to them to satisfy this dependency (hence the suggest by the bundle maybe, not depending on what symfony/psr-http-message-bridge suggests)

[qpautrat]

#24 has been merged.
New major version version added.

Thank you !