Change PlatformContext.sourcegraphURL from string to Observable<URL>

Created by: felixfbecker

felix [1:13 AM] Dan’s PR made me think that requestGraphQL should probably get the sourcegraphURL as a parameter, else we double query it if the caller also queries for it Not a big issue, just a thought

loic [1:13 AM] Yup, good thought.

felix [1:16 AM] Or maybe we should just have a global Observable for sourcegraphURL instead of a function? That way it can buffer the latest value And share that between all callers publishReplay(1) would do that

loic [1:20 AM] As long as we do it in a way that allows us to set and change the sourcegraph URL in tests

felix [1:20 AM] Well do we have a way for that with the current global function?

loic [1:22 AM] No, the only real way to set it is to set window.SOURCEGRAPH_URL https://sourcegraph.com/github.com/sourcegraph/sourcegraph/-/blob/browser/src/platform/context.test.tsx#L8 sourcegraph.com Sourcegraph Sourcegraph is a web-based code search and navigation tool for dev teams. Search, navigate, and review code. Find answers.

felix [1:22 AM] Hmm I could imagine passing that Observable around so we can pass one from tests Also, then we don’t need to branch based on isInPage, the phabricator script can just provide a different Observable

loic [1:23 AM] Yeah, we pass it just like we now inject isExtension

felix [1:27 AM] And in the web app, it would always pass window.location.host

loic [1:28 AM] When we do this, let's take the opportunity to fix PlatformContext.sourcegraphURL, which is passed once and never updated https://sourcegraph.com/github.com/sourcegraph/sourcegraph/-/blob/shared/src/platform/context.ts#L111-123 sourcegraph.com Sourcegraph Sourcegraph is a web-based code search and navigation tool for dev teams. Search, navigate, and review code. Find answers.

felix [1:29 AM] perfect didn’t know that existed

loic [1:29 AM] The TODO on it makes me think it was added as a hacky workaround

felix [1:50 AM] another thing that would be nice is to change sourcegraphURL to be of type URL that way it is guaranteed it never is a weird value e.g. we accidentally passed an empty string

loic [1:51 AM] Or weird downstream failures due to setting malformed URLs in the popup or the options page.

felix [1:51 AM] yup

loic [1:54 AM] I also just noticed that the URL form in the browser extension doesn't trim the value entered by the user, which can break sourcegraphURL === DEFAULT_SOURCEGRAPH_URL checks, to disastrous effect...