feat: handle python input synchronously (#52526)

Co-authored-by: Shaun Hamilton <shauhami020@gmail.com>

tools/client-plugins/browser-scripts/index.d.ts

@@ -22,7 +22,6 @@ export interface InitTestFrameArg {
   };
   getUserInput?: (fileName: string) => string;
   loadEnzyme?: () => void;
-  transformedPython?: string;
 }
 
 export type FrameWindow = Window &

tools/client-plugins/browser-scripts/python-runner.ts

@@ -1,299 +0,0 @@
-// We have to specify pyodide.js because we need to import that file (not .mjs)
-// and 'import' defaults to .mjs
-import { loadPyodide, type PyodideInterface } from 'pyodide/pyodide.js';
-import pkg from 'pyodide/package.json';
-import { IDisposable, Terminal } from 'xterm';
-import { FitAddon } from 'xterm-addon-fit';
-import jQuery from 'jquery'; // TODO: is jQuery needed for the python runner?
-import * as helpers from '@freecodecamp/curriculum-helpers';
-
-import type { PythonDocument, FrameWindow, InitTestFrameArg } from '.';
-
-import 'xterm/css/xterm.css';
-
-(window as FrameWindow).$ = jQuery;
-
-// This will be running in an iframe, so document will be
-// element.contentDocument. This declaration is just to add properties we know
-// exist on this document (but not on the parent)
-const contentDocument = document as PythonDocument;
-
-function createTerminal(disposables: IDisposable[]) {
-  const terminalContainer = document.getElementById('terminal');
-  if (!terminalContainer) throw Error('Could not find terminal container');
-
-  // Setting convertEol so that \n is converted to \r\n. Otherwise the terminal
-  // will interpret \n as line feed and just move the cursor to the next line.
-  // convertEol makes every \n a \r\n.
-  const term = new Terminal({ convertEol: true });
-  const fitAddon = new FitAddon();
-  term.loadAddon(fitAddon);
-  term.open(terminalContainer);
-  fitAddon.fit();
-
-  const resetTerminal = () => {
-    term.reset();
-    disposables.forEach(disposable => disposable.dispose());
-    disposables.length = 0;
-  };
-
-  return { term, resetTerminal };
-}
-
-async function setupPyodide() {
-  // I tried setting jsglobals here, to provide 'input' and 'print' to python,
-  // without having to modify the global window object. However, it didn't work
-  // because pyodide needs access to that object. Instead, I used
-  // registerJsModule when setting up runPython.
-  return await loadPyodide({
-    indexURL: `https://cdn.jsdelivr.net/pyodide/v${pkg.version}/full/`
-  });
-}
-
-type Input = (text: string) => Promise<string>;
-type Print = (...args: unknown[]) => void;
-type ResetTerminal = () => void;
-type EvaluatedTeststring = {
-  input: string[];
-  test: () => Promise<unknown>;
-};
-
-function createJSFunctionsForPython(
-  term: Terminal,
-  disposables: IDisposable[],
-  pyodide: PyodideInterface
-) {
-  const writeLine = (text: string) => term.writeln(`>>> ${text}`);
-
-  // eslint-disable-next-line @typescript-eslint/no-unsafe-call
-  const str = pyodide.globals.get('str') as (x: unknown) => string;
-  function print(...args: unknown[]) {
-    const text = args.map(x => str(x)).join(' ');
-    writeLine(text);
-  }
-  // TODO: prevent user from moving cursor outside the current input line and
-  // handle insertion and deletion properly. While backspace and delete don't
-  // seem to work, we can use "\x1b[0K" to clear from the cursor to the end.
-  // Also, we should not add special characters to the userinput string.
-  const waitForInput = (): Promise<string> =>
-    new Promise(resolve => {
-      let userinput = '';
-      // Eslint is correct that this only gets assigned once, but we can't use
-      // const because the declaration (before keyListener is defined) and
-      // assignment (after keyListener is defined) must be separate.
-      // eslint-disable-next-line prefer-const
-      let disposable: IDisposable | undefined;
-
-      const done = () => {
-        disposable?.dispose();
-        resolve(userinput);
-      };
-
-      const keyListener = (key: string) => {
-        if (key === '\u007F' || key === '\b') {
-          // Backspace or delete key
-          term.write('\b \b'); // Move cursor back, replace character with space, then move cursor back again
-          userinput = userinput.slice(0, -1); // Remove the last character from userinput
-        }
-        if (key == '\r') {
-          term.write('\r\n');
-          done();
-        } else {
-          userinput += key;
-          term.write(key);
-        }
-      };
-
-      disposable = term.onData(keyListener); // Listen for key events and store the disposable
-      disposables.push(disposable);
-    });
-
-  const input = async (text: string) => {
-    writeLine(text);
-    return await waitForInput();
-  };
-
-  return { print, input };
-}
-
-function setupRunPython(
-  pyodide: PyodideInterface,
-  {
-    input,
-    print,
-    resetTerminal
-  }: { input: Input; print: Print; resetTerminal: ResetTerminal }
-) {
-  // Make print and input available to python
-  pyodide.registerJsModule('jscustom', {
-    input,
-    print
-  });
-  pyodide.runPython(`
-  import jscustom
-  from jscustom import print
-  from jscustom import input
-  `);
-
-  async function runPython(code: string) {
-    // eslint-disable-next-line @typescript-eslint/no-unsafe-call
-    pyodide.globals.get('__cancel')?.();
-    resetTerminal();
-
-    // There's no need to clear out globals between runs, because the user's
-    // code is always run in a coroutine and shouldn't pollute them. If we
-    // subsequently want to run code that does interact with globals, we can
-    // revisit this.
-    await pyodide.runPythonAsync(code);
-    return pyodide;
-  }
-
-  contentDocument.__runPython = runPython;
-}
-
-async function initPythonFrame() {
-  const disposables: IDisposable[] = [];
-  const { term, resetTerminal } = createTerminal(disposables);
-  const pyodide = await setupPyodide();
-  const { print, input } = createJSFunctionsForPython(
-    term,
-    disposables,
-    pyodide
-  );
-  setupRunPython(pyodide, { input, print, resetTerminal });
-}
-
-contentDocument.__initPythonFrame = initPythonFrame;
-contentDocument.__initTestFrame = initTestFrame;
-
-// TODO: DRY this and frame-runner.ts's initTestFrame
-async function initTestFrame(e: InitTestFrameArg) {
-  const pyodide = await setupPyodide();
-
-  // transformedPython is used here not because it's necessary (it's not since
-  // the transformation converts `input` into `await input` and the tests
-  // provide a synchronous `input` function), but because we want to run the
-  // tests against exactly the same code that runs in the preview.
-  const code = (e.transformedPython || '').slice();
-  const __file = (id?: string) => {
-    if (id && e.code.original) {
-      return e.code.original[id];
-    } else {
-      return code;
-    }
-  };
-
-  if (!e.getUserInput) {
-    e.getUserInput = () => code;
-  }
-
-  /* eslint-disable @typescript-eslint/no-unused-vars */
-  // Fake Deep Equal dependency
-  const DeepEqual = (a: Record<string, unknown>, b: Record<string, unknown>) =>
-    JSON.stringify(a) === JSON.stringify(b);
-
-  // Hardcode Deep Freeze dependency
-  // eslint-disable-next-line @typescript-eslint/no-explicit-any
-  const DeepFreeze = (o: Record<string, any>) => {
-    Object.freeze(o);
-    Object.getOwnPropertyNames(o).forEach(function (prop) {
-      if (
-        Object.prototype.hasOwnProperty.call(o, prop) &&
-        o[prop] !== null &&
-        (typeof o[prop] === 'object' || typeof o[prop] === 'function') &&
-        !Object.isFrozen(o[prop])
-      ) {
-        // eslint-disable-next-line @typescript-eslint/no-unsafe-argument
-        DeepFreeze(o[prop]);
-      }
-    });
-    return o;
-  };
-
-  const { default: chai } = await import(/* webpackChunkName: "chai" */ 'chai');
-  const assert = chai.assert;
-  const __helpers = helpers;
-  /* eslint-enable @typescript-eslint/no-unused-vars */
-
-  contentDocument.__runTest = async function runTests(testString: string) {
-    // uncomment the following line to inspect
-    // the frame-runner as it runs tests
-    // make sure the dev tools console is open
-    // debugger;
-    try {
-      // eval test string to get the dummy input and actual test
-      const evaluatedTestString = await new Promise<unknown>(
-        (resolve, reject) =>
-          // To avoid race conditions, we have to run the test in a final
-          // frameDocument ready:
-          $(() => {
-            try {
-              // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
-              const test: { input: string[]; test: () => Promise<unknown> } =
-                eval(testString);
-              resolve(test);
-            } catch (err) {
-              reject(err);
-            }
-          })
-      );
-
-      // If the test string does not evaluate to an object, then we assume that
-      // it's a standard JS test and any assertions have already passed.
-      if (typeof evaluatedTestString !== 'object') {
-        return { pass: true };
-      }
-
-      if (!evaluatedTestString || !('test' in evaluatedTestString)) {
-        throw new Error(
-          'Test string did not evaluate to an object with the test property'
-        );
-      }
-
-      const { input, test } = evaluatedTestString as EvaluatedTeststring;
-
-      // TODO: throw helpful error if we run out of input values, since it's likely
-      // that the user added too many input statements.
-      const inputIterator = input ? input.values() : null;
-      setupRunPython(pyodide, {
-        input: () => {
-          return Promise.resolve(
-            inputIterator ? inputIterator.next().value : ''
-          );
-        },
-        // We don't, currently, care what print is called with, but it does need
-        // to exist
-        print: () => void 0,
-        // resetTerminal is only necessary when calling __runPython more than
-        // once, which we don't do in the test frame
-        resetTerminal: () => void 0
-      });
-
-      // We have to declare these variables in the scope of 'eval', so that they
-      // exist when the `testString` is evaluated. Otherwise, they will be
-      // undefined when `test` is called and the tests will not be able to use
-      // __pyodide or __userGlobals.
-      const __pyodide = await this.__runPython(code);
-      // eslint-disable-next-line @typescript-eslint/no-unsafe-call
-      const __userGlobals = __pyodide.globals.get('__locals') as unknown;
-      await test();
-
-      return { pass: true };
-    } catch (err) {
-      if (!(err instanceof chai.AssertionError)) {
-        console.error(err);
-      }
-      // to provide useful debugging information when debugging the tests, we
-      // have to extract the message, stack and, if they exist, expected and
-      // actual before returning
-      return {
-        err: {
-          message: (err as Error).message,
-          stack: (err as Error).stack,
-          expected: (err as { expected?: string }).expected,
-          actual: (err as { actual?: string }).actual
-        }
-      };
-    }
-  };
-}

tools/client-plugins/browser-scripts/python-test-evaluator.ts

@@ -0,0 +1,166 @@
+// We have to specify pyodide.js because we need to import that file (not .mjs)
+// and 'import' defaults to .mjs
+import { loadPyodide, type PyodideInterface } from 'pyodide/pyodide.js';
+import type { PyProxy } from 'pyodide/ffi';
+import pkg from 'pyodide/package.json';
+import * as helpers from '@freecodecamp/curriculum-helpers';
+import chai from 'chai';
+
+const ctx: Worker & typeof globalThis = self as unknown as Worker &
+  typeof globalThis;
+
+let pyodide: PyodideInterface;
+
+interface PythonRunEvent extends MessageEvent {
+  data: {
+    code: {
+      contents: string;
+      editableContents: string;
+      original: { [id: string]: string };
+    };
+    removeComments: boolean;
+    firstTest: unknown;
+    testString: string;
+    build: string;
+    sources: {
+      [fileName: string]: unknown;
+    };
+  };
+}
+
+type EvaluatedTeststring = {
+  input: string[];
+  test: () => Promise<unknown>;
+};
+
+async function setupPyodide() {
+  if (pyodide) return pyodide;
+
+  pyodide = await loadPyodide({
+    // TODO: host this ourselves
+    indexURL: `https://cdn.jsdelivr.net/pyodide/v${pkg.version}/full/`
+  });
+
+  // We freeze this to prevent learners from getting the worker into a
+  // weird state. NOTE: this has to come after pyodide is loaded, because
+  // pyodide modifies self while loading.
+  Object.freeze(self);
+
+  ctx.postMessage({ type: 'contentLoaded' });
+
+  return pyodide;
+}
+
+void setupPyodide();
+
+ctx.onmessage = async (e: PythonRunEvent) => {
+  const pyodide = await setupPyodide();
+  // TODO: Use removeComments when we have it
+  /* eslint-disable @typescript-eslint/no-unused-vars */
+  const code = (e.data.code.contents || '').slice();
+  const editableContents = (e.data.code.editableContents || '').slice();
+  const testString = e.data.testString;
+
+  const assert = chai.assert;
+  const __helpers = helpers;
+  /* eslint-enable @typescript-eslint/no-unused-vars */
+  // uncomment the following line to inspect
+  // the frame-runner as it runs tests
+  // make sure the dev tools console is open
+  // debugger;
+  try {
+    // eval test string to get the dummy input and actual test
+    const evaluatedTestString = await new Promise<unknown>(
+      (resolve, reject) => {
+        try {
+          // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
+          const test: { input: string[]; test: () => Promise<unknown> } =
+            eval(testString);
+          resolve(test);
+        } catch (err) {
+          reject(err);
+        }
+      }
+    );
+
+    // If the test string does not evaluate to an object, then we assume that
+    // it's a standard JS test and any assertions have already passed.
+    if (typeof evaluatedTestString !== 'object') {
+      ctx.postMessage({ pass: true });
+      return;
+    }
+
+    if (!evaluatedTestString || !('test' in evaluatedTestString)) {
+      throw new Error(
+        'Test string did not evaluate to an object with the test property'
+      );
+    }
+
+    const { input, test } = evaluatedTestString as EvaluatedTeststring;
+
+    const inputIterator = (input ?? []).values();
+    const testInput = () => {
+      const next = inputIterator.next();
+      if (next.done) {
+        // TODO: handle this error in the UI
+        throw new Error('Too many input calls');
+      } else {
+        return next.value;
+      }
+    };
+
+    // Make input available to python (print is not used yet)
+    pyodide.registerJsModule('jscustom', {
+      input: testInput
+      // print: () => {}
+    });
+    // Create fresh globals for each test
+    // eslint-disable-next-line @typescript-eslint/no-unsafe-call
+    const __userGlobals = pyodide.globals.get('dict')() as PyProxy;
+    // Some tests rely on __name__ being set to __main__ and we new dicts do not
+    // have this set by default.
+    // eslint-disable-next-line @typescript-eslint/no-unsafe-call
+    __userGlobals.set('__name__', '__main__');
+
+    // The runPython helper is a shortcut for running python code with our
+    // custom globals.
+    const runPython = (pyCode: string) =>
+      pyodide.runPython(pyCode, { globals: __userGlobals }) as unknown;
+    // TODO: remove __pyodide once all the test use runPython.
+    const __pyodide = {
+      runPython
+    };
+
+    runPython(
+      `
+  import jscustom
+  from jscustom import input
+  `
+    );
+
+    // Evaluates the learner's code so that any variables they define are
+    // available to the test.
+    runPython(code);
+    // TODO: remove the next line, creating __locals, once all the tests access
+    // variables directly.
+    runPython('__locals = globals()');
+    await test();
+
+    ctx.postMessage({ pass: true });
+  } catch (err) {
+    if (!(err instanceof chai.AssertionError)) {
+      console.error(err);
+    }
+    // to provide useful debugging information when debugging the tests, we
+    // have to extract the message, stack and, if they exist, expected and
+    // actual before returning
+    ctx.postMessage({
+      err: {
+        message: (err as Error).message,
+        stack: (err as Error).stack,
+        expected: (err as { expected?: string }).expected,
+        actual: (err as { actual?: string }).actual
+      }
+    });
+  }
+};

tools/client-plugins/browser-scripts/python-worker.ts

@@ -0,0 +1,81 @@
+// We have to specify pyodide.js because we need to import that file (not .mjs)
+// and 'import' defaults to .mjs
+import { loadPyodide, type PyodideInterface } from 'pyodide/pyodide.js';
+import pkg from 'pyodide/package.json';
+
+const ctx: Worker & typeof globalThis = self as unknown as Worker &
+  typeof globalThis;
+
+let pyodide: PyodideInterface;
+
+interface PythonRunEvent extends MessageEvent {
+  data: {
+    code: {
+      contents: string;
+      editableContents: string;
+      original: { [id: string]: string };
+    };
+  };
+}
+
+async function setupPyodide() {
+  if (pyodide) return pyodide;
+
+  pyodide = await loadPyodide({
+    // TODO: host this ourselves
+    indexURL: `https://cdn.jsdelivr.net/pyodide/v${pkg.version}/full/`
+  });
+
+  // We freeze this to prevent learners from getting the worker into a
+  // weird state. NOTE: this has to come after pyodide is loaded, because
+  // pyodide modifies self while loading.
+  Object.freeze(self);
+
+  // eslint-disable-next-line @typescript-eslint/no-unsafe-call
+  const str = pyodide.globals.get('str') as (x: unknown) => string;
+
+  function print(...args: unknown[]) {
+    const text = args.map(x => str(x)).join(' ');
+    postMessage({ type: 'print', text });
+  }
+
+  function input(text: string) {
+    // TODO: send unique ids to the main thread and the service worker, so we
+    // can have multiple concurrent input requests.
+    postMessage({ type: 'input', text });
+    const request = new XMLHttpRequest();
+    request.open('POST', '/python/intercept-input/', false);
+    request.send(null);
+
+    return request.responseText;
+  }
+
+  // I tried setting jsglobals here, to provide 'input' and 'print' to python,
+  // without having to modify the global window object. However, it didn't work
+  // because pyodide needs access to that object. Instead, I used
+  // registerJsModule when setting up runPython.
+
+  // Make print available to python
+  pyodide.registerJsModule('jscustom', {
+    print,
+    input
+  });
+  // TODO: use a fresh global object for each runPython call if we stop terminating
+  // the worker when the user input changes. (See python-test-evaluator.ts)
+  pyodide.runPython(`
+  import jscustom
+  from jscustom import print
+  from jscustom import input
+`);
+
+  return pyodide;
+}
+
+void setupPyodide();
+
+ctx.onmessage = async (e: PythonRunEvent) => {
+  const code = (e.data.code.contents || '').slice();
+  const pyodide = await setupPyodide();
+  // use pyodide.runPythonAsync if we want top-level await
+  pyodide.runPython(code);
+};

tools/client-plugins/browser-scripts/tsconfig.json

@@ -2,6 +2,7 @@
   "compilerOptions": {
     "target": "es2022",
     "module": "CommonJS",
+    "lib": ["WebWorker", "DOM"],
     "allowJs": true,
     "strict": true,
     "forceConsistentCasingInFileNames": true,

tools/client-plugins/browser-scripts/webpack.config.js

@@ -17,7 +17,8 @@ module.exports = (env = {}) => {
       'frame-runner': './frame-runner.ts',
       'sass-compile': './sass-compile.ts',
       'test-evaluator': './test-evaluator.ts',
-      'python-runner': './python-runner.ts'
+      'python-worker': './python-worker.ts',
+      'python-test-evaluator': './python-test-evaluator.ts'
     },
     devtool: __DEV__ ? 'inline-source-map' : 'source-map',
     output: {
@@ -61,17 +62,16 @@ module.exports = (env = {}) => {
               ]
             }
           }
-        },
-        // xterm doesn't bundle its css, so we need to load it ourselves
-        {
-          test: /\.css$/,
-          use: ['style-loader', 'css-loader']
         }
       ]
     },
     plugins: [
       new CopyWebpackPlugin({
-        patterns: ['./node_modules/sass.js/dist/sass.sync.js']
+        patterns: [
+          './node_modules/sass.js/dist/sass.sync.js',
+          // TODO: copy this into the css folder, not the js folder
+          './node_modules/xterm/css/xterm.css'
+        ]
       }),
       new webpack.ProvidePlugin({
         process: 'process/browser'