mirror of
https://github.com/sourcegraph/sourcegraph.git
synced 2026-02-06 18:31:54 +00:00
e4a8ce52da
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.
|
||
|---|---|---|
| .. | ||
| branded | ||
| browser | ||
| build-config | ||
| client-api | ||
| codeintellify | ||
| cody-context-filters-test-dataset | ||
| cody-shared | ||
| cody-ui | ||
| common | ||
| eslint-plugin-wildcard | ||
| extension-api | ||
| extension-api-types | ||
| http-client | ||
| jetbrains | ||
| observability-client | ||
| observability-server | ||
| shared | ||
| storybook | ||
| template-parser | ||
| testing | ||
| vscode | ||
| web | ||
| web-sveltekit | ||
| wildcard | ||
| BUILD.bazel | ||
| README.md | ||
Frontend packages
List
- web: The web application deployed to http://sourcegraph.com/
- browser: The Sourcegraph browser extension adds tooltips to code on different code hosts.
- vscode: The Sourcegraph VS Code extension.
- extension-api: The Sourcegraph extension API types for the Sourcegraph extensions. Published as
sourcegraph. - extension-api-types: The Sourcegraph extension API types for client applications that embed Sourcegraph extensions and need to communicate with them. Published as
@sourcegraph/extension-api-types. - sandboxes: All demos-mvp (minimum viable product) for the Sourcegraph web application.
- shared: Contains common TypeScript/React/SCSS client code shared between the browser extension and the web app. Everything in this package is code-host agnostic.
- branded: Contains React components and implements the visual design language we use across our web app and e.g. in the options menu of the browser extension. Over time, components from
sharedandbrandedpackages should be moved into thewildcardpackage. - wildcard: Package that encapsulates storybook configuration and contains our Wildcard design system components. If we're using a component in two or more different areas (e.g.
web-appandbrowser-extension) then it should live in thewildcardpackage. Otherwise the components should be better colocated with the code where they're actually used. - search: Search-related code that may be shared between all clients, both branded (e.g. web, VS Code extension) and unbranded (e.g. browser extension)
- storybook: Storybook configuration.
Further migration plan
-
Fix circular dependency in TS project-references graph wildcard package should not rely on web and probably shared, branded too. Ideally it should be an independent self-contained package.
-
Decide on package naming and update existing package names. Especially it should be done for a shared package because we have multiple
sharedfolders inside of other packages. It's hard to understand from where dependency is coming from and it's not possible to refactor import paths using find-and-replace. -
Investigate if we can painlessly switch to
npmworkspaces. -
Content of packages shared and branded should be moved to wildcard and refactored using the latest FE rules and conventions. Having different packages clearly communicates the migration plan. Developers first should look for components in the wildcard package and then fall-back to legacy packages if wildcard doesn't have the solution to their problem yet.
-
shared contains utility functions, types, polyfills, etc which is not a part of the Wildcard component library. These modules should be moved into utils package and other new packages: e.g. api for GraphQL client and type generators, etc.
-
Packages should use package name (e.g.
@sourcegraph/wildcard) for imports instead of the relative paths (e.g.../../../../wildcard/src/components/Markdown) to avoid long relative-paths and make dependency graph between packages clear. (Typescript will warn if packages have circular dependencies). It's easy to refactor such isolated packages, extract functionality into new ones, or even into new repositories.