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

[6.x] Utilize Symfony’s PSR Factory #31018

Conversation

ivannovak
Copy link

@ivannovak ivannovak commented Jan 3, 2020

  • replace DiactorosFactory with Symfony’s PSR Factory and recommended implementation
  • replace Diactoros Response with Nyholm’s Response per Symfony docs recommendation

Closes #31017.

Ivan Novak added 2 commits January 3, 2020 10:42
- replace DiactorosFactory with Symfony’s PSR Factory and recommended implementaiton
- replace Diactoros Resposne with Nyholm’s Response per Symfony docs recommendation
@GrahamCampbell GrahamCampbell changed the title Utilize Symfony’s PSR Factory. Fixes #31017 [6.x] Utilize Symfony’s PSR Factory. Fixes #31017 Jan 3, 2020
@GrahamCampbell
Copy link
Member

GrahamCampbell commented Jan 3, 2020

Thanks for the PR. Please update the suggest blocks in the composer.json file.

@GrahamCampbell
Copy link
Member

(in both composer.json files)

Ivan Novak added 2 commits January 3, 2020 11:20
- updated routing composer on psr-http-bridge language inline with base composer file
@ivannovak
Copy link
Author

FYI - Updated composer files

@GrahamCampbell GrahamCampbell changed the title [6.x] Utilize Symfony’s PSR Factory. Fixes #31017 [6.x] Utilize Symfony’s PSR Factory Jan 3, 2020
@GrahamCampbell
Copy link
Member

Thanks for this. I've left some more comments. :)

if (class_exists(ZendPsrResponse::class)) {
return new ZendPsrResponse;
}

throw new Exception('Unable to resolve PSR response. Please install nyholm/psr7 or laminas/laminas-diactoros.');
Copy link
Member

Choose a reason for hiding this comment

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

Since we're not actually supporting zendframework/zend-diactoros as per the previous discussion, and the zend stuff is now deprecated, maybe we should only recommend nyholm/psr7 here?

Copy link
Member

Choose a reason for hiding this comment

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

So, we'll have:

Unable to resolve PSR response. Please install nyholm/psr7.

if (class_exists(DiactorosFactory::class)) {
return (new DiactorosFactory)->createRequest($app->make('request'));
}

throw new Exception('Unable to resolve PSR request. Please install nyholm/psr7 or laminas/laminas-diactoros.');
Copy link
Member

@GrahamCampbell GrahamCampbell Jan 4, 2020

Choose a reason for hiding this comment

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

We should probably replace this message with:

Unable to resolve PSR request. Please install symfony/psr-http-message-bridge.

Copy link
Author

@ivannovak ivannovak Jan 4, 2020

Choose a reason for hiding this comment

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

Probably should be both since that Psr17Factory comes from the nyholm package?

Unable to resolve PSR request. Please install symfony/psr-http-message-bridge and nyholm/psr7.

@taylorotwell taylorotwell merged commit 15d6744 into laravel:6.x Jan 4, 2020
iBotPeaches referenced this pull request in laravel/passport Mar 31, 2020
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.

Symfony Http Message Bridge Dropped Support for Diactoros
3 participants