Skip to content

Commit 562520b

Browse files
mtsgrdmtsgrd
committed
fix: validate stackId before operations
Ensure stackId is validated before calling functions that require it. Replace explicit null-checks and early returns with ensureValue(stackId) so callers always pass a non-null stackId to stackService and related helpers. Key changes: - Import ensureValue in multiple components (CommitContextMenu, BranchList, BranchCommitList). - Use ensureValue(stackId) when calling insertBlankCommit, handleCreateNewRef, stackService.uncommit, editPatch, insertBlankCommitInBranch, addDependentBranch modal setup, and integrateUpstreamCommits. - Fix selection close handler in StackView to call itelligentScrollingService with laneId instead of stackId (uses the intended lane identifier). Reason: Avoid scattered null-checks and early returns that silently fail action handlers. Using ensureValue centralizes validation and surfaces errors earlier, leading to more predictable behavior and fewer no-op UI actions.
1 parent 2b1911a commit 562520b

File tree

10 files changed

+47
-43
lines changed

10 files changed

+47
-43
lines changed

‎apps/desktop/src/components/BranchCommitList.svelte

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import { STACK_SERVICE, type SeriesIntegrationStrategy } from '$lib/stacks/stackService.svelte';
3232
import { combineResults } from '$lib/state/helpers';
3333
import { UI_STATE } from '$lib/state/uiState.svelte';
34+
import { ensureValue } from '$lib/utils/validation';
3435
import { inject } from '@gitbutler/shared/context';
3536
import { Button, Modal, TestId } from '@gitbutler/ui';
3637
import { getTimeAgo } from '@gitbutler/ui/utils/timeAgo';
@@ -130,10 +131,9 @@
130131
let confirmResetModal = $state<ReturnType<typeof Modal>>();
131132
132133
async function integrate(strategy?: SeriesIntegrationStrategy): Promise<void> {
133-
if (!stackId) return;
134134
await integrateUpstreamCommits({
135135
projectId,
136-
stackId,
136+
stackId: ensureValue(stackId),
137137
seriesName: branchName,
138138
strategy
139139
});

‎apps/desktop/src/components/BranchList.svelte

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import { combineResults } from '$lib/state/helpers';
2020
import { UI_STATE } from '$lib/state/uiState.svelte';
2121
import { URL_SERVICE } from '$lib/utils/url';
22+
import { ensureValue } from '$lib/utils/validation';
2223
import { copyToClipboard } from '@gitbutler/shared/clipboard';
2324
import { inject } from '@gitbutler/shared/context';
2425
@@ -66,8 +67,12 @@
6667
$state<ReturnType<typeof ConflictResolutionConfirmModal>>();
6768
6869
async function handleUncommit(commitId: string, branchName: string) {
69-
if (!stackId) return;
70-
await stackService.uncommit({ projectId, stackId, branchName, commitId: commitId });
70+
await stackService.uncommit({
71+
projectId,
72+
stackId: ensureValue(stackId),
73+
branchName,
74+
commitId: commitId
75+
});
7176
}
7277
7378
function startEditingCommitMessage(branchName: string, commitId: string) {
@@ -86,15 +91,14 @@
8691
hasConflicts: boolean;
8792
isAncestorMostConflicted: boolean;
8893
}) {
89-
if (!stackId) return;
9094
if (args.type === 'LocalAndRemote' && args.hasConflicts && !args.isAncestorMostConflicted) {
9195
conflictResolutionConfirmationModal?.show();
9296
return;
9397
}
9498
await editPatch({
9599
modeService,
96100
commitId: args.commitId,
97-
stackId,
101+
stackId: ensureValue(stackId),
98102
projectId
99103
});
100104
}
@@ -198,10 +202,9 @@
198202
kind="outline"
199203
tooltip="Create empty commit"
200204
onclick={async () => {
201-
if (!stackId) return;
202205
await insertBlankCommitInBranch({
203206
projectId,
204-
stackId,
207+
stackId: ensureValue(stackId),
205208
commitId: undefined,
206209
offset: -1
207210
});
@@ -224,10 +227,9 @@
224227
kind="outline"
225228
tooltip="Create new branch"
226229
onclick={async () => {
227-
if (!stackId) return;
228230
addDependentBranchModalContext = {
229231
projectId,
230-
stackId
232+
stackId: ensureValue(stackId)
231233
};
232234

233235
await tick();

‎apps/desktop/src/components/CommitContextMenu.svelte

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
import { rewrapCommitMessage } from '$lib/config/uiFeatureFlags';
4747
import { STACK_SERVICE } from '$lib/stacks/stackService.svelte';
4848
import { URL_SERVICE } from '$lib/utils/url';
49+
import { ensureValue } from '$lib/utils/validation';
4950
import { inject } from '@gitbutler/shared/context';
5051
import {
5152
ContextMenu,
@@ -200,17 +201,15 @@
200201
label="Add empty commit above"
201202
disabled={commitInsertion.current.isLoading}
202203
onclick={() => {
203-
if (!stackId) return;
204-
insertBlankCommit(stackId, commitId, 'above');
204+
insertBlankCommit(ensureValue(stackId), commitId, 'above');
205205
close();
206206
}}
207207
/>
208208
<ContextMenuItem
209209
label="Add empty commit below"
210210
disabled={commitInsertion.current.isLoading}
211211
onclick={() => {
212-
if (!stackId) return;
213-
insertBlankCommit(stackId, commitId, 'below');
212+
insertBlankCommit(ensureValue(stackId), commitId, 'below');
214213
close();
215214
}}
216215
/>
@@ -220,17 +219,15 @@
220219
label="Create dependent branch above"
221220
disabled={refCreation.current.isLoading}
222221
onclick={async () => {
223-
if (!stackId) return;
224-
await handleCreateNewRef(stackId, commitId, 'Above');
222+
await handleCreateNewRef(ensureValue(stackId), commitId, 'Above');
225223
close();
226224
}}
227225
/>
228226
<ContextMenuItem
229227
label="Create dependent branch below"
230228
disabled={refCreation.current.isLoading}
231229
onclick={async () => {
232-
if (!stackId) return;
233-
await handleCreateNewRef(stackId, commitId, 'Below');
230+
await handleCreateNewRef(ensureValue(stackId), commitId, 'Below');
234231
close();
235232
}}
236233
/>

‎apps/desktop/src/components/CommitView.svelte

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import { STACK_SERVICE } from '$lib/stacks/stackService.svelte';
1818
import { UI_STATE } from '$lib/state/uiState.svelte';
1919
import { splitMessage } from '$lib/utils/commitMessage';
20+
import { ensureValue } from '$lib/utils/validation';
2021
import { inject, injectOptional } from '@gitbutler/shared/context';
2122
import { AsyncButton, Button, TestId } from '@gitbutler/ui';
2223
@@ -106,7 +107,6 @@
106107
}
107108
108109
async function saveCommitMessage(title: string, description: string) {
109-
if (!stackId) return;
110110
const commitMessage = combineParts(title, description);
111111
if (!branchName) {
112112
throw new Error('No branch selected!');
@@ -118,31 +118,34 @@
118118
119119
const newCommitId = await updateCommitMessage({
120120
projectId,
121-
stackId,
121+
stackId: ensureValue(stackId),
122122
commitId: commitKey.commitId,
123123
message: commitMessage
124124
});
125125
126-
uiState.lane(stackId).selection.set({ branchName, commitId: newCommitId });
126+
uiState.lane(ensureValue(stackId)).selection.set({ branchName, commitId: newCommitId });
127127
setMode('view');
128128
}
129129
130130
async function handleUncommit() {
131-
if (!stackId) return;
132131
if (!branchName) return;
133-
await stackService.uncommit({ projectId, stackId, branchName, commitId: commitKey.commitId });
132+
await stackService.uncommit({
133+
projectId,
134+
stackId: ensureValue(stackId),
135+
branchName,
136+
commitId: commitKey.commitId
137+
});
134138
}
135139
136140
function canEdit() {
137141
return modeService !== undefined;
138142
}
139143
140144
async function handleEditPatch() {
141-
if (!stackId) return;
142145
await editPatch({
143146
modeService,
144147
commitId: commitKey.commitId,
145-
stackId,
148+
stackId: ensureValue(stackId),
146149
projectId
147150
});
148151
}

‎apps/desktop/src/components/NewBranchModal.svelte

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
<script lang="ts">
22
import { STACK_SERVICE } from '$lib/stacks/stackService.svelte';
33
import { URL_SERVICE } from '$lib/utils/url';
4+
import { ensureValue } from '$lib/utils/validation';
45
import { inject } from '@gitbutler/shared/context';
56
67
import { Button, LinkButton, Modal, Textbox, chipToasts } from '@gitbutler/ui';
@@ -32,7 +33,6 @@
3233
const generatedNameDiverges = $derived(!!createRefName && slugifiedRefName !== createRefName);
3334
3435
async function addSeries() {
35-
if (!stackId) return;
3636
if (!slugifiedRefName) {
3737
chipToasts.error('No branch name provided');
3838
createRefModal?.close();
@@ -41,7 +41,7 @@
4141
4242
await createNewBranch({
4343
projectId,
44-
stackId,
44+
stackId: ensureValue(stackId),
4545
request: { targetPatch: undefined, name: slugifiedRefName }
4646
});
4747

‎apps/desktop/src/components/PushButton.svelte

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,6 @@
6969
}
7070
7171
async function push(args: { withForce: boolean; skipForcePushProtection: boolean }) {
72-
if (!stackId) return;
7372
const { withForce, skipForcePushProtection } = args;
7473
try {
7574
const pushResult = await pushStack({

‎apps/desktop/src/components/StackView.svelte

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -241,8 +241,7 @@
241241
selectionId={createWorktreeSelection({ stackId })}
242242
onclose={() => {
243243
idSelection.clear(createWorktreeSelection({ stackId: stackId }));
244-
if (!stackId) return;
245-
intelligentScrollingService.show(projectId, stackId, 'stack');
244+
intelligentScrollingService.show(projectId, laneId, 'stack');
246245
}}
247246
draggableFiles
248247
/>

‎apps/desktop/src/lib/stacks/dropHandler.ts

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { changesToDiffSpec } from '$lib/commits/utils';
22
import { ChangeDropData } from '$lib/dragging/draggables';
33
import StackMacros from '$lib/stacks/macros';
4+
import { ensureValue } from '$lib/utils/validation';
45
import { chipToasts } from '@gitbutler/ui';
56
import type { DropzoneHandler } from '$lib/dragging/handler';
67
import type { DiffService } from '$lib/hunks/diffService.svelte';
@@ -70,9 +71,6 @@ export class OutsideLaneDzHandler implements DropzoneHandler {
7071
case 'commit': {
7172
const { stack, outcome, branchName } = await this.macros.createNewStackAndCommit();
7273

73-
if (!stack.id) {
74-
throw new Error('New stack has no stack id');
75-
}
7674
if (!outcome.newCommit) {
7775
throw new Error('Failed to create a new commit');
7876
}
@@ -82,7 +80,7 @@ export class OutsideLaneDzHandler implements DropzoneHandler {
8280
if (sourceStackId) {
8381
const diffSpec = changesToDiffSpec(await data.treeChanges());
8482
await this.macros.moveChangesToNewCommit(
85-
stack.id,
83+
ensureValue(stack.id),
8684
outcome.newCommit,
8785
sourceStackId,
8886
sourceCommitId,
@@ -100,17 +98,13 @@ export class OutsideLaneDzHandler implements DropzoneHandler {
10098
projectId: this.projectId,
10199
branch: { name: undefined }
102100
});
103-
const stackId = stack.id;
104-
if (!stackId) {
105-
throw new Error('New stack has no stack id');
106-
}
107101

108102
const changes = await data.treeChanges();
109103
const assignments = changes
110104
.flatMap((c) =>
111105
this.uncommittedService.getAssignmentsByPath(data.stackId ?? null, c.path)
112106
)
113-
.map((h) => ({ ...h, stackId }));
107+
.map((h) => ({ ...h, stackId: ensureValue(stack.id) }));
114108
await this.diffService.assignHunk({
115109
projectId: this.projectId,
116110
assignments

‎apps/desktop/src/lib/stacks/macros.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { getStackName } from '$lib/stacks/stack';
2+
import { ensureValue } from '$lib/utils/validation';
23
import type { DiffSpec } from '$lib/hunks/hunk';
34
import type {
45
CreateCommitRequestWorktreeChanges,
@@ -58,13 +59,10 @@ export default class StackMacros {
5859
projectId: this.projectId,
5960
branch: { name }
6061
});
61-
if (!stack.id) {
62-
throw new Error('New stack has no stack id');
63-
}
6462
const branchName = getStackName(stack);
6563
const outcome = await this.stackService.createCommitMutation({
6664
projectId: this.projectId,
67-
stackId: stack.id,
65+
stackId: ensureValue(stack.id),
6866
stackBranchName: branchName,
6967
parentId: undefined,
7068
message: message ?? STUB_COMMIT_MESSAGE,

‎apps/desktop/src/lib/utils/validation.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
/**
2+
* Ensures that a value is not null or undefined, throwing an error if it is.
3+
*/
4+
export function ensureValue<T>(value: T | null | undefined): T {
5+
if (value === null || value === undefined) {
6+
const message = `Expected value but got ${value === null ? 'null' : 'undefined'}`;
7+
const error = new Error(message);
8+
Error.captureStackTrace(error, ensureValue);
9+
throw error;
10+
}
11+
return value;
12+
}

0 commit comments

Comments
 (0)