-
Notifications
You must be signed in to change notification settings - Fork 1.3k
GraphQL API should return absolute URLs #566
Description
The Sourcegraph GraphQL API currently returns relative URLs (without the appURL
), such as in this GraphQL query:
{
repository(name:"github.com/gorilla/mux") {
url
commit(rev:"master") {
url
}
}
}
{
"data": {
"repository": {
"url": "/github.com/gorilla/mux",
"commit": {
"url": "/github.com/gorilla/mux/-/commit/master"
}
}
}
}
This is convenient for our own frontend app, but it's not convenient for API clients. API clients need to (1) know that the result URL is relative and (2) resolve it with the base URL.
For example, I made an extension that shows Git blame annotations on GitHub. It gets the commit URL for a commit from the Sourcegraph GraphQL API and then uses that as the TextDocumentDecoration
's linkURL
. This works on Sourcegraph but not on GitHub because it is a value like /github.com/gorilla/mux/-/commit/521ea7b17d02faf8d3afea6737573942ceac59c5
. Used as a link URL on GitHub, that points to the invalid URL https://github.com/github.com/gorilla/mux/-/commit/521ea7b17d02faf8d3afea6737573942ceac59c5
. The extension could detect if it's running on non-Sourcegraph and resolve all URLs from our GraphQL API. But that's clunky, and most extension authors would not think to do that. (I'll do that workaround for now, though.)
Note that the solution is not as simple as "All URLs in our GraphQL API should be absolute." If we do that, then anytime we use a URL from the GraphQL API in our own frontend, it will break, because <Link to="...">
from react-router-dom
assumes its to
argument DOES NOT have a scheme and host. See remix-run/react-router#1147.
For this same reason, the solution should not require our frontend code's GraphQL queries to be changed. (E.g., I think it's a bad idea to make each GraphQL URL field accept a urlToWhatever(relative: Boolean = false): String!
param, because that's very easy to forget (and then the <Link>
will break).
Proposal:
- Make all URLs returned from our GraphQL API absolute.
- Create a new React component
<Link>
that checks whether theto
parameter starts with theappURL
; if so, it chops that off and uses react-router-dom's<Link>
, otherwise it uses<a href>
. - Add a linter rule to prevent usage of the react-router-dom
<Link>
directly (to avoid mistakes).