From 23616fa5c05c51592c1c0953589587452b83baf6 Mon Sep 17 00:00:00 2001 From: Felix Kling Date: Tue, 16 Jul 2024 06:51:36 +0200 Subject: [PATCH] chore(svelte): Refactor infinity query implementation (#63824) In preparation for other work, this commit substantially refactors the "infinity query" store implementation. The internals have been changed completely which allows us to simplify its public API. - Simpler configuration, especially merging previous and next results. - Restoration support. So far pages/components had to implement restoring the state of an infinity store on their own. Now the restoration strategy is part of the configuration. Pages/components only have to get an opaque snapshot via `store.capture()` and restore it via `store.restore(snapshot)`. - More predictable state. It wasn't always obvious if the store content stale data e.g. during restoring data. Now `data` will only be set when the data is 'fresh'. - Smarter 'incremental restoration' strategy. This strategy makes multiple requests to restore the previous state. It makes multiple requests because normally requests are cached and there this is fast. When the data is not cached though there is a noticable delay due to waterfall requests. Now we use a simple heuristic to determine whether or not GraqhQL data might be cached. If not we make a single request to restore the state. For review I suggest to turn whitespace changes off. ## Test plan Manual testing, unit tests. --- client/web-sveltekit/prettier.config.cjs | 5 +- .../src/lib/graphql/urql.test.ts | 343 +++++++------- client/web-sveltekit/src/lib/graphql/urql.ts | 421 ++++++++++++------ .../src/lib/repo/HistoryPanel.stories.svelte | 14 +- .../src/lib/repo/HistoryPanel.svelte | 58 ++- .../src/lib/search/input/SearchInput.svelte | 3 +- .../(validrev)/(code)/+layout.svelte | 32 +- .../(validrev)/(code)/+layout.ts | 125 ++---- .../(validrev)/(code)/ReferencePanel.svelte | 20 +- .../-/commits/[...path]/+page.svelte | 28 +- .../(validrev)/-/commits/[...path]/+page.ts | 62 +-- .../-/branches/all/+page.svelte | 41 +- .../[...repo=reporev]/-/branches/all/+page.ts | 25 +- .../-/commit/[...revspec]/+page.svelte | 20 +- .../-/commit/[...revspec]/+page.ts | 49 +- .../[...repo=reporev]/-/tags/+page.svelte | 34 +- .../routes/[...repo=reporev]/-/tags/+page.ts | 25 +- 17 files changed, 654 insertions(+), 651 deletions(-) diff --git a/client/web-sveltekit/prettier.config.cjs b/client/web-sveltekit/prettier.config.cjs index 018e218ead9..2dd0d464cce 100644 --- a/client/web-sveltekit/prettier.config.cjs +++ b/client/web-sveltekit/prettier.config.cjs @@ -2,5 +2,8 @@ const baseConfig = require('../../prettier.config.js') module.exports = { ...baseConfig, plugins: [...(baseConfig.plugins || []), 'prettier-plugin-svelte'], - overrides: [...(baseConfig.overrides || []), { files: '*.svelte', options: { parser: 'svelte', htmlWhitespaceSensitivity: 'strict' } }], + overrides: [ + ...(baseConfig.overrides || []), + { files: '*.svelte', options: { parser: 'svelte', htmlWhitespaceSensitivity: 'strict' } }, + ], } diff --git a/client/web-sveltekit/src/lib/graphql/urql.test.ts b/client/web-sveltekit/src/lib/graphql/urql.test.ts index fb9010ec4fd..03abc045397 100644 --- a/client/web-sveltekit/src/lib/graphql/urql.test.ts +++ b/client/web-sveltekit/src/lib/graphql/urql.test.ts @@ -1,215 +1,192 @@ import { type AnyVariables, Client, type OperationResult, CombinedError, cacheExchange } from '@urql/core' -import { test, expect, vi, beforeEach } from 'vitest' +import { describe, test, expect, vi, beforeEach, afterEach } from 'vitest' import { pipe, filter, map, merge } from 'wonka' -import { infinityQuery } from './urql' +import { IncrementalRestoreStrategy, infinityQuery } from './urql' -function getMockClient(responses: Partial>[]): Client { - return new Client({ - url: '#testingonly', - exchanges: [ - cacheExchange, // This is required because infiniteQuery expects that a cache exchange is present - ({ forward }) => - operations$ => { - const mockResults$ = pipe( - operations$, - filter(operation => { - switch (operation.kind) { - case 'query': - case 'mutation': - return true - default: - return false - } - }), - map((operation): OperationResult => { - const response = responses.shift() - if (!response) { +describe('infinityQuery', () => { + function getMockClient(responses: Partial>[]): Client { + return new Client({ + url: '#testingonly', + exchanges: [ + cacheExchange, // This is required because infiniteQuery expects that a cache exchange is present + ({ forward }) => + operations$ => { + const mockResults$ = pipe( + operations$, + filter(operation => { + switch (operation.kind) { + case 'query': + case 'mutation': + return true + default: + return false + } + }), + map((operation): OperationResult => { + const response = responses.shift() + if (!response) { + return { + operation, + error: new CombinedError({ + networkError: new Error('No more responses'), + }), + stale: false, + hasNext: false, + } + } return { + ...response, operation, - error: new CombinedError({ - networkError: new Error('No more responses'), - }), + data: response.data ?? undefined, + error: response.error ?? undefined, stale: false, hasNext: false, } - } - return { - ...response, - operation, - data: response.data ?? undefined, - error: response.error ?? undefined, - stale: false, - hasNext: false, - } - }) - ) + }) + ) - const forward$ = pipe( - operations$, - filter(operation => { - switch (operation.kind) { - case 'query': - case 'mutation': - return false - default: - return true - } - }), - forward - ) + const forward$ = pipe( + operations$, + filter(operation => { + switch (operation.kind) { + case 'query': + case 'mutation': + return false + default: + return true + } + }), + forward + ) - return merge([mockResults$, forward$]) - }, - ], - }) -} + return merge([mockResults$, forward$]) + }, + ], + }) + } -function getQuery(client: Client) { - return infinityQuery({ - client, - query: 'query { list { nodes { id } } pageInfo { hasNextPage, endCursor } } }', - variables: { - first: 2, - afterCursor: null as string | null, - }, - nextVariables: previousResult => { - if (previousResult?.data?.list?.pageInfo?.hasNextPage) { + function getQuery(client: Client) { + return infinityQuery({ + client, + query: 'query { list { nodes { id } } pageInfo { hasNextPage, endCursor } } }', + variables: { + first: 2, + afterCursor: null as string | null, + }, + map: result => { + const list = result.data?.list return { - afterCursor: previousResult.data.list.pageInfo.endCursor, + nextVariables: list?.pageInfo.hasNextPage ? { afterCursor: list.pageInfo.endCursor } : undefined, + data: list?.nodes, + error: result.error, } - } - return undefined - }, - combine: (previousResult, nextResult) => { - if (!nextResult.data?.list) { - return nextResult - } - const previousNodes = previousResult.data?.list?.nodes ?? [] - const nextNodes = nextResult.data.list?.nodes ?? [] - return { - ...nextResult, + }, + merge: (prev, next) => [...(prev ?? []), ...(next ?? [])], + createRestoreStrategy: api => + new IncrementalRestoreStrategy( + api, + n => n.length, + n => ({ first: n.length }) + ), + }) + } + + let query: ReturnType + + beforeEach(() => { + vi.useFakeTimers() + + const client = getMockClient([ + { data: { list: { - ...nextResult.data.list, - nodes: [...previousNodes, ...nextNodes], - }, - }, - } - }, - }) -} - -let query: ReturnType - -beforeEach(() => { - vi.useFakeTimers() - - const client = getMockClient([ - { - data: { - list: { - nodes: [{ id: 1 }, { id: 2 }], - pageInfo: { - hasNextPage: true, - endCursor: '2', + nodes: [{ id: 1 }, { id: 2 }], + pageInfo: { + hasNextPage: true, + endCursor: '2', + }, }, }, }, - }, - { - data: { - list: { - nodes: [{ id: 3 }, { id: 4 }], - pageInfo: { - hasNextPage: true, - endCursor: '4', + { + data: { + list: { + nodes: [{ id: 3 }, { id: 4 }], + pageInfo: { + hasNextPage: true, + endCursor: '4', + }, }, }, }, - }, - { - data: { - list: { - nodes: [{ id: 5 }, { id: 6 }], - pageInfo: { - hasNextPage: false, + { + data: { + list: { + nodes: [{ id: 5 }, { id: 6 }], + pageInfo: { + hasNextPage: false, + }, }, }, }, - }, - ]) - query = getQuery(client) -}) - -test('fetch more', async () => { - const subscribe = vi.fn() - query.subscribe(subscribe) - - await vi.runAllTimersAsync() - - // 1. call: fetching -> true - // 2. call: result - expect(subscribe).toHaveBeenCalledTimes(2) - expect(subscribe.mock.calls[0][0]).toMatchObject({ - fetching: true, + ]) + query = getQuery(client) }) - expect(subscribe.mock.calls[1][0]).toMatchObject({ - fetching: false, - data: { - list: { - nodes: [{ id: 1 }, { id: 2 }], - pageInfo: { - hasNextPage: true, - endCursor: '2', - }, + + afterEach(() => { + vi.useRealTimers() + }) + + test('fetch more', async () => { + const subscribe = vi.fn() + query.subscribe(subscribe) + + await vi.runAllTimersAsync() + + // 1. call: fetching -> true + // 2. call: result + expect(subscribe).toHaveBeenCalledTimes(2) + expect(subscribe.mock.calls[0][0]).toMatchObject({ + fetching: true, + }) + expect(subscribe.mock.calls[1][0]).toMatchObject({ + fetching: false, + data: [{ id: 1 }, { id: 2 }], + nextVariables: { + afterCursor: '2', }, - }, - }) + }) - // Fetch more data - query.fetchMore() - await vi.runAllTimersAsync() + // Fetch more data + query.fetchMore() + await vi.runAllTimersAsync() - // 3. call: fetching -> true - // 4. call: result - expect(subscribe).toHaveBeenCalledTimes(4) - expect(subscribe.mock.calls[2][0]).toMatchObject({ - fetching: true, - }) - expect(subscribe.mock.calls[3][0]).toMatchObject({ - fetching: false, - data: { - list: { - nodes: [{ id: 1 }, { id: 2 }, { id: 3 }, { id: 4 }], - pageInfo: { - hasNextPage: true, - endCursor: '4', - }, + // 3. call: fetching -> true + // 4. call: result + expect(subscribe).toHaveBeenCalledTimes(4) + expect(subscribe.mock.calls[2][0]).toMatchObject({ + fetching: true, + }) + expect(subscribe.mock.calls[3][0]).toMatchObject({ + fetching: false, + data: [{ id: 1 }, { id: 2 }, { id: 3 }, { id: 4 }], + nextVariables: { + afterCursor: '4', }, - }, - }) -}) - -test('restoring state', async () => { - const subscribe = vi.fn() - query.subscribe(subscribe) - await vi.runAllTimersAsync() - await query.restore(result => (result.data as any).list.nodes.length < 5) - - expect(subscribe).toHaveBeenCalledTimes(6) - expect(subscribe.mock.calls[4][0]).toMatchObject({ - restoring: true, - }) - expect(subscribe.mock.calls[5][0]).toMatchObject({ - restoring: false, - data: { - list: { - nodes: [{ id: 1 }, { id: 2 }, { id: 3 }, { id: 4 }, { id: 5 }, { id: 6 }], - pageInfo: { - hasNextPage: false, - }, - }, - }, + }) + }) + + test('restoring state', async () => { + const subscribe = vi.fn() + query.subscribe(subscribe) + const snapshot = query.capture() + query.restore({ ...snapshot!, count: 6 }) + await vi.runAllTimersAsync() + + expect(subscribe.mock.calls[3][0]).toMatchObject({ + fetching: false, + data: [{ id: 1 }, { id: 2 }, { id: 3 }, { id: 4 }, { id: 5 }, { id: 6 }], + }) }) }) diff --git a/client/web-sveltekit/src/lib/graphql/urql.ts b/client/web-sveltekit/src/lib/graphql/urql.ts index a4fb7ecdaf1..9bdddf19f53 100644 --- a/client/web-sveltekit/src/lib/graphql/urql.ts +++ b/client/web-sveltekit/src/lib/graphql/urql.ts @@ -2,22 +2,20 @@ import { Client, cacheExchange, fetchExchange, - mapExchange, - type Exchange, makeOperation, + mapExchange, type AnyVariables, - type OperationResult, - createRequest, type DocumentInput, + type Exchange, + type OperationResult, } from '@urql/core' import type { OperationDefinitionNode } from 'graphql' import { once } from 'lodash' -import { from, isObservable, Subject, type Observable, concat, of } from 'rxjs' -import { map, switchMap, scan, startWith } from 'rxjs/operators' -import { type Readable, readable, get } from 'svelte/store' +import { type Readable, get, writable, type Writable } from 'svelte/store' import type { GraphQLResult } from '@sourcegraph/http-client' +import { uniqueID } from '$lib/dom' import { GRAPHQL_URI } from '$lib/http-client' import { getHeaders } from './shared' @@ -65,7 +63,7 @@ export function query(query, variables).toPromise() } -interface InfinityQueryArgs { +interface InfinityQueryArgs { /** * The {@link Client} instance to use for the query. */ @@ -74,76 +72,112 @@ interface InfinityQueryArgs + query: DocumentInput /** * The initial variables to use for the query. */ - variables: TVariables | Observable + variables: TVariables | Promise /** - * A function that returns the next set of variables to use for the query. + * Process the result of the query. This function maps the response to the data used + * and computes the next set of query variables, if any. * + * @param result - The result of the query. * @param previousResult - The previous result of the query. - * - * @remarks - * `nextVariables` is called when {@link InfinityQueryStore.fetchMore} is called to get the next set - * of variables to fetch the next page of data. This function to extract the cursor for the next - * page from the previous result. + * @returns The new/combined result state. */ - nextVariables: (previousResult: OperationResult) => Partial | undefined + map: (result: OperationResult) => InfinityStoreResult /** - * A function to combine the previous result with the next result. - * - * @param previousResult - The previous result of the query. - * @param nextResult - The next result of the query. - * @returns The combined result of the query. - * - * @remarks - * `combine` is called when the next result is received to merge the previous result with the new - * result. This function is used to append the new data to the previous data. + * Optional callback to merge the data from the previous result with the new data. + * If not provided the new data will replace the old data. */ - combine: ( - previousResult: OperationResultState, - nextResult: OperationResultState - ) => OperationResultState + merge?: (previousData: TData | undefined, newData: TData | undefined) => TData + + /** + * Returns a strategy for restoring the data when navigating back to a page. + */ + createRestoreStrategy?: (api: InfinityAPI) => RestoreStrategy } -interface OperationResultState - extends OperationResult { +/** + * Internal API for the infinity query store. Used by restore strategies to control the store. + */ +interface InfinityAPI { + /** + * The internal store representing the current state of the query. + */ + store: Writable> + /** + * Helper function for fetching and processing the next set of data. + */ + fetch( + variables: Partial, + previous: InfinityStoreResult + ): Promise> +} + +/** + * The processed/combined result of a GraphQL query. + */ +export interface InfinityStoreResult { + data?: TData + + /** + * Set if there was an error fetching the data. When set, no more data will be fetched. + */ + error?: Error + + /** + * The set of variables to use for the next fetch. If not set no more data will be fetched. + */ + nextVariables?: Partial +} + +/** + * The state of the infinity query store. + */ +interface InfinityStoreResultState + extends InfinityStoreResult { /** * Whether a GraphQL request is currently in flight. */ fetching: boolean - /** - * Whether the store is currently restoring data. - */ - restoring: boolean } // This needs to be exported so that TS type inference can work in SvelteKit generated files. -export interface InfinityQueryStore - extends Readable> { +export interface InfinityQueryStore + extends Readable> { /** - * Reruns the query with the next set of variables returned by {@link InfinityQueryArgs.nextVariables}. + * Reruns the query with the next set of query variables. * * @remarks - * A new query will only be executed if there is no query currently in flight and {@link InfinityQueryArgs.nextVariables} - * returns a value different from `undefined`. + * A new query will only be executed if there is no query currently in flight and {@link InfinityStoreResult.nextVariables} + * is set. */ fetchMore: () => void /** - * Fetches more data until the given restoreHandler returns `false`. - * - * @param restoreHandler - A function that returns `true` if more data should be fetched. - * - * @remarks - * When navigating back to a page that was previously fetched with `infinityQuery`, the page - * should call `restore` until the previous data state is restored. + * Fetches data while the given predicate is true. Using this function is different f + * rom calling {@link fetchMore} in a loop, because it will set/unset the fetching state + * only once. */ - restore: (restoreHandler: (result: OperationResultState) => boolean) => Promise + fetchWhile: (predicate: (data: TData) => boolean) => Promise + + /** + * Restores the data state from a snapshot, which is returned by {@link capture}. + * + * @param snapshot - The snapshot to restore. + * @returns A promise that resolves when the data has been restored. + */ + restore: (snapshot: TSnapshot | undefined) => Promise + + /** + * Captures the current data state to a snapshot that can be used to restore the data later. + * @returns The snapshot. + */ + capture: () => TSnapshot | undefined } /** @@ -157,125 +191,216 @@ export interface InfinityQueryStore( - args: InfinityQueryArgs -): InfinityQueryStore { - // This is a hacky workaround to create an initialState. The empty object is - // invalid but the request will never be executed with these variables anyway. - const initialVariables = isObservable(args.variables) ? args.variables : of(args.variables) - const operation = args.client.createRequestOperation( - 'query', - isObservable(args.variables) - ? createRequest(args.query, {} as TVariables) - : createRequest(args.query, args.variables) - ) - const initialState: OperationResultState = { - operation, - error: undefined, - data: undefined, - extensions: undefined, - stale: false, - fetching: false, - restoring: false, - hasNext: false, +export function infinityQuery< + TData = any, + TPayload = any, + TVariables extends AnyVariables = AnyVariables, + TSnapshot = void +>(args: InfinityQueryArgs): InfinityQueryStore { + const initialVariables = Promise.resolve(args.variables) + + async function fetch( + variables: Partial, + previousResult: InfinityStoreResult + ): Promise> { + const result = args.map( + await initialVariables.then(initialVariables => + args.client.query(args.query, { ...initialVariables, ...variables }) + ) + ) + if (args.merge) { + result.data = args.merge(previousResult.data, result.data) + } + return result } - const nextVariables = new Subject>() - let shouldRestore: ((result: OperationResultState) => boolean) | null = null + + const initialState: InfinityStoreResultState = { fetching: true } + const store = writable(initialState) + const restoreStrategy = args.createRestoreStrategy?.({ store, fetch }) // Prefetch data. We don't want to wait until the store is subscribed to. That allows us to use this function // inside a data loader and the data will be prefetched before the component is rendered. - initialVariables.subscribe(variables => { - void args.client.query(args.query, variables).toPromise() + fetch({}, {}).then(result => { + store.update(current => { + // Only set the initial state if we haven't already started another fetch process, + // e.g. when restoring the state. + if (current === initialState) { + return { ...result, fetching: false } + } + return current + }) }) - const result = readable(initialState, set => { - const subscription = initialVariables - .pipe( - switchMap(initialVariables => - nextVariables.pipe( - startWith(initialVariables), // nextVaribles will not emit until the first fetchMore is called - switchMap(variables => { - const operation = args.client.createRequestOperation( - 'query', - createRequest(args.query, { ...initialVariables, ...variables }) - ) - return concat( - of({ fetching: true, stale: false, restoring: false }), - from(args.client.executeRequestOperation(operation).toPromise()).pipe( - map(({ data, stale, operation, error, extensions }) => ({ - fetching: false, - data, - stale: !!stale, - operation, - error, - extensions, - })) - ) - ) - }) - ) - ), - scan((result, update) => { - const newResult = { ...result, ...update } - return update.fetching ? newResult : args.combine(result, newResult) - }, initialState) - ) - .subscribe(result => { - if (shouldRestore) { - result.restoring = Boolean( - (result.data || result.error) && shouldRestore(result) && args.nextVariables(result) - ) + /** + * Resolves when the store is not fetching anymore. + */ + function waitTillReady(): Promise { + let unsubscribe: () => void + return new Promise(resolve => { + unsubscribe = store.subscribe(current => { + if (!current.fetching) { + resolve() } - set(result) }) - - return () => subscription.unsubscribe() - }) + }).finally(() => unsubscribe()) + } return { - ...result, + subscribe: store.subscribe, fetchMore: () => { - const current = get(result) - if (current.fetching || current.restoring) { + const previous = get(store) + + if (previous.fetching) { + // When a fetch is already in progress, we don't want to start another one for the same variables. return } - const newVariables = args.nextVariables(current) - if (!newVariables) { - return - } - nextVariables.next(newVariables) - }, - restore: restoreHandler => { - shouldRestore = result => { - return Boolean((result.data || result.error) && restoreHandler(result) && args.nextVariables(result)) - } - return new Promise(resolve => { - const unsubscribe = result.subscribe(result => { - if (result.fetching) { - return - } - if (result.data || result.error) { - const newVariables = args.nextVariables(result) - if (restoreHandler(result) && newVariables) { - shouldRestore = restoreHandler - nextVariables.next(newVariables) - } else { - unsubscribe() - shouldRestore = null - resolve() + + if (previous.nextVariables && !previous.error) { + store.set({ ...previous, fetching: true }) + fetch(previous.nextVariables, previous).then(result => { + store.update(current => { + if (previous.nextVariables === current.nextVariables) { + return { ...result, fetching: false } } - } + return current + }) }) - }) + } }, + fetchWhile: async predicate => { + // We need to wait until the store is not fetching anymore to ensure that we don't start + // another fetch process while one is already in progress. + await waitTillReady() + const current = get(store) + + store.set({ ...current, fetching: true }) + + let result: InfinityStoreResult = current + while (!result.error && result.nextVariables && result.data && predicate(result.data)) { + result = await fetch(result.nextVariables, result) + } + + store.set({ ...result, fetching: false }) + }, + capture: () => restoreStrategy?.capture(get(store)), + restore: snapshot => { + if (restoreStrategy && snapshot) { + return restoreStrategy.restore(snapshot) + } + return Promise.resolve() + }, + } +} + +/** + * A restore strategy captures and restores the data state of a query. + */ +interface RestoreStrategy { + capture(result: InfinityStoreResult): TSnapshot | undefined + restore(snapshot: TSnapshot): Promise +} + +// This needs to be exported so that TS type inference can work in SvelteKit generated files. +export interface IncrementalRestoreStrategySnapshot { + count: number + variables?: Partial + nonce: string +} + +// We use this to indentify snapshots that were created in the current "session", which +// means there is a high chance that the data is still in the cache. +const NONCE = uniqueID('repeat-restore') + +/** + * The incremental restore strategy captures and restores the data by counting the number of items. + * It will fetch more data until the count matches the snapshot count. + * + * This strategy is useful when every fetch returns a fixed number of items (i.e. after a cursor). + * In this case we want to make use of our GraphQL client's caching strategy and simply + * "replay" the previous fetches. + * + * This strategy works well when GraphQL requests are cached. To avoid waterfall requests in case the + * data is not cached, the strategy will fall back to requesting the data once with query variables + * from the snapshot. + */ +export class IncrementalRestoreStrategy + implements RestoreStrategy, TData> +{ + constructor( + private api: InfinityAPI, + /** + * A function to map the data to a number. This number will be used to count the items. + */ + private mapper: (data: TData) => number, + /** + * A function to map the data to query variables. These variables will be used to fetch the data + * once when if there is a chance that the data is not in the cache (fallback). + */ + private variablesMapper?: (data: TData) => Partial + ) {} + + public capture(result: InfinityStoreResult): IncrementalRestoreStrategySnapshot | undefined { + return result.data + ? { + count: this.mapper(result.data), + variables: this.variablesMapper ? this.variablesMapper(result.data) : undefined, + nonce: NONCE, + } + : undefined + } + + public async restore(snapshot: IncrementalRestoreStrategySnapshot): Promise { + this.api.store.set({ fetching: true }) + const result = await (snapshot.nonce !== NONCE && snapshot.variables + ? this.api.fetch(snapshot.variables, {}) + : this.fetch(snapshot)) + this.api.store.set({ ...result, fetching: false }) + } + + private async fetch( + snapshot: IncrementalRestoreStrategySnapshot + ): Promise> { + let current: InfinityStoreResult = { nextVariables: {} } + while (current.nextVariables && ((current.data && this.mapper(current.data)) || 0) < snapshot.count) { + current = await this.api.fetch(current.nextVariables, current) + if (current.error || !current.data) { + break + } + } + return current + } +} + +/** + * A restore strategy that overwrites the current store state with the response of a new query. + * The strategy uses the query variables form the snapshot to fetch the data. + */ +export class OverwriteRestoreStrategy + implements RestoreStrategy<{ variables: Partial }, TData> +{ + constructor( + private api: InfinityAPI, + private variablesMapper: (data: TData) => Partial + ) {} + + capture(result: InfinityStoreResult): { variables: Partial } | undefined { + if (!result.data) { + return undefined + } + const variables = this.variablesMapper(result.data) + return variables ? { variables } : undefined + } + + async restore(snapshot: { variables: Partial }): Promise { + this.api.store.set({ fetching: true }) + const result = await this.api.fetch(snapshot.variables, {}) + this.api.store.set({ ...result, fetching: false }) } } diff --git a/client/web-sveltekit/src/lib/repo/HistoryPanel.stories.svelte b/client/web-sveltekit/src/lib/repo/HistoryPanel.stories.svelte index 67e0326d704..948bbefe2ff 100644 --- a/client/web-sveltekit/src/lib/repo/HistoryPanel.stories.svelte +++ b/client/web-sveltekit/src/lib/repo/HistoryPanel.stories.svelte @@ -2,12 +2,15 @@ import { createHistoryResults } from '$testing/testdata' import { Story } from '@storybook/addon-svelte-csf' import HistoryPanel from './HistoryPanel.svelte' + import { readable } from 'svelte/store' export const meta = { component: HistoryPanel, parameters: { sveltekit_experimental: { stores: { - page: {}, + page: { + url: new URL(window.location.href), + }, }, }, }, @@ -17,12 +20,19 @@

Commits to show:


{#key commitCount} - {}} /> + {/key}
diff --git a/client/web-sveltekit/src/lib/repo/HistoryPanel.svelte b/client/web-sveltekit/src/lib/repo/HistoryPanel.svelte index 6a2f1c82e8f..b34d9cd88dc 100644 --- a/client/web-sveltekit/src/lib/repo/HistoryPanel.svelte +++ b/client/web-sveltekit/src/lib/repo/HistoryPanel.svelte @@ -1,76 +1,64 @@ - - {#if history} + + {#if $history.data} - {#each history.nodes as commit (commit.id)} + {#each $history.data as commit (commit.id)} {@const selected = commit.abbreviatedOID === selectedRev || commit.oid === selectedRev}
@@ -111,12 +99,22 @@ {/each}
{/if} - {#if !history || loading} - + {#if $history.fetching} +
+ +
+ {:else if $history.error} +
+ Unable to load history: {$history.error.message} +
{/if}