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

Replace deprated/removed GuzzleHttp\Psr7\build_query #500

Merged
merged 2 commits into from
May 5, 2021

Conversation

bartvanraaij
Copy link
Contributor

@bartvanraaij bartvanraaij commented May 5, 2021

Description

This PR replaces the Psr7\build_query() calls with Psr7\Query::build().
Psr7\build_query() is deprecated in version 1.x of guzzlehttp/psr7 and will be removed in 2.0.

This also lead me to see that this project doesn't explicitely require guzzlehttp/psr7 as a dependency, but instead relies on guzzlehttp/guzzle to supply it, which is not safe to assume. Because methods of guzzlehttp/psr7 are used in this package, you should explicitely require guzzlehttp/psr7 as a dependency in this package.

Guzzlehttp/guzzle changed their requirement of guzzlehttp/psr7 to ^1.7 || ^2.0.
After 2.0 will be stable, if you composer require auth0/auth0-php, it will break because auth0-php will require guzzlehttp/guzzle which in turn will install guzzlehttp/psr7 version 2, which will not have the build_query function that auth0-php uses.
You can already see this behaviour by setting your composer minimum stability to dev.

To mitigate this, this PR also explicitely requires guzzlehttp/psr7 1.x as a dependency in composer.json.

References

psr7^2.0 added to guzzle: guzzle/guzzle@0103c78#diff-d2ab9925cad7eac58e0ff4cc0d251a937ecf49e4b6bf57f8b95aab76648a9d34

Testing

Any static analysis tool will tell you it doesn't use deprecated methods anymore.

Checklist

  • I have added documentation for new/changed functionality in this PR or in auth0.com/docs
  • [] All active GitHub checks for tests, formatting, and security are passing
  • The correct base branch is being used, if not master

@bartvanraaij bartvanraaij requested a review from a team as a code owner May 5, 2021 14:37
Copy link
Member

@evansims evansims left a comment

Choose a reason for hiding this comment

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

Hey, @bartvanraaij 👋 Good catch on this, thank you! I really appreciate your contribution here. We'll get this merged for the next release.

@evansims evansims added this to the 7.10.0 milestone May 5, 2021
@evansims evansims merged commit 7a58886 into auth0:master May 5, 2021
@kangadrewie
Copy link

Hi @evansims - Has this change been released yet?

@evansims
Copy link
Member

evansims commented Jul 6, 2021

Hi @kangadrewie there hasn't been a release containing this change as of yet.

@kangadrewie
Copy link

kangadrewie commented Jul 6, 2021

Ok thanks @evansims ! I have changed it manually but fyi the latest composer require auth0/auth0-php is currently broken as a result of this.

@evansims
Copy link
Member

evansims commented Jul 6, 2021

Thanks for the heads up on that! I'll cut a bug-fix release today in that case 👍

@evansims evansims added this to the 7.9.1 milestone Jul 6, 2021
@evansims
Copy link
Member

evansims commented Jul 6, 2021

🚀 Shipped in 7.9.1

@github-actions
Copy link
Contributor

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants