Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Allow psr/http-message ^2.0 #542

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

Open
valzargaming wants to merge 5 commits into reactphp:3.x
base: 3.x
Choose a base branch
Loading
from valzargaming:patch-1

Conversation

@valzargaming
Copy link

@valzargaming valzargaming commented May 17, 2025
edited
Loading

Requires php: ^7.2 || ^8.0, but still allowing usage of ^1.0 maintains backwards compatibility with the current PHP requirements
Closes #513

kwhat, szynek-fingoweb, and compwright reacted with eyes emoji
Requires php: ^7.2 || ^8.0
Closes reactphp#513 
Copy link

@clue @WyriHaximus would it be possible to get some eyes on this and released if all good?

composer.json Outdated
"evenement/evenement": "^3.0 || ^2.0 || ^1.0",
"fig/http-message-util": "^1.1",
"psr/http-message": "^1.0",
"psr/http-message": "^2.0 || ^1.0",
Copy link
Member

@WyriHaximus WyriHaximus May 22, 2025

Choose a reason for hiding this comment

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

The only way this will work is if we go:

Suggested change
"psr/http-message": "^2.0 || ^1.0",
"psr/http-message": "^2.0",

Reason for this is that we have a bunch of messages in https://github.com/reactphp/http/tree/3.x/src/Message that don't have a return type on those methods. If we'll allow 2.0 without adding that you'll get fatal errors, so IMHO we should switch to 2.0 and add the return types to all the relevant classes. /cc @clue

Copy link

Choose a reason for hiding this comment

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

@WyriHaximus #541 was not what you expect?

Copy link
Member

@WyriHaximus WyriHaximus May 26, 2025

Choose a reason for hiding this comment

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

@L3o-pold Aside from a few things, yes that should have worked.

Copy link

Choose a reason for hiding this comment

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

@WyriHaximus can you clarify what so we can move forward?

Copy link

Choose a reason for hiding this comment

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

@valzargaming I have opened a PR against your branch to add the minimum required to make this update work. I had to remove clue/http-proxy-react because it is only required for 1 example, and it eventually requires ringcentral/psr7 which in turn depends on "psr/http-message": "^1.0". You will need to rebase after merge because your branch is 2 commits behind and I didn't want to introduce a merge commit.

andrew-demb reacted with thumbs up emoji valzargaming reacted with heart emoji
Copy link

Choose a reason for hiding this comment

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

@valzargaming sorry, one more thing. I didn't realize the CI ran against 7.1, you will need to remove https://github.com/reactphp/http/blob/3.x/.github/workflows/ci.yml#L22

andrew-demb reacted with thumbs up emoji
Copy link
Author

@valzargaming valzargaming Jun 10, 2025

Choose a reason for hiding this comment

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

No worries. I don't have access to my PC right now, but if you make a PR for it I'll merge it.

kwhat reacted with thumbs up emoji
Copy link

Choose a reason for hiding this comment

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

@valzargaming PR opened.

andrew-demb reacted with thumbs up emoji
Copy link
Member

@WyriHaximus WyriHaximus Jun 10, 2025

Choose a reason for hiding this comment

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

@L3o-pold As mentioned in clue/reactphp-http-proxy#64 (comment) it boils down to this function call: https://github.com/clue/reactphp-http-proxy/blob/1.x/src/ProxyConnector.php#L187 Mento to respond to you earlier but life got in the way. Had a look at fixing it but didn't get far

Remove 7.1 from the CI matrix because psr/http-message ^2.0 requires 7.2 or later.
Copy link

Hi, what is the latest on this PR? Will this be merged? This is creating an incompatibility with the AWS SDK which requires 2.0

Thanks.

Copy link

kwhat commented Jun 14, 2025

@marcelfolaron I believe that is the goal. I think the part holding us back right now is the PR to clue/reactphp-http-proxy which would allow us to keep that examples/11-client-http-proxy.php in the codebase.

marcelfolaron reacted with thumbs up emoji WyriHaximus reacted with heart emoji

Copy link

@clue can i help on anything to get this one merged?

CodeWithKyrian, zleigh-digistorm, fergusean, and menturion reacted with thumbs up emoji peterjthomson reacted with eyes emoji

Copy link

Same here.
I am not able to use "react/http". Many other packages are downgraded just because of the "psr/http-message": "^1.0" dependency. Please merge this!

zleigh-digistorm and compwright reacted with thumbs up emoji

Copy link

RRosalia commented Sep 2, 2025

Any updates? Or any help needed we need this merged!

szynek-fingoweb, zleigh-digistorm, and compwright reacted with thumbs up emoji

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@WyriHaximus WyriHaximus WyriHaximus requested changes

+3 more reviewers

@kwhat kwhat kwhat left review comments

@L3o-pold L3o-pold L3o-pold left review comments

@thyagomuris thyagomuris thyagomuris approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add support for psr/http-message v2

AltStyle によって変換されたページ (->オリジナル) /