svelte: Use parallel data loading (#60303)

Author: fkling

client/shared/src/search/query/completion-utils.ts

@@ -4,8 +4,6 @@
 
 import { escapeRegExp } from 'lodash'
 
-import type { RepositoryMatch } from '../stream'
-
 import { escapeSpaces, FILTERS, type FilterType, isNegatableFilter } from './filters'
 
 export const PREDICATE_REGEX = /^([.A-Za-z]+)\((.*?)\)?$/
@@ -23,7 +21,7 @@ export const regexInsertText = (value: string): string => {
  * repositoryInsertText escapes the provides value so that it can be used as a
  * value for the `repo:` filter.
  */
-export const repositoryInsertText = ({ repository }: RepositoryMatch): string => regexInsertText(repository)
+export const repositoryInsertText = ({ repository }: { repository: string }): string => regexInsertText(repository)
 
 /**
  * Given a list of filter types, this function returns a list of objects which

client/web-sveltekit/src/lib/repo/api/tree.ts

@@ -7,11 +7,11 @@ import type { TreeProvider } from '$lib/TreeView'
 const MAX_FILE_TREE_ENTRIES = 1000
 
 const treeEntriesQuery = gql`
-    query TreeEntries($repoID: ID!, $commitID: String!, $filePath: String!, $first: Int) {
-        node(id: $repoID) {
+    query TreeEntries($repoName: String!, $revision: String!, $filePath: String!, $first: Int) {
+        repository(name: $repoName) {
             id
             ... on Repository {
-                commit(rev: $commitID) {
+                commit(rev: $revision) {
                     ...GitCommitFieldsWithTree
                 }
             }
@@ -48,10 +48,10 @@ export async function fetchTreeEntries(args: TreeEntriesVariables): Promise<GitC
         }
         // mightContainPrivateInfo: true,
     )
-    if (data.node?.__typename !== 'Repository' || !data.node.commit) {
+    if (!data.repository?.commit) {
         throw new Error('Unable to fetch repository information')
     }
-    return data.node.commit
+    return data.repository.commit
 }
 
 export const NODE_LIMIT: unique symbol = Symbol()
@@ -62,17 +62,17 @@ type ExpandableFileTreeNodeValues = TreeEntryFields
 export type FileTreeNodeValue = ExpandableFileTreeNodeValues | typeof NODE_LIMIT
 
 export async function fetchSidebarFileTree({
-    repoID,
-    commitID,
+    repoName,
+    revision,
     filePath,
 }: {
-    repoID: Scalars['ID']['input']
-    commitID: string
+    repoName: Scalars['ID']['input']
+    revision: string
     filePath: string
 }): Promise<{ root: TreeRoot; values: FileTreeNodeValue[] }> {
     const result = await fetchTreeEntries({
-        repoID,
-        commitID,
+        repoName,
+        revision,
         filePath,
         first: MAX_FILE_TREE_ENTRIES,
     })
@@ -88,17 +88,17 @@ export async function fetchSidebarFileTree({
 }
 
 export type FileTreeLoader = (args: {
-    repoID: Scalars['ID']['input']
-    commitID: string
+    repoName: string
+    revision: string
     filePath: string
     parent?: FileTreeProvider
 }) => Promise<FileTreeProvider>
 
 interface FileTreeProviderArgs {
     root: NonNullable<GitCommitFieldsWithTree['tree']>
     values: FileTreeNodeValue[]
-    repoID: Scalars['ID']['input']
-    commitID: string
+    repoName: string
+    revision: string
     loader: FileTreeLoader
     parent?: TreeProvider<FileTreeNodeValue>
 }
@@ -110,8 +110,8 @@ export class FileTreeProvider implements TreeProvider<FileTreeNodeValue> {
         return this.args.root
     }
 
-    public getRepoID(): Scalars['ID']['input'] {
-        return this.args.repoID
+    public getRepoName(): string {
+        return this.args.repoName
     }
 
     public getEntries(): FileTreeNodeValue[] {
@@ -130,8 +130,8 @@ export class FileTreeProvider implements TreeProvider<FileTreeNodeValue> {
         }
 
         return this.args.loader({
-            repoID: this.args.repoID,
-            commitID: this.args.commitID,
+            repoName: this.args.repoName,
+            revision: this.args.revision,
             filePath: entry.path,
             parent: this,
         })
@@ -140,8 +140,8 @@ export class FileTreeProvider implements TreeProvider<FileTreeNodeValue> {
     public async fetchParent(): Promise<FileTreeProvider> {
         const parentPath = dirname(this.args.root.path)
         return this.args.loader({
-            repoID: this.args.repoID,
-            commitID: this.args.commitID,
+            repoName: this.args.repoName,
+            revision: this.args.revision,
             filePath: parentPath,
         })
     }

client/web-sveltekit/src/lib/repo/utils.ts

@@ -43,3 +43,24 @@ export function getFileURL(repoURL: string, file: { canonicalURL: string }): str
     // TODO: Find out whether there is a safer way to do this
     return repoURL + file.canonicalURL.slice(file.canonicalURL.indexOf('/-/'))
 }
+
+/**
+ * This function is supposed to be used in repository data loaders.
+ *
+ * In order to ensure data consistency when navigating between repository pages, we have
+ * to ensure that the pages fetch data for the same revision. If a revision specifier is
+ * present in the URL and is a commit ID, we can use it directly. If it's a branch name,
+ * tag name or is missing, we have to wait for the parent loader to resolve the revision
+ * to a commit ID.
+ */
+export async function resolveRevision(
+    parent: () => Promise<{ resolvedRevision: ResolvedRevision }>,
+    revisionFromURL: string | undefined
+): Promise<string> {
+    // There is a chance that a commit ID is used as a branch or tag name,
+    // but it's unlikely. Avoiding waterfall requests is worth the risk.
+    if (revisionFromURL && /[0-9a-f]{40}/.test(revisionFromURL)) {
+        return revisionFromURL
+    }
+    return (await parent()).resolvedRevision.commitID
+}

client/web-sveltekit/src/lib/search/utils.ts

@@ -20,6 +20,7 @@ type FilterGroups = Record<Filter['kind'], Filter[]>
 
 export function groupFilters(filters: Filter[] | null | undefined): FilterGroups {
     const groupedFilters: FilterGroups = {
+        type: [],
         file: [],
         repo: [],
         lang: [],

client/web-sveltekit/src/routes/[...repo=reporev]/(validrev)/(code)/+layout.svelte

@@ -13,7 +13,6 @@
 
     import type { LayoutData, Snapshot } from './$types'
     import FileTree from './FileTree.svelte'
-    import type { Scalars } from '$lib/graphql-operations'
     import type { GitHistory_HistoryConnection } from './layout.gql'
     import Tabs from '$lib/Tabs.svelte'
     import TabPanel from '$lib/TabPanel.svelte'
@@ -61,7 +60,7 @@
                 })
         )
 
-    async function updateFileTreeProvider(repoID: Scalars['ID']['input'], commitID: string, parentPath: string) {
+    async function updateFileTreeProvider(repoName: string, revision: string, parentPath: string) {
         const result = await data.fileTree
         if (!result) {
             treeProvider = null
@@ -70,18 +69,14 @@
         const { root, values } = result
 
         // Do nothing if update was called with new arguments in the meantime
-        if (
-            repoID !== data.resolvedRevision.repo.id ||
-            commitID !== data.resolvedRevision.commitID ||
-            parentPath !== data.parentPath
-        ) {
+        if (repoName !== data.repoName || revision !== (data.revision ?? '') || parentPath !== data.parentPath) {
             return
         }
         treeProvider = new FileTreeProvider({
             root,
             values,
-            repoID,
-            commitID,
+            repoName,
+            revision,
             loader: fileTreeLoader,
         })
     }
@@ -111,12 +106,10 @@
     let historyPanel: HistoryPanel
     let rootElement: HTMLElement | null = null
 
-    $: ({ revision, parentPath, resolvedRevision } = data)
-    $: commitID = resolvedRevision.commitID
-    $: repoID = resolvedRevision.repo.id
+    $: ({ revision = '', parentPath, repoName } = data)
     // Only update the file tree provider (which causes the tree to rerender) when repo, revision/commit or file path
     // update
-    $: updateFileTreeProvider(repoID, commitID, parentPath)
+    $: updateFileTreeProvider(repoName, revision, parentPath)
     $: commitHistoryQuery = data.commitHistory
     let commitHistory: GitHistory_HistoryConnection | null
     $: if (commitHistoryQuery) {
@@ -125,10 +118,7 @@
         // file/folder until the new commit history is loaded.
         commitHistory = null
     }
-    $: commitHistory =
-        $commitHistoryQuery?.data.node?.__typename === 'Repository'
-            ? $commitHistoryQuery.data.node.commit?.ancestors ?? null
-            : null
+    $: commitHistory = $commitHistoryQuery?.data?.repository?.commit?.ancestors ?? null
 
     const sidebarSize = getSeparatorPosition('repo-sidebar', 0.2)
     $: sidebarWidth = `max(200px, ${$sidebarSize * 100}%)`

client/web-sveltekit/src/routes/[...repo=reporev]/(validrev)/(code)/+layout.ts

@@ -1,7 +1,9 @@
 import { dirname } from 'path'
 
 import { browser } from '$app/environment'
+import { getGraphQLClient } from '$lib/graphql'
 import { fetchSidebarFileTree } from '$lib/repo/api/tree'
+import { parseRepoRevision } from '$lib/shared'
 
 import type { LayoutLoad } from './$types'
 import { GitHistoryQuery } from './layout.gql'
@@ -27,22 +29,23 @@ if (browser) {
 }
 
 export const load: LayoutLoad = async ({ parent, params }) => {
-    const { resolvedRevision, repoName, graphqlClient } = await parent()
+    const client = await getGraphQLClient()
+    const { repoName, revision = '' } = parseRepoRevision(params.repo)
     const parentPath = getRootPath(repoName, params.path ? dirname(params.path) : REPO_ROOT)
 
     // Fetches the most recent commits for current blob, tree or repo root
-    const commitHistory = graphqlClient.watchQuery({
+    const commitHistory = client.watchQuery({
         query: GitHistoryQuery,
         variables: {
-            repo: resolvedRevision.repo.id,
-            revspec: resolvedRevision.commitID,
+            repoName,
+            revspec: revision,
             filePath: params.path ?? '',
             first: HISTORY_COMMITS_PER_PAGE,
             afterCursor: null,
         },
         notifyOnNetworkStatusChange: true,
     })
-    if (!graphqlClient.readQuery({ query: GitHistoryQuery, variables: commitHistory.variables })) {
+    if (!client.readQuery({ query: GitHistoryQuery, variables: commitHistory.variables })) {
         // Eagerly fetch data if it isn't in the cache already. This ensures that the data is fetched
         // as soon as possible, not only after the layout subscribes to the query.
         commitHistory.refetch()
@@ -52,8 +55,8 @@ export const load: LayoutLoad = async ({ parent, params }) => {
         parentPath,
         commitHistory,
         fileTree: fetchSidebarFileTree({
-            repoID: resolvedRevision.repo.id,
-            commitID: resolvedRevision.commitID,
+            repoName,
+            revision,
             filePath: parentPath,
         }),
     }

client/web-sveltekit/src/routes/[...repo=reporev]/(validrev)/(code)/+page.ts

@@ -23,6 +23,10 @@ export const load: PageLoad = async ({ parent }) => {
                 })
                 .then(result => {
                     if (result.data.node?.__typename !== 'Repository') {
+                        // This page will never render when the repository is not found.
+                        // The (validrev) data loader will render an error page instead.
+                        // Still, this error will show up as an unhandled promise rejection
+                        // in the console. We should find a better way to handle this.
                         throw new Error('Expected Repository')
                     }
                     return result.data.node.commit?.blob ?? null

client/web-sveltekit/src/routes/[...repo=reporev]/(validrev)/(code)/-/blob/[...path]/+page.svelte

@@ -24,22 +24,22 @@
 
     export let data: PageData
 
-    const {
+    // We use the latest value here because we want to keep showing the old document while loading
+    // the new one.
+    const { loading, combinedBlobData, set: setBlobData } = createBlobDataHandler()
+    let selectedPosition: LineOrPositionOrRange | null = null
+
+    $: ({
         revision,
         resolvedRevision: { commitID },
         repoName,
         filePath,
         settings,
         graphqlClient,
-    } = data
-    // We use the latest value here because we want to keep showing the old document while loading
-    // the new one.
-    const { loading, combinedBlobData, set: setBlobData } = createBlobDataHandler()
-    let selectedPosition: LineOrPositionOrRange | null = null
-
+    } = data)
     $: setBlobData(data.blob, data.highlights)
-    $: blobData = $combinedBlobData.blob
-    $: formatted = !!blobData?.richHTML
+    $: ({ blob, highlights = '' } = $combinedBlobData)
+    $: formatted = !!blob?.richHTML
     $: showRaw = $page.url.searchParams.get('view') === 'raw'
     $: codeIntelAPI = createCodeIntelAPI({
         settings: setting => (isErrorLike(settings.final) ? undefined : settings.final?.[setting]),
@@ -53,7 +53,7 @@
 </script>
 
 <svelte:head>
-    <title>{data.filePath} - {data.displayRepoName} - Sourcegraph</title>
+    <title>{filePath} - {data.displayRepoName} - Sourcegraph</title>
 </svelte:head>
 
 <FileHeader>
@@ -68,7 +68,7 @@
             {#if formatted}
                 <FormatAction />
             {/if}
-            <Permalink commitID={data.resolvedRevision.commitID} />
+            <Permalink {commitID} />
         {/if}
     </svelte:fragment>
 </FileHeader>
@@ -84,21 +84,21 @@
                 Unable to load iff
             {/if}
         {/await}
-    {:else if blobData}
-        {#if blobData.richHTML && !showRaw}
+    {:else if blob}
+        {#if blob.richHTML && !showRaw}
             <div class="rich">
-                {@html blobData.richHTML}
+                {@html blob.richHTML}
             </div>
         {:else}
             <CodeMirrorBlob
                 blobInfo={{
-                    ...blobData,
+                    ...blob,
                     revision: revision ?? '',
                     commitID,
                     repoName: repoName,
                     filePath,
                 }}
-                highlights={$combinedBlobData.highlights || ''}
+                {highlights}
                 wrapLines={$lineWrap}
                 selectedLines={selectedPosition?.line ? selectedPosition : null}
                 on:selectline={event => {