diff --git a/src/commands/sync.tsx b/src/commands/sync.tsx index 4d4541a..275c8ff 100644 --- a/src/commands/sync.tsx +++ b/src/commands/sync.tsx @@ -1,5 +1,5 @@ import ErrorDisplay from '../components/error-display.js'; -import React, { useCallback, useState } from 'react'; +import React, { useCallback, useEffect, useMemo, useState } from 'react'; import { Action, useAction } from '../hooks/use-action.js'; import { CommandConfig, CommandProps } from '../types.js'; import { ConfirmStatement } from '../components/confirm-statement.js'; @@ -93,11 +93,18 @@ const useSyncAction = ({ rootBranchName: string; }): UseSyncActionResult => { const git = useGit(); - const { currentTree, removeBranch } = useTree(); + const { removeBranch, get } = useTree(); const [allContestedBranches, setAllContestedBranches] = useState( [] ); + /** + * We need a snapshot of the tree instead of "currentTree", because deleting branches while doing cleanup will + * cause the "currentTree" to change, which problematically tries to re-trigger the whole sync process in the + * middle, which causes errors. + */ + const currentTreeSnapshot = useMemo(() => get(), []); + const skipContestedBranch = useCallback((branch: string) => { setAllContestedBranches((prev) => prev.filter((b) => b !== branch)); }, []); @@ -114,18 +121,26 @@ const useSyncAction = ({ const performAction = useCallback(async () => { // todo: unsure if this is the correct condition - if (!currentTree.length) return; + if (!currentTreeSnapshot.length) return; await git.checkout(rootBranchName); await git.pull(); + const contestedBranches = []; - for (const node of currentTree) { + for (const node of currentTreeSnapshot) { const closedOnRemote = await git.isClosedOnRemote(node.key); if (closedOnRemote) { - setAllContestedBranches((prev) => [...prev, node.key]); + contestedBranches.push(node.key); } } - }, [git, currentTree]); + + /** + * We need to update the state of contested branches all at once to prevent the user attempting to run the + * branch deletion function while a git.isClosedOnRemote() is running. Git does not allow multiple commands + * to be run in parallel and enforces this with an internal lockfile. + */ + setAllContestedBranches(contestedBranches); + }, [git, currentTreeSnapshot]); const action = useAction({ asyncAction: performAction, diff --git a/src/contexts/tree-display.context.tsx b/src/contexts/tree-display.context.tsx index c516eba..83d0ee9 100644 --- a/src/contexts/tree-display.context.tsx +++ b/src/contexts/tree-display.context.tsx @@ -1,18 +1,12 @@ -import React, { - ReactNode, - createContext, - useCallback, - useContext, - useMemo, -} from 'react'; +import React, { ReactNode, createContext, useContext, useMemo } from 'react'; import { DisplayNode, getDisplayNodes, maxWidthFromDisplayNodes, } from '../utils/tree-display.js'; import { treeToParentChildRecord } from '../utils/tree-helpers.js'; -import { useAsyncValue } from '../hooks/use-async-value.js'; -import { useGit } from '../hooks/use-git.js'; +import { useBranchNeedsRebaseRecord } from '../hooks/computed-values/use-branch-needs-rebase-record.js'; +import { useCleanCurrentTree } from '../hooks/computed-values/use-clean-current-tree.js'; import { useTree } from '../hooks/use-tree.js'; interface TreeDisplayContextType { @@ -30,43 +24,23 @@ const TreeDisplayContext = createContext({ }); export const TreeDisplayProvider = ({ children }: { children: ReactNode }) => { - const git = useGit(); - const { rootBranchName, currentTree } = useTree(); + const { rootBranchName } = useTree(); + + const { value: currentTree, isLoading: isLoadingCurrentTree } = + useCleanCurrentTree(); + const treeParentChildRecord = useMemo( () => treeToParentChildRecord(currentTree), [currentTree] ); - - const getBranchNeedsRebaseRecord = useCallback(async () => { - const record: Record = {}; - await Promise.all( - currentTree.map(async (_node) => { - if (!_node.parent) return null; - - record[_node.key] = await git.needsRebaseOnto({ - branch: _node.key, - ontoBranch: _node.parent, - }); - return null; - }) - ); - return record; - }, [currentTree, git.needsRebaseOnto]); - - const branchNeedsRebaseRecordResult = useAsyncValue({ - getValue: getBranchNeedsRebaseRecord, - }); - - const branchNeedsRebaseRecord = useMemo(() => { - if (branchNeedsRebaseRecordResult.isLoading) - return {} as Record; - - return branchNeedsRebaseRecordResult.value; - }, [branchNeedsRebaseRecordResult]); + const { + value: branchNeedsRebaseRecord, + isLoading: isLoadingBranchNeedsRebaseRecord, + } = useBranchNeedsRebaseRecord({ currentTree }); const isLoading = useMemo(() => { - return branchNeedsRebaseRecordResult.isLoading; - }, [branchNeedsRebaseRecordResult]); + return isLoadingBranchNeedsRebaseRecord || isLoadingCurrentTree; + }, [isLoadingBranchNeedsRebaseRecord, isLoadingCurrentTree]); const nodes: DisplayNode[] = rootBranchName ? getDisplayNodes({ diff --git a/src/hooks/computed-values/use-branch-needs-rebase-record.ts b/src/hooks/computed-values/use-branch-needs-rebase-record.ts new file mode 100644 index 0000000..48b2d4e --- /dev/null +++ b/src/hooks/computed-values/use-branch-needs-rebase-record.ts @@ -0,0 +1,34 @@ +import { Tree } from '../../services/tree.js'; +import { useAsyncValueWithDefault } from '../use-async-value.js'; +import { useCallback, useMemo } from 'react'; +import { useGit } from '../use-git.js'; + +export const useBranchNeedsRebaseRecord = ({ + currentTree, +}: { + currentTree: Tree; +}) => { + const git = useGit(); + + const getBranchNeedsRebaseRecord = useCallback(async () => { + const record: Record = {}; + + for (const _node of currentTree) { + if (!_node.parent) continue; + + record[_node.key] = await git.needsRebaseOnto({ + branch: _node.key, + ontoBranch: _node.parent, + }); + } + return record; + }, [currentTree, git.needsRebaseOnto]); + + // We need to memoize the default value to avoid infinite renders + const defaultValue = useMemo(() => ({}), []); + + return useAsyncValueWithDefault({ + getValue: getBranchNeedsRebaseRecord, + defaultValue, + }); +}; diff --git a/src/hooks/computed-values/use-clean-current-tree.ts b/src/hooks/computed-values/use-clean-current-tree.ts new file mode 100644 index 0000000..d909bbc --- /dev/null +++ b/src/hooks/computed-values/use-clean-current-tree.ts @@ -0,0 +1,34 @@ +import { Tree } from '../../services/tree.js'; +import { useAsyncValueWithDefault } from '../use-async-value.js'; +import { useCallback, useMemo } from 'react'; +import { useGit } from '../use-git.js'; +import { useTree } from '../use-tree.js'; + +/** + * The "cleaned" tree is the current tree without the branches that don't exist locally. + */ +export const useCleanCurrentTree = () => { + const git = useGit(); + const { currentTree: uncleanCurrentTree, removeBranch } = useTree(); + + const getCleanCurrentTree = useCallback(async () => { + const cleanTree: Tree = []; + for (const _node of uncleanCurrentTree) { + const branchExistsLocally = await git.branchExistsLocally( + _node.key + ); + + if (branchExistsLocally) cleanTree.push(_node); + else removeBranch(_node.key, { ignoreBranchDoesNotExist: true }); + } + return cleanTree; + }, [uncleanCurrentTree, git.branchExistsLocally, removeBranch]); + + // We need to memoize the default value to avoid infinite renders + const defaultValue = useMemo(() => [], []); + + return useAsyncValueWithDefault({ + getValue: getCleanCurrentTree, + defaultValue, + }); +}; diff --git a/src/hooks/use-async-value.ts b/src/hooks/use-async-value.ts index 8a269e9..5739ac7 100644 --- a/src/hooks/use-async-value.ts +++ b/src/hooks/use-async-value.ts @@ -1,5 +1,5 @@ -import { AsyncResult } from '../types.js'; -import { useEffect, useState } from 'react'; +import { AsyncResult, AsyncResultWithDefault } from '../types.js'; +import { useEffect, useMemo, useState } from 'react'; type State = { type: 'LOADING' } | { type: 'COMPLETE'; value: T }; @@ -26,3 +26,21 @@ export const useAsyncValue = ({ isLoading: false, }; }; + +export const useAsyncValueWithDefault = ({ + getValue, + defaultValue, +}: { + getValue: () => Promise; + defaultValue: T; +}): AsyncResultWithDefault => { + const result = useAsyncValue({ getValue }); + + return useMemo(() => { + if (result.isLoading) { + return { value: defaultValue, isLoading: true }; + } + + return { value: result.value, isLoading: false }; + }, [result.isLoading, result.value, defaultValue]); +}; diff --git a/src/services/git.ts b/src/services/git.ts index e91f396..b577a75 100644 --- a/src/services/git.ts +++ b/src/services/git.ts @@ -12,7 +12,10 @@ export interface GitService { branchLocal: () => Promise>; currentBranch: () => Promise; listBranches: () => Promise; - checkout: (branch: string) => Promise>; + checkout: ( + branch: string, + options?: { fallbackBranch?: string } + ) => Promise>; addAllFiles: () => Promise; commit: (args: { message: string }) => Promise; createBranch: (args: { branchName: string }) => Promise; @@ -32,6 +35,7 @@ export interface GitService { fetchPrune: () => Promise; pull: () => Promise; branchDelete: (branch: string) => Promise; + branchExistsLocally: (branch: string) => Promise; } export const createGitService = ({ @@ -40,6 +44,16 @@ export const createGitService = ({ options: Partial; }): GitService => { const gitEngine = simpleGit(options); + + async function branchExistsLocally(branch: string) { + // todo: we should probably be using this function a lot more lmao + const branchRef = await gitEngine.raw([ + 'show-ref', + `refs/heads/${branch}`, + ]); + return Boolean(branchRef); + } + return { _git: gitEngine, // @ts-expect-error - being weird about the return type @@ -55,7 +69,17 @@ export const createGitService = ({ return all; }, // @ts-expect-error - being weird about the return type - checkout: async (branch: string) => { + checkout: async ( + branch: string, + options?: { + fallbackBranch?: string; + } + ) => { + const _branchExistsLocally = await branchExistsLocally(branch); + if (!_branchExistsLocally && options?.fallbackBranch) { + return gitEngine.checkout(options.fallbackBranch); + } + return gitEngine.checkout(branch); }, addAllFiles: async () => { @@ -89,7 +113,12 @@ export const createGitService = ({ } }, rebaseContinue: async () => { - await gitEngine.rebase(['--continue']); + await gitEngine.raw([ + '-c', + 'core.editor=true', + 'rebase', + '--continue', + ]); }, mergeBaseBranch: async (branchA: string, branchB: string) => { const result = await gitEngine.raw([ @@ -120,6 +149,13 @@ export const createGitService = ({ branch: string; ontoBranch: string; }) => { + if ( + !(await branchExistsLocally(branch)) || + !(await branchExistsLocally(branch)) + ) { + return false; + } + const result = await gitEngine.raw([ 'merge-base', branch, @@ -168,5 +204,6 @@ export const createGitService = ({ branchDelete: async (branch: string) => { await gitEngine.deleteLocalBranch(branch, true); }, + branchExistsLocally, }; }; diff --git a/src/services/resolver.ts b/src/services/resolver.ts index 358bebd..c63f4c3 100644 --- a/src/services/resolver.ts +++ b/src/services/resolver.ts @@ -48,7 +48,8 @@ export const recursiveRebase = async ({ rebasedEventHandler(rebaseAction, 'COMPLETED'); } - await git.checkout(endBranch); + const rootBranchName = tree.find((b) => b.parent === null)?.key; + await git.checkout(endBranch, { fallbackBranch: rootBranchName }); completeEventHandler(); }; diff --git a/src/services/tree.ts b/src/services/tree.ts index 693408c..7008e0c 100644 --- a/src/services/tree.ts +++ b/src/services/tree.ts @@ -67,10 +67,18 @@ const moveOnto = ( const removeBranch = ( branch: string, - deps: { storeService: StoreService; setCurrentTree: SetTreeFunction } + deps: { storeService: StoreService; setCurrentTree: SetTreeFunction }, + options?: { ignoreBranchDoesNotExist?: boolean } ): BranchNode | undefined => { const tree = _readTree(deps); - const branchToRemove = _findBranch({ branch, tree }); + let branchToRemove: BranchNode; + + try { + branchToRemove = _findBranch({ branch, tree }); + } catch (e) { + if (options?.ignoreBranchDoesNotExist) return; + throw e; + } if (!branchToRemove) return; @@ -204,7 +212,10 @@ export interface TreeService { registerRoot: (branch: string) => void; attachTo: (args: { newBranch: string; parent: string }) => void; moveOnto: (args: { branch: string; parent: string }) => void; - removeBranch: (branch: string) => void; + removeBranch: ( + branch: string, + options?: { ignoreBranchDoesNotExist?: boolean } + ) => void; get: () => Tree; getRoot: () => BranchNode | undefined; ROOT: symbol; @@ -229,8 +240,12 @@ export const createTreeService = (config?: TreeServiceConfig): TreeService => { moveOnto: (args) => { return moveOnto(args, { storeService, setCurrentTree }); }, - removeBranch: (branch) => { - return removeBranch(branch, { storeService, setCurrentTree }); + removeBranch: (branch, options) => { + return removeBranch( + branch, + { storeService, setCurrentTree }, + options + ); }, get: () => { return _readTree({ storeService, setCurrentTree }); diff --git a/src/types.ts b/src/types.ts index 570baa9..b3af5cc 100644 --- a/src/types.ts +++ b/src/types.ts @@ -5,6 +5,8 @@ export type AsyncResult = | { value: T; isLoading: false } | { value: undefined; isLoading: true }; +export type AsyncResultWithDefault = { value: T; isLoading: boolean }; + export type SanitizeProps< T extends Record, // eslint-disable-next-line @typescript-eslint/no-explicit-any