-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Fix debug client after breaking change in dependency graphql-request #5899
Fix debug client after breaking change in dependency graphql-request #5899
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev-2.x #5899 +/- ##
=============================================
+ Coverage 68.61% 68.63% +0.02%
- Complexity 16823 16837 +14
=============================================
Files 1927 1927
Lines 72942 72955 +13
Branches 7476 7482 +6
=============================================
+ Hits 50050 50075 +25
+ Misses 20318 20299 -19
- Partials 2574 2581 +7 ☔ View full report in Codecov by Sentry. |
…nt with hostname if it does not have one
8339d62
to
0177051
Compare
try { | ||
// the URL constructor will throw exception if endpoint is not a valid URL, | ||
// e.g. if it is a relative path | ||
new URL(endpoint); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this line do anything?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it throws an exception if endpoint is not a valid URL
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤦 oh ok, reading the comments helps
As agreed with the dev team, this only needs a single reviewer. |
Debug client is broken if VITE_API_URL is a relative path (which it is by default). This is due to a breaking change in v7.0.0 of graphql-request, which I overlooked when approving the upgrade: https://github.com/jasonkuhrt/graphql-request/releases/tag/7.0.0
This PR checks validity of VITE_API_URL by passing it to the URL constructor. If exception is thrown, we prefix it with the window's origin (which used to be the default behavior).
I still need to test this PRTested locally.