Skip to content

Commit

Permalink
Add FilesSample type with filesSample and totalFileCount, use when fe…
Browse files Browse the repository at this point in the history
…tching files

Summary:
Use a dedicated type with a subset of all files, combined with the count of all files. This way, the UI can know the total number of files but only render or use a subset, and also know when it's using a subset or if it knows all the files.

This matches what already exists in `CommitInfo`, but we can use this when fetching just files without the rest of the CommitInfo.

Reviewed By: quark-zju

Differential Revision: D57458042

fbshipit-source-id: 8bde18f0ce6de7d550a901d59d831b2ed678a791
  • Loading branch information
evangrayk authored and facebook-github-bot committed May 16, 2024
1 parent a76bfd2 commit b87081d
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 14 deletions.
5 changes: 3 additions & 2 deletions addons/isl-server/src/ServerToClientAPI.ts
Original file line number Diff line number Diff line change
Expand Up @@ -627,8 +627,9 @@ export default class ServerToClientAPI {
this.postMessage({
type: 'fetchedCommitChangedFiles',
hash: data.hash,
result: {value: files.slice(0, data.limit)},
totalFileCount: files.length,
result: {
value: {filesSample: files.slice(0, data.limit), totalFileCount: files.length},
},
});
})
.catch(err => {
Expand Down
15 changes: 6 additions & 9 deletions addons/isl/src/ChangedFilesWithFetching.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* LICENSE file in the root directory of this source tree.
*/

import type {Hash, Result, ChangedFile, CommitInfo} from './types';
import type {Hash, Result, CommitInfo, FilesSample} from './types';

import serverAPI from './ClientToServerAPI';
import {ChangedFiles} from './UncommittedChanges';
Expand All @@ -14,7 +14,7 @@ import {ComparisonType} from 'shared/Comparison';
import {LRU} from 'shared/LRU';

// Cache fetches in progress so we don't double fetch
const commitFilesCache = new LRU<Hash, Promise<Result<Array<ChangedFile>>>>(10);
const commitFilesCache = new LRU<Hash, Promise<Result<FilesSample>>>(10);

/**
* The basic CommitInfo we fetch in bulk only contains the first 25 files.
Expand All @@ -23,7 +23,7 @@ const commitFilesCache = new LRU<Hash, Promise<Result<Array<ChangedFile>>>>(10);
* to augment the subset we already have.
*/
export function ChangedFilesWithFetching({commit}: {commit: CommitInfo}) {
const [fetchedAllFiles, setFetchedAllFiles] = useState<Array<ChangedFile> | undefined>(undefined);
const [fetchedAllFiles, setFetchedAllFiles] = useState<FilesSample | undefined>(undefined);

const hasAllFilesAlready = commit.filesSample.length === commit.totalFileCount;
useEffect(() => {
Expand All @@ -40,8 +40,8 @@ export function ChangedFilesWithFetching({commit}: {commit: CommitInfo}) {

return (
<ChangedFiles
filesSubset={fetchedAllFiles ?? commit.filesSample}
totalFiles={commit.totalFileCount}
filesSubset={fetchedAllFiles?.filesSample ?? commit.filesSample}
totalFiles={fetchedAllFiles?.totalFileCount ?? commit.totalFileCount}
comparison={
commit.isDot
? {type: ComparisonType.HeadChanges}
Expand All @@ -58,10 +58,7 @@ export function ChangedFilesWithFetching({commit}: {commit: CommitInfo}) {
* Get changed files in a given commit.
* A small subset of the files may have already been fetched,
* or in some cases no files may be cached yet and all files need to be fetched asynchronously. */
export function getChangedFilesForHash(
hash: Hash,
limit = 1000,
): Promise<Result<Array<ChangedFile>>> {
export function getChangedFilesForHash(hash: Hash, limit = 1000): Promise<Result<FilesSample>> {
const foundPromise = commitFilesCache.get(hash);
if (foundPromise != null) {
return foundPromise;
Expand Down
2 changes: 1 addition & 1 deletion addons/isl/src/__tests__/ChangedFilesWithFetching.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ describe('ChangedFilesWithFetching', () => {
simulateMessageFromServer({
type: 'fetchedCommitChangedFiles',
hash: 'b',
result: {value: makeFiles(510)},
result: {value: {filesSample: makeFiles(510), totalFileCount: 510}},
});
});

Expand Down
7 changes: 5 additions & 2 deletions addons/isl/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,10 @@ export type ChangedFile = {
* */
copy?: RepoRelativePath;
};
export type FilesSample = {
filesSample: Array<ChangedFile>;
totalFileCount: number;
};

export type SucceedableRevset = {type: 'succeedable-revset'; revset: Revset};
export type ExactRevset = {type: 'exact-revset'; revset: Revset};
Expand Down Expand Up @@ -725,8 +729,7 @@ export type ServerToClientMessage =
| {
type: 'fetchedCommitChangedFiles';
hash: Hash;
result: Result<Array<ChangedFile>>;
totalFileCount?: number;
result: Result<FilesSample>;
}
| {type: 'typeaheadResult'; id: string; result: Array<TypeaheadResult>}
| {type: 'applicationInfo'; info: ApplicationInfo}
Expand Down

0 comments on commit b87081d

Please sign in to comment.