From e4a8ce52daaf1c755e2430c0a5cd90890bc8d642 Mon Sep 17 00:00:00 2001 From: Felix Kling Date: Tue, 6 Aug 2024 11:53:50 +0200 Subject: [PATCH] fix(svelte): Update top-level route list (#64272) tl;dr: Everything is derived from `window.context.svelteKit.knownRoutes` *buckle up* ### Context After the new web app was enabled by default on dotcom I looked at dotcom data in Sentry to see if there was anything out of the ordinary. I noticed a high number of errors related to resolving repository information. The most common non-existing repository that was reported was `-/sign-out`. Of course this shouldn't happen. `/-/sign-out` is the sign out URL and shouldn't be interpreted as a repository. Currently we prevent SvelteKit from interpreting specific paths as repositories by using the `reporev` parameter validator: ```ts // src/params/reporev.ts const topLevelPaths = [ 'insights', 'search-jobs', 'saved-searches', // ... many more here ] const topLevelPathRegex = new RegExp(`^(${topLevelPaths.join('|')})($|/)`) // This ensures that we never consider paths containing /-/ and pointing // to non-existing pages as repo name export const match: ParamMatcher = param => { // Note: param doesn't have a leading slash return !topLevelPathRegex.test(param) && !param.includes('/-/') } ``` `-/sign-out` was not in that list, including others which have been added since this file was originally created. Adding it would have been the simple solution but having to maintain this list manually has always irked me. Now we have a better way... ### Production changes #### Client side It turns out we are already sending a list of known routes to the client in `window.context.svelteKit.knownRoutes` to basically do the same thing: test whether a given path is a page or a repository. ```ts // src/lib/navigation.ts let knownRoutesRegex: RegExp | undefined function getKnownRoutesRegex(): RegExp { if (!knownRoutesRegex) { knownRoutesRegex = new RegExp(`(${window.context?.svelteKit?.knownRoutes?.join(')|(')})`) } return knownRoutesRegex } // ... export function isRouteEnabled(pathname: string): boolean { let foundRoute: SvelteKitRoute | undefined // [...] if (foundRoute) { if (foundRoute.isRepoRoot) { // Check known routes to see if there is a more specific route than the repo root. // If yes then we should load the React app (if the more specific route was enabled // it would have been found above). return !getKnownRoutesRegex().test(pathname) } return true } return false } ``` Why do we have the `reporev` validator and `isRouteEnabled`? They basically do the same thing (check whether a path is a known page or a repository) the first (`reporev`) is used by SvelteKit to determine which route a path corresponds to (i.e. to navigate to the repository page or whether the page doesn't exist) and the second one (`isRouteEnabled`) is used after the route resolution but before route loading. It's used to trigger a full page refresh to fetch the React version if necessary. Anyways, since we already have a list of known pages from the server, the parameter validator should just use it too. Except it didn't work, which made the following server side changes necessary. #### Server We register routes in multiple places. Right now `knownRoutes` is populated from the router created in `cmd/frontend/internal/app/ui/router.go`, but this does not include `/-/sign-out`. This route (and others) are defined in `cmd/frontend/internal/app/router/router.go` (I don't know what the difference is). I extended the sveltekit route registration code so that's possible to register routes from multiple routers. I couldn't test it yet however because I currently can't get Sourcegraph to run locally (Camden tested it; it works). ### Development mode changes After the above changes, navigating to a React only page in development, e.g. `/insights` would show a 'repo not found error' because `window.context.svelteKit.knownRoutes` was empty in development. So every non-existing page would be interpreted as missing repository. Hardcoding a list of known pages *again* just for development seemed to be a step backwards. So I spent quite a bit of time to find a way to extract the JS context object from the HTML page returned by the origin server and inject it into the HTML page generated by SvelteKit, similar to how we do it for the React app. Additionally the value of `window.context.svelteKit.knownRoutes` is now used to setup the proxy to non-supported pages from the server, so we don't have to maintain this regex anymore either: ```ts // Proxy requests to specific endpoints to a real Sourcegraph // instance. '^(/sign-(in|out)|/.assets|/-|/.api|/.auth|/search/stream|/users|/notebooks|/insights|/batch-changes|/contexts)|/-/(raw|own|code-graph|batch-changes|settings)(/|$)': { ... } ``` This means that requesting non-svelte pages will also return the corresponding React version in development. ## Test plan Manual testing. --- client/web-sveltekit/.env | 5 - client/web-sveltekit/.env.dotcom | 5 - client/web-sveltekit/BUILD.bazel | 2 - client/web-sveltekit/dev/vite-sg-proxy.ts | 255 ++++++++++++++++++ client/web-sveltekit/playwright.config.ts | 4 + client/web-sveltekit/src/app.html | 27 +- client/web-sveltekit/src/lib/navigation.ts | 16 +- client/web-sveltekit/src/params/reporev.ts | 72 +---- .../(code)/-/blob/[...path]/page.spec.ts | 2 +- .../(validrev)/(code)/page.spec.ts | 33 ++- .../web-sveltekit/src/routes/layout.spec.ts | 4 +- .../web-sveltekit/src/testing/integration.ts | 39 ++- client/web-sveltekit/vite.config.ts | 24 +- cmd/frontend/internal/app/router/BUILD.bazel | 1 + cmd/frontend/internal/app/router/router.go | 6 + .../internal/app/ui/sveltekit/sveltekit.go | 8 +- 16 files changed, 361 insertions(+), 142 deletions(-) delete mode 100644 client/web-sveltekit/.env delete mode 100644 client/web-sveltekit/.env.dotcom create mode 100644 client/web-sveltekit/dev/vite-sg-proxy.ts diff --git a/client/web-sveltekit/.env b/client/web-sveltekit/.env deleted file mode 100644 index 9d01e51baa3..00000000000 --- a/client/web-sveltekit/.env +++ /dev/null @@ -1,5 +0,0 @@ -PUBLIC_DOTCOM= -PUBLIC_CODY_ENABLED_ON_INSTANCE=true -PUBLIC_CODY_ENABLED_FOR_CURRENT_USER=true -PUBLIC_BATCH_CHANGES_ENABLED=true -PUBLIC_CODE_INSIGHTS_ENABLED=true diff --git a/client/web-sveltekit/.env.dotcom b/client/web-sveltekit/.env.dotcom deleted file mode 100644 index 65ae7b2b3e3..00000000000 --- a/client/web-sveltekit/.env.dotcom +++ /dev/null @@ -1,5 +0,0 @@ -PUBLIC_DOTCOM=true -PUBLIC_CODY_ENABLED_ON_INSTANCE=true -PUBLIC_CODY_ENABLED_FOR_CURRENT_USER=true -PUBLIC_BATCH_CHANGES_ENABLED= -PUBLIC_CODE_INSIGHTS_ENABLED= diff --git a/client/web-sveltekit/BUILD.bazel b/client/web-sveltekit/BUILD.bazel index 0f905f2c126..3130cb3bc6e 100644 --- a/client/web-sveltekit/BUILD.bazel +++ b/client/web-sveltekit/BUILD.bazel @@ -18,8 +18,6 @@ SRCS = [ ".eslintignore", ".eslintrc.cjs", ".prettierignore", - ".env", - ".env.dotcom", "//client/wildcard:sass-breakpoints", "//client/wildcard:global-style-sources", "//client/web/dist/img:copy", diff --git a/client/web-sveltekit/dev/vite-sg-proxy.ts b/client/web-sveltekit/dev/vite-sg-proxy.ts new file mode 100644 index 00000000000..92d39e41691 --- /dev/null +++ b/client/web-sveltekit/dev/vite-sg-proxy.ts @@ -0,0 +1,255 @@ +import type { IncomingMessage } from 'http' +import { Transform } from 'stream' + +import { createLogger, type Plugin, type ProxyOptions } from 'vite' + +import { svelteKitRoutes, type SvelteKitRoute } from '../src/lib/routes' + +interface Options { + target: string +} + +/** + * This plugin proxies certain requests to a real Sourcegraph instance. These include + * - API and auth requests + * - asset requests for the React app + * - requests for pages that are not known by the SvelteKit app (e.g. code insights) + * + * It does this by first fetching the sign-in page from the real Sourcegraph instance + * and extracting the JS context object from it. This object contains a list of + * routes known by the server, some of which will be handled by the SvelteKit app. + * (the other data in JS context is ignored) + * Those that are not will be proxied to the real Sourcegraph instance. + * + * Additionally, the plugin injects the JS context provided by the origin server into + * locally generated HTML pages. + * + * This plugin is only enabled in 'serve' mode. + */ +export function sgProxy(options: Options): Plugin { + const name = 'sg:proxy' + const logger = createLogger(undefined, { prefix: `[${name}]` }) + // Needs to be kept in sync with app.html + const contextPlaceholder = '// ---window.context---' + + // Additional endpoints that should be proxied to the real Sourcegraph instance. + // These are not part of the known routes, but are required for the SvelteKit app to work. + const additionalEndpoints = ['/.api/', '/.assets/', '/.auth/'] + + // Routes known by the server that need to (potentially) be proxied to the real Sourcegraph instance. + let knownServerRoutes: string[] = [] + + function extractContextRaw(body: string): string | null { + const match = body.match(/window\.context\s*=\s*{.*}/) + return match?.[0] ?? null + } + + function extractContext(body: string): Window['context'] | null { + const context = extractContextRaw(body) + if (!context) { + return null + } + return new Function(`return ${context.match(/\{.*\}/)?.[0] ?? ''}`)() + } + + /** + * Returns true if the request should be handled by the SvelteKit app. This uses similar + * logic to the `isRouteEnabled` function in the SvelteKit app. + * If the request + */ + function isHandledBySvelteKit(req: IncomingMessage, knownRoutes: string[]) { + const url = new URL(req.url ?? '', `http://${req.headers.host}`) + let foundRoute: SvelteKitRoute | undefined + + for (const route of svelteKitRoutes) { + if (route.pattern.test(url.pathname)) { + foundRoute = route + if (!route.isRepoRoot) { + break + } + } + } + + if (foundRoute) { + return foundRoute.isRepoRoot ? !knownRoutes.some(route => new RegExp(route).test(url.pathname)) : true + } + return false + } + + return { + name, + apply: 'serve', + async config() { + if (!options.target) { + logger.info('No target specified, not proxying requests', { timestamp: true }) + return + } + + let context: Window['context'] | null + + // At startup we fetch the sign-in page from the real Sourcegraph instance to extract the `knownRoutes` array + // from the JS context object. This is used to determine which requests should be proxied to the real Sourcegraph + // instance. + try { + // The /sign-in endpoint is always available on dotcom and enterprise instances. + context = await fetch(`${options.target}/sign-in`) + .then(response => response.text()) + .then(extractContext) + } catch (error) { + console.error(error) + logger.error(`Failed to fetch JS context: ${(error as Error).message}`, { timestamp: true }) + return + } + + if (!context) { + logger.error('Failed to extract JS context from origin', { timestamp: true }) + return + } + + knownServerRoutes = context.svelteKit?.knownRoutes ?? [] + if (!knownServerRoutes.length) { + logger.error('Failed to extract known routes from JS context', { timestamp: true }) + return + } + + logger.info(`Known routes from origin JS context\n - ${knownServerRoutes.join('\n - ')}\n`, { + timestamp: true, + }) + + const baseOptions: ProxyOptions = { + target: options.target, + changeOrigin: true, + secure: false, + headers: context.xhrHeaders, + } + + const proxyConfig: Record = { + // Proxy requests to specific endpoints to a real Sourcegraph instance. + [`^(${additionalEndpoints.join('|')})`]: baseOptions, + } + + const dynamicOptions: ProxyOptions = { + bypass(req) { + if (!req.url) { + return null + } + // If the request is for a SvelteKit route, we want to serve the SvelteKit app. + return isHandledBySvelteKit(req, knownServerRoutes) ? req.url : null + }, + ...baseOptions, + } + + for (const route of knownServerRoutes) { + // vite's proxy server matches full URL, including query parameters. + // That means a route regex like `^/search[/]?$` (which the server provides) + // would not match `/search?q=foo`. We extend every route regex to allow + // for any query parameters + proxyConfig[route.replace(/\$$/, '(\\?.*)?$')] = dynamicOptions + } + + return { + server: { + proxy: proxyConfig, + }, + } + }, + configureServer(server) { + if (!options.target) { + return + } + + server.middlewares.use(function proxyHTML(req, res, next) { + // When a request is made for an HTML page that is handled by the SvelteKit + // we want to inject the same JS context object that we would have fetched + // from the origin server. + // The implementation is quite hacky but apparently but it seems there is no + // better way to do this. It was inspired by the express compression middleware: + // https://github.com/expressjs/compression/blob/f3e6f389cb87e090438e13c04d67cec9e22f8098/index.js + if (req.headers.accept?.includes('html') && isHandledBySvelteKit(req, knownServerRoutes)) { + const setHeader = res.setHeader + const write = res.write + const on = res.on + const end = res.end + + const context = fetch(`${options.target}${req.url}`, { + headers: req.headers.cookie ? { cookie: req.headers.cookie } : {}, + }) + .then(response => response.text()) + .then(body => { + const context = extractContextRaw(body) + if (!context) { + throw new Error('window.context not found in response from origin') + } + return context + }) + + const transform = new Transform({ + transform(chunk, encoding, callback) { + context + .then(context => { + let body = Buffer.from(chunk).toString() + if (body.includes(contextPlaceholder)) { + body = body.replace(contextPlaceholder, context) + logger.info(`${req.url} - injected JS context`, { timestamp: true }) + } + callback(null, body) + }) + .catch(error => { + logger.error(`Error fetching JS context: ${error.message}`, { timestamp: true }) + callback(error, chunk) + }) + }, + }) + transform + .on('data', chunk => { + // @ts-expect-error - the overload signature of write seems to prevent TS from recognizing the correct arguments + if (write.call(res, chunk) === false) { + transform.pause() + } + }) + .on('end', () => { + // @ts-expect-error - the overload signature of end seems to prevent TS from recognizing the correct arguments + end.call(res) + }) + + res.on('drain', () => transform.resume()) + + let ended = false + + res.setHeader = (name, value) => { + // content-length is set and sent before we have a chance to modify the response + // we need to ignore it, otherwise the browser will not render the page + // properly + return name === 'content-length' ? res : setHeader.call(res, name, value) + } + + // @ts-expect-error - the overload signature of write seems to prevent TS from recognizing the correct arguments + res.write = (chunk, encoding, cb) => { + if (ended) { + return false + } + return transform.write(chunk, encoding, cb) + } + + // @ts-expect-error - the overload signature of write seems to prevent TS from recognizing the correct arguments + res.end = (chunk, encoding, cb) => { + if (ended) { + return false + } + ended = true + return transform.end(chunk, encoding, cb) + } + + // @ts-expect-error - the overload signature of write seems to prevent TS from recognizing the correct arguments + res.on = (type, listener) => { + if (type === 'drain') { + return transform.on(type, listener) + } + return on.call(res, type, listener) + } + } + next() + }) + }, + } +} diff --git a/client/web-sveltekit/playwright.config.ts b/client/web-sveltekit/playwright.config.ts index 1dd818ea786..63ef96e5817 100644 --- a/client/web-sveltekit/playwright.config.ts +++ b/client/web-sveltekit/playwright.config.ts @@ -11,6 +11,10 @@ const config: PlaywrightTestConfig = { command: 'pnpm build:preview && pnpm preview', port: PORT, reuseExistingServer: true, + env: { + PLAYWRIGHT: '1', + }, + timeout: 5 * 60_000, } : undefined, reporter: 'list', diff --git a/client/web-sveltekit/src/app.html b/client/web-sveltekit/src/app.html index 0dc9e416cd3..85ca93a3004 100644 --- a/client/web-sveltekit/src/app.html +++ b/client/web-sveltekit/src/app.html @@ -12,22 +12,19 @@ diff --git a/client/web-sveltekit/src/lib/navigation.ts b/client/web-sveltekit/src/lib/navigation.ts index adbc7cba572..592817766f4 100644 --- a/client/web-sveltekit/src/lib/navigation.ts +++ b/client/web-sveltekit/src/lib/navigation.ts @@ -1,4 +1,4 @@ -import { dev } from '$app/environment' +import { browser, dev } from '$app/environment' import { svelteKitRoutes, type SvelteKitRoute } from './routes' @@ -6,11 +6,21 @@ let knownRoutesRegex: RegExp | undefined function getKnownRoutesRegex(): RegExp { if (!knownRoutesRegex) { - knownRoutesRegex = new RegExp(`(${window.context?.svelteKit?.knownRoutes?.join(')|(')})`) + const knownRoutes = (browser && window.context?.svelteKit?.knownRoutes) || [] + knownRoutesRegex = + knownRoutes.length === 0 ? /$^/ : new RegExp(`(${window.context?.svelteKit?.knownRoutes?.join(')|(')})`) } return knownRoutesRegex } +/** + * Returns true if the given pathname is a known sub page. + * This depends on the list of known routes provided by the server. + */ +export function isKnownSubPage(pathname: string): boolean { + return getKnownRoutesRegex().test(pathname) +} + /** * Returns whether the SvelteKit app is enabled for the given route ID. * If not the caller should trigger a page reload to load the React app. @@ -55,7 +65,7 @@ export function isRouteEnabled(pathname: string): boolean { // Check known routes to see if there is a more specific route than the repo root. // If yes then we should load the React app (if the more specific route was enabled // it would have been found above). - return !getKnownRoutesRegex().test(pathname) + return !isKnownSubPage(pathname) } return true } diff --git a/client/web-sveltekit/src/params/reporev.ts b/client/web-sveltekit/src/params/reporev.ts index 23aedbfe721..cd38718500d 100644 --- a/client/web-sveltekit/src/params/reporev.ts +++ b/client/web-sveltekit/src/params/reporev.ts @@ -1,75 +1,11 @@ import type { ParamMatcher } from '@sveltejs/kit' -// These are top level paths that are Sourcegraph pages, not repositories. -// By explicitly excluding them we force SvelteKit to _not_ match them, which -// will cause SvelteKit to fetch the page from the server, which will then -// load the React version. -// Note that any routes in the `routes/` directory will be handled by the -// SvelteKit app, whether they are in this list or not. -// This list is taken from 'cmd/frontend/internal/app/ui/router.go'. -const topLevelPaths = [ - 'insights', - 'search-jobs', - 'saved-searches', - 'setup', - 'batch-changes', - 'code-monitoring', - 'notebooks', - 'request-access', - 'api/console', - 'sign-in', - 'ping-from-self-hosted', - 'sign-up', - 'threads', - 'organizations', - 'teams', - 'settings', - 'site-admin', - 'snippets', - 'subscriptions', - 'views', - 'own', - 'contexts', - 'registry', - 'search/cody', - 'app', - 'cody', - 'post-sign-up', - 'unlock-account', - 'password-reset', - 'survey', - 'embed', - 'users', - 'user', - 'search', - - // sourcegraph.com specific routes that redirect to subdomains - // are ignored (for now) - - // community search contexts - 'kubernetes', - 'stanford', - 'stackstorm', - 'temporal', - 'o3de', - 'chakraui', - 'julia', - 'backstage', - - // legacy routes - 'login', - 'careers', - 'extensions', - - // Help pages - 'help', -] - -const topLevelPathRegex = new RegExp(`^(${topLevelPaths.join('|')})($|/)`) +import { isKnownSubPage } from '$lib/navigation' // This ensures that we never consider paths containing /-/ and pointing // to non-existing pages as repo name export const match: ParamMatcher = param => { - // Note: param doesn't have a leading slash - return !topLevelPathRegex.test(param) && !param.includes('/-/') + // Note: /-/ is a separator between repo revision and repo sub pages + // Note 2: param doesn't have a leading slash + return !param.includes('/-/') && !isKnownSubPage('/' + param) } diff --git a/client/web-sveltekit/src/routes/[...repo=reporev]/(validrev)/(code)/-/blob/[...path]/page.spec.ts b/client/web-sveltekit/src/routes/[...repo=reporev]/(validrev)/(code)/-/blob/[...path]/page.spec.ts index 9366a8ecfa2..caafc80d9ae 100644 --- a/client/web-sveltekit/src/routes/[...repo=reporev]/(validrev)/(code)/-/blob/[...path]/page.spec.ts +++ b/client/web-sveltekit/src/routes/[...repo=reporev]/(validrev)/(code)/-/blob/[...path]/page.spec.ts @@ -152,7 +152,7 @@ test.describe('file header', () => { }), }) - sg.signIn({ username: 'test' }) + await sg.signIn({ username: 'test' }) await page.goto(url) const link = page.getByLabel('Open in IntelliJ IDEA') await expect(link, 'links to correct editor').toHaveAttribute( diff --git a/client/web-sveltekit/src/routes/[...repo=reporev]/(validrev)/(code)/page.spec.ts b/client/web-sveltekit/src/routes/[...repo=reporev]/(validrev)/(code)/page.spec.ts index 9a6860fbbac..4b04b5c1f97 100644 --- a/client/web-sveltekit/src/routes/[...repo=reporev]/(validrev)/(code)/page.spec.ts +++ b/client/web-sveltekit/src/routes/[...repo=reporev]/(validrev)/(code)/page.spec.ts @@ -325,26 +325,25 @@ test.describe('cody sidebar', () => { } test.describe('dotcom', () => { - test.beforeEach(({ sg }) => { - sg.dotcomMode() + test.beforeEach(async ({ sg }) => { + await sg.dotcomMode() }) - test('enabled when signed out', async ({ page, sg }) => { + test('disabled when signed out', async ({ page }) => { await page.goto(path) - await hasCody(page) - await expect( - page.getByText('Cody is only available to signed-in users. Sign in to use Cody.') - ).toBeVisible() + await doesNotHaveCody(page) }) test('enabled when signed in', async ({ page, sg }) => { - sg.signIn() + await sg.signIn() await page.goto(path) await hasCody(page) }) test('ignores context filters', async ({ page, sg }) => { + await sg.signIn() + sg.mockTypes({ Site: () => ({ codyContextFilters: { @@ -361,8 +360,8 @@ test.describe('cody sidebar', () => { }) test.describe('enterprise', () => { - test.beforeEach(({ sg }) => { - sg.signIn() + test.beforeEach(async ({ sg }) => { + await sg.signIn() sg.mockTypes({ Site: () => ({ @@ -373,11 +372,11 @@ test.describe('cody sidebar', () => { }) }) - test.fixme('disabled when disabled on instance', async ({ page, sg }) => { + test('disabled when disabled on instance', async ({ page, sg }) => { // These tests seem to take longer than the default timeout test.setTimeout(10000) - sg.setWindowContext({ + await sg.setWindowContext({ codyEnabledOnInstance: false, }) @@ -385,11 +384,11 @@ test.describe('cody sidebar', () => { await doesNotHaveCody(page) }) - test.fixme('disabled when disabled for user', async ({ page, sg }) => { + test('disabled when disabled for user', async ({ page, sg }) => { // These tests seem to take longer than the default timeout test.setTimeout(10000) - sg.setWindowContext({ + await sg.setWindowContext({ codyEnabledOnInstance: true, codyEnabledForCurrentUser: false, }) @@ -402,7 +401,7 @@ test.describe('cody sidebar', () => { // teardown takes longer than default timeout test.setTimeout(10000) - sg.setWindowContext({ + await sg.setWindowContext({ codyEnabledOnInstance: true, codyEnabledForCurrentUser: true, }) @@ -412,7 +411,7 @@ test.describe('cody sidebar', () => { }) test('disabled for excluded repo', async ({ page, sg }) => { - sg.setWindowContext({ + await sg.setWindowContext({ codyEnabledOnInstance: true, codyEnabledForCurrentUser: true, }) @@ -431,7 +430,7 @@ test.describe('cody sidebar', () => { }) test('disabled with invalid context filter', async ({ page, sg }) => { - sg.setWindowContext({ + await sg.setWindowContext({ codyEnabledOnInstance: true, codyEnabledForCurrentUser: true, }) diff --git a/client/web-sveltekit/src/routes/layout.spec.ts b/client/web-sveltekit/src/routes/layout.spec.ts index 3862fdf8ef2..874e4079007 100644 --- a/client/web-sveltekit/src/routes/layout.spec.ts +++ b/client/web-sveltekit/src/routes/layout.spec.ts @@ -11,7 +11,7 @@ test('has sign in button', async ({ page }) => { }) test('has user menu', async ({ sg, page }) => { - sg.signIn({ username: 'test' }) + await sg.signIn({ username: 'test' }) const userMenu = page.getByLabel('Open user menu') await page.goto('/') @@ -64,7 +64,7 @@ test.describe('cody top level navigation', () => { await sg.setWindowContext(context) if (signedIn) { - sg.signIn({ username: 'test' }) + await sg.signIn({ username: 'test' }) } await page.goto('/') diff --git a/client/web-sveltekit/src/testing/integration.ts b/client/web-sveltekit/src/testing/integration.ts index 932fa7366f4..138bbaae081 100644 --- a/client/web-sveltekit/src/testing/integration.ts +++ b/client/web-sveltekit/src/testing/integration.ts @@ -85,6 +85,9 @@ const typeDefs = glob class Sourcegraph { private debugMode = false + private dotcomModeEnabled = false + private signedIn = false + constructor(private readonly page: Page, private readonly graphqlMock: GraphQLMockServer) {} async setup(): Promise { @@ -117,10 +120,12 @@ class Sourcegraph { }, }) }) - await this.page.addInitScript(() => - window.localStorage.setItem('temporarySettings', '{"webNext.welcomeOverlay.dismissed": true}') - ) } + + await this.page.addInitScript(() => { + window.localStorage.setItem('temporarySettings', '{"webNext.welcomeOverlay.dismissed": true}') + }) + // mock graphql calls await this.page.route(/\.api\/graphql/, route => { const { query, variables, operationName } = JSON.parse(route.request().postData() ?? '') @@ -244,7 +249,8 @@ class Sourcegraph { }, context) } - public signIn(userMock: UserMock = {}): void { + public async signIn(userMock: UserMock = {}): Promise { + this.signedIn = true this.mockTypes({ Query: () => ({ currentUser: { @@ -253,21 +259,40 @@ class Sourcegraph { }, }), }) + + if (this.dotcomModeEnabled) { + await this.setWindowContext({ + codyEnabledForCurrentUser: true, + }) + } } - public signOut(): void { + public async signOut(): Promise { + this.signedIn = false this.mockTypes({ Query: () => ({ currentUser: null, }), }) + + if (this.dotcomModeEnabled) { + await this.setWindowContext({ + codyEnabledForCurrentUser: false, + }) + } } /** * Mock the current window context to be in "dotcom mode" (sourcegraph.com). */ - public dotcomMode(): void { - this.setWindowContext({ sourcegraphDotComMode: true }) + public async dotcomMode(): Promise { + this.dotcomModeEnabled = true + await this.setWindowContext({ + sourcegraphDotComMode: true, + // These are enabled by default on sourcegraph.com + codyEnabledOnInstance: true, + codyEnabledForCurrentUser: this.signedIn, + }) } public teardown(): void { diff --git a/client/web-sveltekit/vite.config.ts b/client/web-sveltekit/vite.config.ts index 6d7a4729650..9e33a2d325d 100644 --- a/client/web-sveltekit/vite.config.ts +++ b/client/web-sveltekit/vite.config.ts @@ -10,8 +10,10 @@ import inspect from 'vite-plugin-inspect' import type { UserConfig as VitestUserConfig } from 'vitest' import graphqlCodegen from './dev/vite-graphql-codegen' +import { sgProxy } from './dev/vite-sg-proxy' const BAZEL = !!process.env.BAZEL +const PLAYWRIGHT = !!process.env.PLAYWRIGHT export default defineConfig(({ mode }) => { // Using & VitestUserConfig shouldn't be necessary but without it `svelte-check` complains when run @@ -41,6 +43,13 @@ export default defineConfig(({ mode }) => { // Generates typescript types for gql-tags and .gql files graphqlCodegen(), inspect(), + // This plugin proxies requests to resources that are not handled by the SvelteKit app + // to a real Sourcegraph instance. + // It also extracts the JS context object from the origin server and injects it into the local HTML page. + !PLAYWRIGHT && + sgProxy({ + target: process.env.SOURCEGRAPH_API_URL || 'https://sourcegraph.sourcegraph.com', + }), ], build: { sourcemap: true, @@ -78,21 +87,6 @@ export default defineConfig(({ mode }) => { // our existing caddy setup (which proxies requests to a specific port). port: process.env.SK_PORT ? +process.env.SK_PORT : undefined, strictPort: !!process.env.SK_PORT, - proxy: { - // Proxy requests to specific endpoints to a real Sourcegraph - // instance. - '^(/sign-(in|out)|/.assets|/-|/.api|/.auth|/search/stream|/users|/notebooks|/insights|/batch-changes|/contexts)|/-/(raw|own|code-graph|batch-changes|settings)(/|$)': - { - target: process.env.SOURCEGRAPH_API_URL || 'https://sourcegraph.sourcegraph.com', - changeOrigin: true, - secure: false, - headers: { - // This needs to be set to make the cody sidebar work, which doesn't use the web graphql client work. - // todo(fkling): Figure out how the React app makes this work without this header. - 'X-Requested-With': 'Sourcegraph', - }, - }, - }, }, resolve: { diff --git a/cmd/frontend/internal/app/router/BUILD.bazel b/cmd/frontend/internal/app/router/BUILD.bazel index bd0d856d66d..83b608ffdc0 100644 --- a/cmd/frontend/internal/app/router/BUILD.bazel +++ b/cmd/frontend/internal/app/router/BUILD.bazel @@ -12,6 +12,7 @@ go_library( tags = [TAG_SEARCHSUITE], visibility = ["//cmd/frontend:__subpackages__"], deps = [ + "//cmd/frontend/internal/app/ui/sveltekit", "//cmd/frontend/internal/routevar", "//internal/api", "@com_github_gorilla_mux//:mux", diff --git a/cmd/frontend/internal/app/router/router.go b/cmd/frontend/internal/app/router/router.go index b541a0ad017..5707ecb50d8 100644 --- a/cmd/frontend/internal/app/router/router.go +++ b/cmd/frontend/internal/app/router/router.go @@ -7,6 +7,7 @@ package router import ( "github.com/gorilla/mux" + "github.com/sourcegraph/sourcegraph/cmd/frontend/internal/app/ui/sveltekit" "github.com/sourcegraph/sourcegraph/cmd/frontend/internal/routevar" ) @@ -98,6 +99,11 @@ func newRouter() *mux.Router { base.Path("/site-admin/pings/latest").Methods("GET").Name(LatestPing) + // Make the abov paths known to the SvelteKit app + // (the repo route is ignored since it's basically catch-all; + // it's also handled by ui/router.go) + sveltekit.RegisterSvelteKit(base, nil) + repoPath := `/` + routevar.Repo repo := base.PathPrefix(repoPath + "/" + routevar.RepoPathDelim + "/").Subrouter() repo.Path("/badge.svg").Methods("GET").Name(RepoBadge) diff --git a/cmd/frontend/internal/app/ui/sveltekit/sveltekit.go b/cmd/frontend/internal/app/ui/sveltekit/sveltekit.go index 67da8504ad1..ba4fa54012d 100644 --- a/cmd/frontend/internal/app/ui/sveltekit/sveltekit.go +++ b/cmd/frontend/internal/app/ui/sveltekit/sveltekit.go @@ -53,7 +53,7 @@ func (r *svelteKitRoute) matches(url *url.URL) bool { } // RegisterSvelteKit registers a middleware that determines which routes are enabled for SvelteKit. -// It also extends the request context with inormation that is sent to the client apps via JSContext. +// It also extends the request context with information that is sent to the client apps via JSContext. func RegisterSvelteKit(r *mux.Router, repoRootRoute *mux.Route) { var knownRoutes []string @@ -107,7 +107,11 @@ func RegisterSvelteKit(r *mux.Router, repoRootRoute *mux.Route) { } value := &contextValue{enabledRoutes: enabledRoutes, knownRoutes: knownRoutes, enabled: enabled} - next.ServeHTTP(w, req.WithContext(context.WithValue(req.Context(), contextKey{}, value))) + existingValue := fromContext(ctx) + if existingValue != nil { + value.knownRoutes = append(existingValue.knownRoutes, knownRoutes...) + } + next.ServeHTTP(w, req.WithContext(context.WithValue(ctx, contextKey{}, value))) }) }) }