Skip to content

Commit fed151c

Browse files
leeyi45leeyi45
committed
Upgrade saga handler to allow using takeLatest or takeLeading
1 parent 9237602 commit fed151c

File tree

5 files changed

+155
-121
lines changed

5 files changed

+155
-121
lines changed

‎src/commons/redux/utils.ts‎

Lines changed: 42 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,15 @@
11
import {
2-
ActionCreatorWithOptionalPayload,
3-
ActionCreatorWithoutPayload,
4-
ActionCreatorWithPreparedPayload,
2+
type ActionCreatorWithOptionalPayload,
3+
type ActionCreatorWithoutPayload,
4+
type ActionCreatorWithPreparedPayload,
55
createAction
66
} from '@reduxjs/toolkit';
77
import * as Sentry from '@sentry/browser';
8-
import { SagaIterator } from 'redux-saga';
9-
import { StrictEffect, takeEvery } from 'redux-saga/effects';
8+
import type { SagaIterator } from 'redux-saga';
9+
import { type StrictEffect, takeLatest, takeLeading } from 'redux-saga/effects';
10+
11+
import { safeTakeEvery } from '../sagas/SafeEffects';
12+
import { objectEntries } from '../utils/TypeHelper';
1013

1114
/**
1215
* Creates actions, given a base name and base actions
@@ -39,6 +42,20 @@ export function createActions<BaseName extends string, BaseActions extends Recor
3942
);
4043
}
4144

45+
function wrapSaga<T extends (...args: any[]) => Generator>(saga: T) {
46+
return function* (...args: Parameters<T>) {
47+
try {
48+
return yield* saga(...args);
49+
} catch (error) {
50+
handleUncaughtError(error);
51+
}
52+
};
53+
}
54+
55+
type SagaHandler<
56+
T extends ActionCreatorWithPreparedPayload<any, any> | ActionCreatorWithoutPayload<any>
57+
> = (action: ReturnType<T>) => Generator<StrictEffect>;
58+
4259
export function combineSagaHandlers<
4360
TActions extends Record<
4461
string,
@@ -47,17 +64,29 @@ export function combineSagaHandlers<
4764
>(
4865
actions: TActions,
4966
handlers: {
50-
// TODO: Maybe this can be stricter? And remove the optional type after
51-
// migration is fully done
52-
[K in keyof TActions]?: (action: ReturnType<TActions[K]>) => SagaIterator;
67+
// TODO: Maybe this can be stricter? And remove the optional type after migration is fully done
68+
[K in keyof TActions]?:
69+
| SagaHandler<TActions[K]>
70+
| { takeLeading: SagaHandler<TActions[K]> }
71+
| { takeLatest: SagaHandler<TActions[K]> };
5372
},
5473
others?: (takeEvery: typeof saferTakeEvery) => SagaIterator
5574
): () => SagaIterator {
56-
const sagaHandlers = Object.entries(handlers).map(([actionName, saga]) =>
57-
saferTakeEvery(actions[actionName], saga)
58-
);
5975
return function* (): SagaIterator {
60-
yield* sagaHandlers;
76+
for (const [actionName, saga] of objectEntries(handlers)) {
77+
if (saga === undefined) {
78+
continue;
79+
} else if (typeof saga === 'function') {
80+
yield safeTakeEvery(actions[actionName].type, saga);
81+
} else if ('takeLeading' in saga) {
82+
yield takeLeading(actions[actionName].type, wrapSaga(saga.takeLeading));
83+
} else if ('takeLatest' in saga) {
84+
yield takeLatest(actions[actionName].type, wrapSaga(saga.takeLatest));
85+
} else {
86+
throw new Error(`Unknown saga handler type for ${actionName as string}`);
87+
}
88+
}
89+
6190
if (others) {
6291
const obj = others(saferTakeEvery);
6392
while (true) {
@@ -74,15 +103,7 @@ export function saferTakeEvery<
74103
| ActionCreatorWithOptionalPayload<any>
75104
| ActionCreatorWithPreparedPayload<any[], any>
76105
>(actionPattern: Action, fn: (action: ReturnType<Action>) => Generator<StrictEffect<any>>) {
77-
function* wrapper(action: ReturnType<Action>) {
78-
try {
79-
yield* fn(action);
80-
} catch (error) {
81-
handleUncaughtError(error);
82-
}
83-
}
84-
85-
return takeEvery(actionPattern.type, wrapper);
106+
return safeTakeEvery(actionPattern.type, fn);
86107
}
87108

88109
function handleUncaughtError(error: any) {

‎src/commons/sagas/PlaygroundSaga.ts‎

Lines changed: 65 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
1-
import { FSModule } from 'browserfs/dist/node/core/FS';
1+
import type { FSModule } from 'browserfs/dist/node/core/FS';
22
import { Chapter } from 'js-slang/dist/types';
33
import { compressToEncodedURIComponent } from 'lz-string';
44
import qs from 'query-string';
5-
import { SagaIterator } from 'redux-saga';
65
import { call, delay, put, race, select } from 'redux-saga/effects';
76
import CseMachine from 'src/features/cseMachine/CseMachine';
87
import { CseMachine as JavaCseMachine } from 'src/features/cseMachine/java/CseMachine';
@@ -14,21 +13,20 @@ import {
1413
type OverallState
1514
} from '../application/ApplicationTypes';
1615
import { retrieveFilesInWorkspaceAsRecord } from '../fileSystem/utils';
16+
import { combineSagaHandlers } from '../redux/utils';
1717
import { visitSideContent } from '../sideContent/SideContentActions';
1818
import { SideContentType } from '../sideContent/SideContentTypes';
1919
import Constants from '../utils/Constants';
2020
import { showSuccessMessage, showWarningMessage } from '../utils/notifications/NotificationsHelper';
2121
import WorkspaceActions from '../workspace/WorkspaceActions';
22-
import { safeTakeEvery as takeEvery, selectWorkspace } from './SafeEffects';
22+
import { selectWorkspace } from './SafeEffects';
2323

24-
export default function* PlaygroundSaga(): SagaIterator {
25-
yield takeEvery(PlaygroundActions.generateLzString.type, updateQueryString);
26-
27-
yield takeEvery(
28-
PlaygroundActions.shortenURL.type,
29-
function* (action: ReturnType<typeof PlaygroundActions.shortenURL>): any {
24+
const PlaygroundSaga = combineSagaHandlers(
25+
PlaygroundActions,
26+
{
27+
generateLzString: updateQueryString,
28+
shortenURL: function* ({ payload: keyword }) {
3029
const queryString = yield select((state: OverallState) => state.playground.queryString);
31-
const keyword = action.payload;
3230
const errorMsg = 'ERROR';
3331

3432
let resp, timeout;
@@ -59,72 +57,76 @@ export default function* PlaygroundSaga(): SagaIterator {
5957
}
6058
yield put(PlaygroundActions.updateShortURL(Constants.urlShortenerBase + resp.url.keyword));
6159
}
62-
);
63-
64-
yield takeEvery(
65-
visitSideContent.type,
66-
function* ({
67-
payload: { newId, prevId, workspaceLocation }
68-
}: ReturnType<typeof visitSideContent>) {
69-
if (workspaceLocation !== 'playground' || newId === prevId) return;
60+
},
61+
function* (takeEvery) {
62+
yield takeEvery(
63+
visitSideContent,
64+
function* ({ payload: { newId, prevId, workspaceLocation } }) {
65+
if (workspaceLocation !== 'playground' || newId === prevId) return;
66+
67+
// Do nothing when clicking the mobile 'Run' tab while on the stepper tab.
68+
if (
69+
prevId === SideContentType.substVisualizer &&
70+
newId === SideContentType.mobileEditorRun
71+
) {
72+
return;
73+
}
7074

71-
// Do nothing when clicking the mobile 'Run' tab while on the stepper tab.
72-
if (prevId === SideContentType.substVisualizer && newId === SideContentType.mobileEditorRun) {
73-
return;
74-
}
75+
const {
76+
context: { chapter: playgroundSourceChapter },
77+
editorTabs
78+
} = yield* selectWorkspace('playground');
7579

76-
const {
77-
context: { chapter: playgroundSourceChapter },
78-
editorTabs
79-
} = yield* selectWorkspace('playground');
80+
if (prevId === SideContentType.substVisualizer) {
81+
if (newId === SideContentType.mobileEditorRun) return;
82+
const hasBreakpoints = editorTabs.find(({ breakpoints }) => breakpoints.find(x => !!x));
8083

81-
if (prevId === SideContentType.substVisualizer) {
82-
if (newId === SideContentType.mobileEditorRun) return;
83-
const hasBreakpoints = editorTabs.find(({ breakpoints }) => breakpoints.find(x => !!x));
84+
if (!hasBreakpoints) {
85+
yield put(WorkspaceActions.toggleUsingSubst(false, workspaceLocation));
86+
yield put(WorkspaceActions.clearReplOutput(workspaceLocation));
87+
}
88+
}
8489

85-
if (!hasBreakpoints) {
86-
yield put(WorkspaceActions.toggleUsingSubst(false, workspaceLocation));
87-
yield put(WorkspaceActions.clearReplOutput(workspaceLocation));
90+
if (newId !== SideContentType.cseMachine) {
91+
yield put(WorkspaceActions.toggleUsingCse(false, workspaceLocation));
92+
yield call([CseMachine, CseMachine.clearCse]);
93+
yield call([JavaCseMachine, JavaCseMachine.clearCse]);
94+
yield put(WorkspaceActions.updateCurrentStep(-1, workspaceLocation));
95+
yield put(WorkspaceActions.updateStepsTotal(0, workspaceLocation));
96+
yield put(WorkspaceActions.toggleUpdateCse(true, workspaceLocation));
97+
yield put(WorkspaceActions.setEditorHighlightedLines(workspaceLocation, 0, []));
8898
}
89-
}
9099

91-
if (newId !== SideContentType.cseMachine) {
92-
yield put(WorkspaceActions.toggleUsingCse(false, workspaceLocation));
93-
yield call([CseMachine, CseMachine.clearCse]);
94-
yield call([JavaCseMachine, JavaCseMachine.clearCse]);
95-
yield put(WorkspaceActions.updateCurrentStep(-1, workspaceLocation));
96-
yield put(WorkspaceActions.updateStepsTotal(0, workspaceLocation));
97-
yield put(WorkspaceActions.toggleUpdateCse(true, workspaceLocation));
98-
yield put(WorkspaceActions.setEditorHighlightedLines(workspaceLocation, 0, []));
99-
}
100+
if (playgroundSourceChapter === Chapter.FULL_JAVA && newId === SideContentType.cseMachine) {
101+
yield put(WorkspaceActions.toggleUsingCse(true, workspaceLocation));
102+
}
100103

101-
if (playgroundSourceChapter === Chapter.FULL_JAVA && newId === SideContentType.cseMachine) {
102-
yield put(WorkspaceActions.toggleUsingCse(true, workspaceLocation));
103-
}
104+
if (
105+
isSourceLanguage(playgroundSourceChapter) &&
106+
(newId === SideContentType.substVisualizer || newId === SideContentType.cseMachine)
107+
) {
108+
if (playgroundSourceChapter <= Chapter.SOURCE_2) {
109+
yield put(WorkspaceActions.toggleUsingSubst(true, workspaceLocation));
110+
} else {
111+
yield put(WorkspaceActions.toggleUsingCse(true, workspaceLocation));
112+
}
113+
}
104114

105-
if (
106-
isSourceLanguage(playgroundSourceChapter) &&
107-
(newId === SideContentType.substVisualizer || newId === SideContentType.cseMachine)
108-
) {
109-
if (playgroundSourceChapter <= Chapter.SOURCE_2) {
110-
yield put(WorkspaceActions.toggleUsingSubst(true, workspaceLocation));
115+
if (newId === SideContentType.upload) {
116+
yield put(WorkspaceActions.toggleUsingUpload(true, workspaceLocation));
111117
} else {
112-
yield put(WorkspaceActions.toggleUsingCse(true, workspaceLocation));
118+
yield put(WorkspaceActions.toggleUsingUpload(false, workspaceLocation));
113119
}
114-
}
115120

116-
if (newId === SideContentType.upload) {
117-
yield put(WorkspaceActions.toggleUsingUpload(true, workspaceLocation));
118-
} else {
119-
yield put(WorkspaceActions.toggleUsingUpload(false, workspaceLocation));
121+
if (isSchemeLanguage(playgroundSourceChapter) && newId === SideContentType.cseMachine) {
122+
yield put(WorkspaceActions.toggleUsingCse(true, workspaceLocation));
123+
}
120124
}
125+
);
126+
}
127+
);
121128

122-
if (isSchemeLanguage(playgroundSourceChapter) && newId === SideContentType.cseMachine) {
123-
yield put(WorkspaceActions.toggleUsingCse(true, workspaceLocation));
124-
}
125-
}
126-
);
127-
}
129+
export default PlaygroundSaga;
128130

129131
function* updateQueryString() {
130132
const fileSystem: FSModule = yield select(

‎src/commons/sagas/RemoteExecutionSaga.ts‎

Lines changed: 25 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -36,32 +36,33 @@ const dummyLocation = {
3636
// TODO: Refactor and combine in a future commit
3737
const sagaActions = { ...RemoteExecutionActions, ...InterpreterActions };
3838
const RemoteExecutionSaga = combineSagaHandlers(sagaActions, {
39-
// TODO: Should be `takeLatest`, not `takeEvery`
40-
remoteExecFetchDevices: function* () {
41-
const [tokens, session]: [any, DeviceSession | undefined] = yield select(
42-
(state: OverallState) => [
43-
{
44-
accessToken: state.session.accessToken,
45-
refreshToken: state.session.refreshToken
46-
},
47-
state.session.remoteExecutionSession
48-
]
49-
);
50-
const devices: Device[] = yield call(fetchDevices, tokens);
39+
remoteExecFetchDevices: {
40+
takeLatest: function* () {
41+
const [tokens, session]: [any, DeviceSession | undefined] = yield select(
42+
(state: OverallState) => [
43+
{
44+
accessToken: state.session.accessToken,
45+
refreshToken: state.session.refreshToken
46+
},
47+
state.session.remoteExecutionSession
48+
]
49+
);
50+
const devices: Device[] = yield call(fetchDevices, tokens);
5151

52-
yield put(actions.remoteExecUpdateDevices(devices));
52+
yield put(actions.remoteExecUpdateDevices(devices));
5353

54-
if (!session) {
55-
return;
56-
}
57-
const updatedDevice = devices.find(({ id }) => id === session.device.id);
58-
if (updatedDevice) {
59-
yield put(
60-
actions.remoteExecUpdateSession({
61-
...session,
62-
device: updatedDevice
63-
})
64-
);
54+
if (!session) {
55+
return;
56+
}
57+
const updatedDevice = devices.find(({ id }) => id === session.device.id);
58+
if (updatedDevice) {
59+
yield put(
60+
actions.remoteExecUpdateSession({
61+
...session,
62+
device: updatedDevice
63+
})
64+
);
65+
}
6566
}
6667
},
6768
remoteExecConnect: function* (action): any {

‎src/commons/sagas/StoriesSaga.ts‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,15 +28,16 @@ import { evalCodeSaga } from './WorkspaceSaga/helpers/evalCode';
2828
// TODO: Refactor and combine in a future commit
2929
const sagaActions = { ...StoriesActions, ...SessionActions };
3030
const StoriesSaga = combineSagaHandlers(sagaActions, {
31-
// TODO: This should be using `takeLatest`, not `takeEvery`
32-
getStoriesList: function* () {
33-
const tokens: Tokens = yield selectTokens();
34-
const allStories: StoryListView[] = yield call(async () => {
35-
const resp = await getStories(tokens);
36-
return resp ?? [];
37-
});
38-
39-
yield put(actions.updateStoriesList(allStories));
31+
getStoriesList: {
32+
takeLatest: function* () {
33+
const tokens: Tokens = yield selectTokens();
34+
const allStories: StoryListView[] = yield call(async () => {
35+
const resp = await getStories(tokens);
36+
return resp ?? [];
37+
});
38+
39+
yield put(actions.updateStoriesList(allStories));
40+
}
4041
},
4142
setCurrentStoryId: function* (action) {
4243
const tokens: Tokens = yield selectTokens();

‎src/commons/utils/TypeHelper.ts‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -157,11 +157,20 @@ export function objectValues<T>(obj: Record<any, T>) {
157157
return Object.values(obj) as T[];
158158
}
159159

160+
/**
161+
* Utility type for getting the key-value tuple types
162+
* of a record
163+
*/
164+
type DeconstructRecord<T extends Record<any, any>> = Exclude<
165+
{
166+
[K in keyof T]: [K, T[K]];
167+
}[keyof T],
168+
undefined
169+
>[];
170+
160171
/**
161172
* Type safe `Object.entries`
162173
*/
163-
export function objectEntries<K extends string | number | symbol, V>(
164-
obj: Partial<Record<K, V>>
165-
): [K, V][] {
166-
return Object.entries(obj) as [K, V][];
174+
export function objectEntries<T extends Record<any, any>>(obj: T) {
175+
return Object.entries(obj) as DeconstructRecord<T>;
167176
}

0 commit comments

Comments
 (0)