Remove Concurrent and Interpreter (#3130)

* Remove concurrent variant

* Fix broken tests

* Begin fixing

* Fix all broken workspace reducer tests

* FIx more broken workspace saga tests

* Refactor eval code to utilize more type safety

* Minor refactoring

* Fix broken tsc

* Upgrade saga handler to allow using takeLatest or takeLeading

* Modify sagas to use new combineSagaHandlers function

* Add redux tests

* Add more tests

* Modify actions and sagas to use new redux utils

* Fix Linting

---------

Co-authored-by: Martin Henz <[email protected]>

Author: leeyi45

src/commons/__tests__/Markdown.tsx

@@ -31,9 +31,6 @@ test('Markdown page renders correct Source information', () => {
   const source3Default = <Markdown {...mockProps(Chapter.SOURCE_3, Variant.DEFAULT)} />;
   expect(source3Default.props.content).toContain('Source \xa73');
 
-  const source3Concurrent = <Markdown {...mockProps(Chapter.SOURCE_3, Variant.CONCURRENT)} />;
-  expect(source3Concurrent.props.content).toContain('Source \xa73 Concurrent');
-
   const source4Default = <Markdown {...mockProps(Chapter.SOURCE_4, Variant.DEFAULT)} />;
   expect(source4Default.props.content).toContain('Source \xa74');
 });

src/commons/application/ApplicationTypes.ts

@@ -1,27 +1,27 @@
-import { Chapter, Language, SourceError, Variant } from 'js-slang/dist/types';
+import { Chapter, Language, type SourceError, type Value, Variant } from 'js-slang/dist/types';
 
-import { AchievementState } from '../../features/achievement/AchievementTypes';
-import { DashboardState } from '../../features/dashboard/DashboardTypes';
-import { PlaygroundState } from '../../features/playground/PlaygroundTypes';
+import type { AchievementState } from '../../features/achievement/AchievementTypes';
+import type { DashboardState } from '../../features/dashboard/DashboardTypes';
+import type { PlaygroundState } from '../../features/playground/PlaygroundTypes';
 import { PlaybackStatus, RecordingStatus } from '../../features/sourceRecorder/SourceRecorderTypes';
-import { StoriesEnvState, StoriesState } from '../../features/stories/StoriesTypes';
+import type { StoriesEnvState, StoriesState } from '../../features/stories/StoriesTypes';
 import { freshSortState } from '../../pages/academy/grading/subcomponents/GradingSubmissionsTable';
 import { WORKSPACE_BASE_PATHS } from '../../pages/fileSystem/createInBrowserFileSystem';
 import { defaultFeatureFlags, FeatureFlagsState } from '../featureFlags';
-import { FileSystemState } from '../fileSystem/FileSystemTypes';
-import { SideContentManagerState, SideContentState } from '../sideContent/SideContentTypes';
+import type { FileSystemState } from '../fileSystem/FileSystemTypes';
+import type { SideContentManagerState, SideContentState } from '../sideContent/SideContentTypes';
 import Constants from '../utils/Constants';
 import { createContext } from '../utils/JsSlangHelper';
-import {
+import type {
   DebuggerContext,
   WorkspaceLocation,
   WorkspaceManagerState,
   WorkspaceState
 } from '../workspace/WorkspaceTypes';
-import { RouterState } from './types/CommonsTypes';
+import type { RouterState } from './types/CommonsTypes';
 import { ExternalLibraryName } from './types/ExternalTypes';
-import { SessionState } from './types/SessionTypes';
-import { VscodeState as VscodeState } from './types/VscodeTypes';
+import type { SessionState } from './types/SessionTypes';
+import type { VscodeState as VscodeState } from './types/VscodeTypes';
 
 export type OverallState = {
   readonly router: RouterState;
@@ -74,7 +74,7 @@ export type CodeOutput = {
  */
 export type ResultOutput = {
   type: 'result';
-  value: any;
+  value: Value;
   consoleLogs: string[];
   runtime?: number;
   isProgram?: boolean;
@@ -160,7 +160,6 @@ type LanguageFeatures = Partial<{
 const variantDisplay: Map<Variant, string> = new Map([
   [Variant.TYPED, 'Typed'],
   [Variant.WASM, 'WebAssembly'],
-  [Variant.CONCURRENT, 'Concurrent'],
   [Variant.NATIVE, 'Native'],
   [Variant.EXPLICIT_CONTROL, 'Explicit-Control']
 ]);
@@ -266,7 +265,6 @@ const sourceSubLanguages: Array<Pick<SALanguage, 'chapter' | 'variant'>> = [
 
   { chapter: Chapter.SOURCE_3, variant: Variant.DEFAULT },
   { chapter: Chapter.SOURCE_3, variant: Variant.TYPED },
-  { chapter: Chapter.SOURCE_3, variant: Variant.CONCURRENT },
   { chapter: Chapter.SOURCE_3, variant: Variant.NATIVE },
 
   { chapter: Chapter.SOURCE_4, variant: Variant.DEFAULT },
@@ -288,13 +286,13 @@ export const sourceLanguages: SALanguage[] = sourceSubLanguages.map(sublang => {
     (variant === Variant.DEFAULT || variant === Variant.NATIVE || variant === Variant.TYPED);
 
   // Enable CSE Machine for Source Chapter 3 and above
-  supportedFeatures.cseMachine = chapter >= Chapter.SOURCE_3 && variant !== Variant.CONCURRENT;
+  supportedFeatures.cseMachine = chapter >= Chapter.SOURCE_3;
 
   // Local imports/exports require Source 2+ as Source 1 does not have lists.
   supportedFeatures.multiFile = chapter >= Chapter.SOURCE_2;
 
   // Disable REPL for concurrent variants
-  supportedFeatures.repl = variant !== Variant.CONCURRENT;
+  supportedFeatures.repl = true;
 
   return {
     ...sublang,

src/commons/application/__tests__/ApplicationTypes.ts

@@ -55,11 +55,6 @@ describe('available Source language configurations', () => {
         variant: Variant.TYPED,
         supports: { dataVisualizer: true, cseMachine: true }
       },
-      {
-        chapter: Chapter.SOURCE_3,
-        variant: Variant.CONCURRENT,
-        supports: { dataVisualizer: true }
-      },
       {
         chapter: Chapter.SOURCE_3,
         variant: Variant.NATIVE,

src/commons/application/__tests__/__snapshots__/ApplicationTypes.ts.snap

@@ -188,19 +188,6 @@ Array [
     },
     "variant": "typed",
   },
-  Object {
-    "chapter": 3,
-    "displayName": "Source §3 Concurrent",
-    "mainLanguage": "JavaScript",
-    "supports": Object {
-      "cseMachine": false,
-      "dataVisualizer": true,
-      "multiFile": true,
-      "repl": false,
-      "substVisualizer": false,
-    },
-    "variant": "concurrent",
-  },
   Object {
     "chapter": 3,
     "displayName": "Source §3 Native",

src/commons/application/actions/SessionActions.ts

@@ -4,27 +4,27 @@ import {
   unpublishedToBackendParams
 } from 'src/features/grading/GradingUtils';
 import { freshSortState } from 'src/pages/academy/grading/subcomponents/GradingSubmissionsTable';
-import { OptionType } from 'src/pages/academy/teamFormation/subcomponents/TeamFormationForm';
+import type { OptionType } from 'src/pages/academy/teamFormation/subcomponents/TeamFormationForm';
 
-import {
+import type {
   AllColsSortStates,
   GradingOverviews,
   GradingQuery
 } from '../../../features/grading/GradingTypes';
-import { TeamFormationOverview } from '../../../features/teamFormation/TeamFormationTypes';
-import {
+import type { TeamFormationOverview } from '../../../features/teamFormation/TeamFormationTypes';
+import type {
   Assessment,
   AssessmentConfiguration,
   AssessmentOverview,
   ContestEntry
 } from '../../assessment/AssessmentTypes';
-import {
+import type {
   Notification,
   NotificationFilterFunction
 } from '../../notificationBadge/NotificationBadgeTypes';
 import { generateOctokitInstance } from '../../utils/GitHubPersistenceHelper';
 import { Role, StoriesRole } from '../ApplicationTypes';
-import {
+import type {
   AdminPanelCourseRegistration,
   CourseRegistration,
   Tokens,
@@ -153,6 +153,4 @@ const SessionActions = createActions('session', {
 });
 
 // For compatibility with existing code (actions helper)
-export default {
-  ...SessionActions
-};
+export default SessionActions;

src/commons/mocks/ContextMocks.ts

@@ -1,9 +1,8 @@
 import { parse } from 'acorn';
-import { FunctionExpression, Node } from 'estree';
+import type { Node } from 'estree';
 import { ACORN_PARSE_OPTIONS } from 'js-slang/dist/constants';
 import createContext, { EnvTree } from 'js-slang/dist/createContext';
-import Closure from 'js-slang/dist/interpreter/closure';
-import { Context, Environment } from 'js-slang/dist/types';
+import type { Context } from 'js-slang/dist/types';
 import { TypeError } from 'js-slang/dist/utils/rttc';
 
 export function mockContext(chapter = 1): Context {
@@ -44,10 +43,6 @@ export function mockRuntimeContext(): Context {
   return context;
 }
 
-export function mockClosure(): Closure {
-  return new Closure({} as FunctionExpression, {} as Environment, {} as Context);
-}
-
 export function mockTypeError(): TypeError {
   // Typecast to Node to fix estree-acorn compatability.
   return new TypeError(parse('', ACORN_PARSE_OPTIONS) as Node, '', '', '');

src/commons/redux/__tests__/utils.ts

@@ -0,0 +1,69 @@
+import { testSaga } from 'redux-saga-test-plan';
+import WorkspaceActions from 'src/commons/workspace/WorkspaceActions';
+
+import { combineSagaHandlers, createActions } from '../utils';
+
+// Would have used spyOn, but for some reason that doesn't work properly
+jest.mock('src/commons/sagas/SafeEffects', () => ({
+  ...jest.requireActual('src/commons/sagas/SafeEffects'),
+  // Mock wrap saga to just be a passthrough so that the identity
+  // checking that testSaga uses will pass
+  wrapSaga: (x: any) => x
+}));
+
+test('test combineSagaHandlers', () => {
+  const mockTakeEveryHandler = jest.fn();
+  const mockTakeLatestHandler = jest.fn();
+  const mockTakeLeadingHandler = jest.fn();
+
+  const saga = combineSagaHandlers({
+    [WorkspaceActions.toggleUsingUpload.type]: mockTakeEveryHandler,
+    [WorkspaceActions.toggleFolderMode.type]: {
+      takeEvery: mockTakeEveryHandler
+    },
+    [WorkspaceActions.toggleUsingCse.type]: {
+      takeLatest: mockTakeLatestHandler
+    },
+    [WorkspaceActions.toggleUsingSubst.type]: {
+      takeLeading: mockTakeLeadingHandler
+    },
+    [WorkspaceActions.toggleEditorAutorun.type]: {
+      takeEvery: mockTakeEveryHandler,
+      takeLeading: mockTakeLeadingHandler
+    }
+  });
+
+  testSaga(saga)
+    .next()
+    .takeEvery(WorkspaceActions.toggleUsingUpload.type, mockTakeEveryHandler)
+    .next()
+    .takeEvery(WorkspaceActions.toggleFolderMode.type, mockTakeEveryHandler)
+    .next()
+    .takeLatest(WorkspaceActions.toggleUsingCse.type, mockTakeLatestHandler)
+    .next()
+    .takeLeading(WorkspaceActions.toggleUsingSubst.type, mockTakeLeadingHandler)
+    .next()
+    .takeEvery(WorkspaceActions.toggleEditorAutorun.type, mockTakeEveryHandler)
+    .next()
+    .takeLeading(WorkspaceActions.toggleEditorAutorun.type, mockTakeLeadingHandler)
+    .next()
+    .isDone();
+});
+
+test('createActions', () => {
+  const actions = createActions('workspace', {
+    act0: false,
+    act1: (value: string) => ({ value }),
+    act2: 525600
+  });
+
+  const act0 = actions.act0();
+  expect(act0.type).toEqual('workspace/act0');
+
+  const act1 = actions.act1('test');
+  expect(act1.type).toEqual('workspace/act1');
+  expect(act1.payload).toMatchObject({ value: 'test' });
+
+  const act2 = actions.act2();
+  expect(act2.type).toEqual('workspace/act2');
+});

src/commons/redux/utils.ts

@@ -1,17 +1,20 @@
 import {
-  ActionCreatorWithOptionalPayload,
-  ActionCreatorWithoutPayload,
-  ActionCreatorWithPreparedPayload,
+  type ActionCreatorWithOptionalPayload,
+  type ActionCreatorWithoutPayload,
+  type ActionCreatorWithPreparedPayload,
   createAction
 } from '@reduxjs/toolkit';
-import * as Sentry from '@sentry/browser';
-import { SagaIterator } from 'redux-saga';
-import { StrictEffect, takeEvery } from 'redux-saga/effects';
+import type { SagaIterator } from 'redux-saga';
+import { type StrictEffect, takeEvery, takeLatest, takeLeading } from 'redux-saga/effects';
+
+import { safeTakeEvery, wrapSaga } from '../sagas/SafeEffects';
+import type { SourceActionType } from '../utils/ActionsHelper';
+import { type ActionTypeToCreator, objectEntries } from '../utils/TypeHelper';
 
 /**
  * Creates actions, given a base name and base actions
  * @param baseName The base name of the actions
- * @param baseActions The base actions. Use a falsy value to create an action without a payload.
+ * @param baseActions The base actions. Use a non function value to create an action without a payload.
  * @returns An object containing the actions
  */
 export function createActions<BaseName extends string, BaseActions extends Record<string, any>>(
@@ -21,11 +24,12 @@ export function createActions<BaseName extends string, BaseActions extends Recor
   return Object.entries(baseActions).reduce(
     (res, [name, func]) => ({
       ...res,
-      [name]: func
-        ? createAction(`${baseName}/${name}`, (...args: any) => ({ payload: func(...args) }))
-        : createAction(`${baseName}/${name}`)
+      [name]:
+        typeof func === 'function'
+          ? createAction(`${baseName}/${name}`, (...args: any) => ({ payload: func(...args) }))
+          : createAction(`${baseName}/${name}`)
     }),
-    {} as {
+    {} as Readonly<{
       [K in keyof BaseActions]: K extends string
         ? BaseActions[K] extends (...args: any) => any
           ? ActionCreatorWithPreparedPayload<
@@ -35,35 +39,40 @@ export function createActions<BaseName extends string, BaseActions extends Recor
             >
           : ActionCreatorWithoutPayload<`${BaseName}/${K}`>
         : never;
-    }
+    }>
   );
 }
 
-export function combineSagaHandlers<
-  TActions extends Record<
-    string,
-    ActionCreatorWithPreparedPayload<any, any> | ActionCreatorWithoutPayload<any>
-  >
->(
-  actions: TActions,
-  handlers: {
-    // TODO: Maybe this can be stricter? And remove the optional type after
-    // migration is fully done
-    [K in keyof TActions]?: (action: ReturnType<TActions[K]>) => SagaIterator;
-  },
-  others?: (takeEvery: typeof saferTakeEvery) => SagaIterator
-): () => SagaIterator {
-  const sagaHandlers = Object.entries(handlers).map(([actionName, saga]) =>
-    saferTakeEvery(actions[actionName], saga)
-  );
+type SagaHandler<T extends SourceActionType['type']> = (
+  action: ReturnType<ActionTypeToCreator<T>>
+) => Generator<StrictEffect>;
+
+type SagaHandlers = {
+  [K in SourceActionType['type']]?:
+    | SagaHandler<K>
+    | Partial<Record<'takeEvery' | 'takeLatest' | 'takeLeading', SagaHandler<K>>>;
+};
+
+export function combineSagaHandlers(handlers: SagaHandlers) {
   return function* (): SagaIterator {
-    yield* sagaHandlers;
-    if (others) {
-      const obj = others(saferTakeEvery);
-      while (true) {
-        const { done, value } = obj.next();
-        if (done) break;
-        yield value;
+    for (const [actionName, saga] of objectEntries(handlers)) {
+      if (saga === undefined) {
+        continue;
+      } else if (typeof saga === 'function') {
+        yield takeEvery(actionName, wrapSaga(saga));
+        continue;
+      }
+
+      if (saga.takeEvery) {
+        yield takeEvery(actionName, wrapSaga(saga.takeEvery));
+      }
+
+      if (saga.takeLeading) {
+        yield takeLeading(actionName, wrapSaga(saga.takeLeading));
+      }
+
+      if (saga.takeLatest) {
+        yield takeLatest(actionName, wrapSaga(saga.takeLatest));
       }
     }
   };
@@ -74,26 +83,5 @@ export function saferTakeEvery<
     | ActionCreatorWithOptionalPayload<any>
     | ActionCreatorWithPreparedPayload<any[], any>
 >(actionPattern: Action, fn: (action: ReturnType<Action>) => Generator<StrictEffect<any>>) {
-  function* wrapper(action: ReturnType<Action>) {
-    try {
-      yield* fn(action);
-    } catch (error) {
-      handleUncaughtError(error);
-    }
-  }
-
-  return takeEvery(actionPattern.type, wrapper);
-}
-
-function handleUncaughtError(error: any) {
-  if (process.env.NODE_ENV === 'development') {
-    // react-error-overlay is a "special" package that's automatically included
-    // in development mode by CRA
-
-    // eslint-disable-next-line @typescript-eslint/ban-ts-comment
-    // @ts-ignore
-    import('react-error-overlay').then(reo => reo.reportRuntimeError(error));
-  }
-  Sentry.captureException(error);
-  console.error(error);
+  return safeTakeEvery(actionPattern.type, fn);
 }