Skip to content

Commit e243c2f

Browse files
JamieB-guJamieB-gu
authored
Refactor Result type (#14980)
A simple version of this type was introduced to see if we would find it useful. We've started to use it more widely, so this change adds some ergonomic improvements to make it easier to work with. There are three parts to this, as detailed below. This keeps the convenience functions `ok` and `error` for constructing `Result`, but it's possible these are no longer necessary as the `Ok` and `Err` classes could be constructed directly. An update to this may be considered in a future change. `kind` Becomes `ok` ------------------- The `kind` property, which was previously of type `'ok' | 'error'`, has been replaced with an `ok` property of type `boolean`. This makes code that checks this property less verbose, and it's similar to the `ok` property found on the `Response` type in the Fetch API. **Before:** ```ts if (result.kind === 'ok') { console.log(result.value); } else { console.error(result.error); } ``` **After:** ```ts if (result.ok) { console.log(result.value); } else { console.error(result.error); } ``` New Methods ----------- 1. `map` **Before:** ```ts if (result.kind === 'error') { return result; } return doSomething(result.value); ``` **After:** ```ts return result.map(doSomething); ``` 2. flatMap **Before:** ```ts if (result.kind === 'error') { return result; } return doSomethingThatCouldFail(result.value); ``` **After:** ```ts return result.flatMap(doSomethingThatCouldFail); ``` 3. `mapError` **Before:** ```ts if (result.kind === 'error') { return error(`New error message: ${result.error}`); } ``` **After:** ```ts return result.mapError(err => `New error message: ${err}`); ``` `okOrThrow` And `errorOrThrow` Become Methods --------------------------------------------- Previously these were functions, but they have now become methods alongside the others described above. They've also been renamed to better suit the method approach. **Before:** ```ts const value = okOrThrow(doSomething(data), 'Expected this to succeed'); const error = errorOrThrow( doSomething(data), 'Expected this to fail', ); ``` **After:** ```ts const value = doSomething(data).getOrThrow('Expected this to succeed'); const error = doSomething(data).getErrorOrThrow( 'Expected this to fail', ); ```
1 parent d802b3e commit e243c2f

26 files changed

+559
-344
lines changed

‎dotcom-rendering/src/components/Discussion.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -388,7 +388,7 @@ export const Discussion = ({
388388
signal: controller.signal,
389389
})
390390
.then((result) => {
391-
if (result.kind === 'error') {
391+
if (!result.ok) {
392392
if (result.error !== 'AbortedSignal') {
393393
console.error(`getDiscussion - error: ${result.error}`);
394394
}
@@ -440,7 +440,7 @@ export const Discussion = ({
440440
remapToValidFilters(filters, hashCommentId),
441441
)
442442
.then((context) => {
443-
if (context.kind === 'ok') {
443+
if (context.ok) {
444444
dispatch({
445445
type: 'updateCommentPage',
446446
commentPage: context.value.page,

‎dotcom-rendering/src/components/Discussion/AbuseReportForm.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ export const AbuseReportForm = ({
205205
commentId,
206206
})
207207
.then((response) => {
208-
if (response.kind === 'error') {
208+
if (!response.ok) {
209209
// Fallback to errors returned from the API
210210
setErrors({ ...errors, response: response.error });
211211
} else {

‎dotcom-rendering/src/components/Discussion/Comment.stories.tsx‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import type {
66
ReplyType,
77
Staff,
88
} from '../../lib/discussion';
9-
import { ok } from '../../lib/result';
9+
import { error, ok, type Result } from '../../lib/result';
1010
import { Comment } from './Comment';
1111

1212
type Props = Parameters<typeof Comment>[0];
@@ -113,10 +113,11 @@ const longBothReplyCommentData: ReplyType = {
113113
},
114114
};
115115

116-
const commentResponseError = {
117-
kind: 'error',
118-
error: 'NetworkError',
119-
} as const;
116+
const commentResponseError = function <A>(): Promise<
117+
Result<'NetworkError', A>
118+
> {
119+
return Promise.resolve(error('NetworkError'));
120+
};
120121

121122
const user: Reader = {
122123
kind: 'Reader',
@@ -134,8 +135,8 @@ const user: Reader = {
134135
hasCommented: true,
135136
},
136137
},
137-
onComment: () => Promise.resolve(commentResponseError),
138-
onReply: () => Promise.resolve(commentResponseError),
138+
onComment: commentResponseError,
139+
onReply: commentResponseError,
139140
onRecommend: () => Promise.resolve(true),
140141
addUsername: () => Promise.resolve(ok(true)),
141142
reportAbuse: () => Promise.resolve(ok(true)),
@@ -157,12 +158,12 @@ const staffUser: Staff = {
157158
hasCommented: true,
158159
},
159160
},
160-
onComment: () => Promise.resolve(commentResponseError),
161-
onReply: () => Promise.resolve(commentResponseError),
161+
onComment: commentResponseError,
162+
onReply: commentResponseError,
162163
onRecommend: () => Promise.resolve(true),
163164
addUsername: () => Promise.resolve(ok(true)),
164-
onPick: () => Promise.resolve(commentResponseError),
165-
onUnpick: () => Promise.resolve(commentResponseError),
165+
onPick: commentResponseError,
166+
onUnpick: commentResponseError,
166167
reportAbuse: () => Promise.resolve(ok(true)),
167168
};
168169

‎dotcom-rendering/src/components/Discussion/Comment.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -342,7 +342,7 @@ export const Comment = ({
342342
const pick = async (staffUser: Staff) => {
343343
setPickError('');
344344
const response = await staffUser.onPick(comment.id);
345-
if (response.kind === 'error') {
345+
if (!response.ok) {
346346
setPickError(response.error);
347347
} else {
348348
setIsHighlighted(response.value);
@@ -352,7 +352,7 @@ export const Comment = ({
352352
const unPick = async (staffUser: Staff) => {
353353
setPickError('');
354354
const response = await staffUser.onUnpick(comment.id);
355-
if (response.kind === 'error') {
355+
if (!response.ok) {
356356
setPickError(response.error);
357357
} else {
358358
setIsHighlighted(response.value);

‎dotcom-rendering/src/components/Discussion/CommentContainer.stories.tsx‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { splitTheme } from '../../../.storybook/decorators/splitThemeDecorator';
22
import { ArticleDesign, ArticleDisplay, Pillar } from '../../lib/articleFormat';
33
import type { CommentType, Reader, ReplyType } from '../../lib/discussion';
4-
import { ok } from '../../lib/result';
4+
import { error, ok, type Result } from '../../lib/result';
55
import { CommentContainer } from './CommentContainer';
66

77
export default { title: 'Discussion/CommentContainer' };
@@ -134,10 +134,11 @@ const commentDataWithLongThread: CommentType = {
134134
},
135135
};
136136

137-
const commentResponseError = {
138-
kind: 'error',
139-
error: 'NetworkError',
140-
} as const;
137+
const commentResponseError = function <A>(): Promise<
138+
Result<'NetworkError', A>
139+
> {
140+
return Promise.resolve(error('NetworkError'));
141+
};
141142

142143
const aUser: Reader = {
143144
kind: 'Reader',
@@ -155,8 +156,8 @@ const aUser: Reader = {
155156
hasCommented: true,
156157
},
157158
},
158-
onComment: () => Promise.resolve(commentResponseError),
159-
onReply: () => Promise.resolve(commentResponseError),
159+
onComment: commentResponseError,
160+
onReply: commentResponseError,
160161
onRecommend: () => Promise.resolve(true),
161162
addUsername: () => Promise.resolve(ok(true)),
162163
reportAbuse: () => Promise.resolve(ok(true)),

‎dotcom-rendering/src/components/Discussion/CommentContainer.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ export const CommentContainer = ({
120120
setLoading(true);
121121
getAllReplies(commentId)
122122
.then((result) => {
123-
if (result.kind === 'error') {
123+
if (!result.ok) {
124124
console.error(result.error);
125125
return;
126126
}

‎dotcom-rendering/src/components/Discussion/CommentForm.stories.tsx‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { useState } from 'react';
22
import { splitTheme } from '../../../.storybook/decorators/splitThemeDecorator';
33
import { ArticleDesign, ArticleDisplay, Pillar } from '../../lib/articleFormat';
44
import type { CommentType, Reader } from '../../lib/discussion';
5-
import { ok } from '../../lib/result';
5+
import { error, ok, type Result } from '../../lib/result';
66
import { CommentForm } from './CommentForm';
77

88
export default { component: CommentForm, title: 'Discussion/CommentForm' };
@@ -15,10 +15,11 @@ const defaultFormat = {
1515
theme: Pillar.News,
1616
};
1717

18-
const commentResponseError = {
19-
kind: 'error',
20-
error: 'NetworkError',
21-
} as const;
18+
const commentResponseError = function <A>(): Promise<
19+
Result<'NetworkError', A>
20+
> {
21+
return Promise.resolve(error('NetworkError'));
22+
};
2223

2324
const aUser: Reader = {
2425
kind: 'Reader',
@@ -36,8 +37,8 @@ const aUser: Reader = {
3637
hasCommented: true,
3738
},
3839
},
39-
onComment: () => Promise.resolve(commentResponseError),
40-
onReply: () => Promise.resolve(commentResponseError),
40+
onComment: commentResponseError,
41+
onReply: commentResponseError,
4142
onRecommend: () => Promise.resolve(true),
4243
addUsername: () => Promise.resolve(ok(true)),
4344
reportAbuse: () => Promise.resolve(ok(true)),

‎dotcom-rendering/src/components/Discussion/CommentForm.tsx‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,7 @@ export const CommentForm = ({
313313
const preview = onPreview ?? defaultPreview;
314314
const response = await preview(body);
315315

316-
if (response.kind === 'error') {
316+
if (!response.ok) {
317317
// If the preview fails, we handle the error and reset the preview body
318318
handleError(response.error, false);
319319
setPreviewBody('');
@@ -416,7 +416,7 @@ export const CommentForm = ({
416416
? await user.onReply(shortUrl, body, commentBeingRepliedTo.id)
417417
: await user.onComment(shortUrl, body);
418418
// Check response message for error states
419-
if (response.kind === 'error') {
419+
if (!response.ok) {
420420
handleError(response.error);
421421
} else {
422422
onAddComment(
@@ -447,7 +447,7 @@ export const CommentForm = ({
447447
}
448448

449449
const response = await user.addUsername(userName);
450-
if (response.kind === 'ok') {
450+
if (response.ok) {
451451
// If we are able to submit userName we should continue with submitting comment
452452
void submitForm();
453453
setUserNameMissing(false);

‎dotcom-rendering/src/components/Discussion/Comments.stories.tsx‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import type {
1212
Reader,
1313
} from '../../lib/discussion';
1414
import { discussionApiResponseSchema } from '../../lib/discussion';
15-
import { ok } from '../../lib/result';
15+
import { error, ok, type Result } from '../../lib/result';
1616
import { Comments } from './Comments';
1717

1818
const meta = {
@@ -44,10 +44,11 @@ const discussionWithTwoComments = parse(
4444
);
4545
if (discussionWithTwoComments.status !== 'ok') throw new Error('Invalid mock');
4646

47-
const commentResponseError = {
48-
kind: 'error',
49-
error: 'NetworkError',
50-
} as const;
47+
const commentResponseError = function <A>(): Promise<
48+
Result<'NetworkError', A>
49+
> {
50+
return Promise.resolve(error('NetworkError'));
51+
};
5152

5253
const aUser: Reader = {
5354
kind: 'Reader',
@@ -65,8 +66,8 @@ const aUser: Reader = {
6566
hasCommented: true,
6667
},
6768
},
68-
onComment: () => Promise.resolve(commentResponseError),
69-
onReply: () => Promise.resolve(commentResponseError),
69+
onComment: commentResponseError,
70+
onReply: commentResponseError,
7071
onRecommend: () => Promise.resolve(true),
7172
addUsername: () => Promise.resolve(ok(true)),
7273
reportAbuse: () => Promise.resolve(ok(true)),

‎dotcom-rendering/src/components/Discussion/Comments.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,7 @@ export const Comments = ({
228228

229229
useEffect(() => {
230230
void getPicks(shortUrl).then((result) => {
231-
if (result.kind === 'error') {
231+
if (!result.ok) {
232232
console.error(result.error);
233233
return;
234234
}

0 commit comments

Comments
 (0)