kill unwrapped_remote (#1490)

* kill unwrapped_remote

* linting

* don't use callKwargs for python plugins

* fix tests and improve types

pyscriptjs/src/interpreter_client.ts

@@ -13,7 +13,6 @@ InterpreterClient class is responsible to request code execution
 */
 export class InterpreterClient extends Object {
     _remote: Synclink.Remote<RemoteInterpreter>;
-    _unwrapped_remote: RemoteInterpreter;
     config: AppConfig;
     /**
      * global symbols table for the underlying interface.
@@ -21,16 +20,10 @@ export class InterpreterClient extends Object {
     globals: Synclink.Remote<PyProxyDict>;
     stdio: Stdio;
 
-    constructor(
+    constructor(config: AppConfig, stdio: Stdio, remote: Synclink.Remote<RemoteInterpreter>) {
-        config: AppConfig,
-        stdio: Stdio,
-        remote: Synclink.Remote<RemoteInterpreter>,
-        unwrapped_remote: RemoteInterpreter,
-    ) {
         super();
         this.config = config;
         this._remote = remote;
-        this._unwrapped_remote = unwrapped_remote;
         this.stdio = stdio;
     }
 

pyscriptjs/src/main.ts

@@ -212,7 +212,7 @@ export class PyScriptApp {
         */
         const interpreterURL = interpreter_cfg.src;
         await import(interpreterURL);
-        return { remote_interpreter, wrapped_remote_interpreter };
+        return wrapped_remote_interpreter;
     }
 
     async _startInterpreter_worker(interpreter_cfg: InterpreterConfig) {
@@ -222,8 +222,7 @@ export class PyScriptApp {
         const worker = new Worker(base_url + '/interpreter_worker.js');
         const worker_initialize: any = Synclink.wrap(worker);
         const wrapped_remote_interpreter = await worker_initialize(interpreter_cfg);
-        const remote_interpreter = undefined; // this is _unwrapped_remote
+        return wrapped_remote_interpreter;
-        return { remote_interpreter, wrapped_remote_interpreter };
     }
 
     // lifecycle (4)
@@ -238,19 +237,17 @@ export class PyScriptApp {
         }
 
         const cfg = this.config.interpreters[0];
-        let x;
+        let wrapped_remote_interpreter;
         if (this.config.execution_thread == 'worker') {
-            x = await this._startInterpreter_worker(cfg);
+            wrapped_remote_interpreter = await this._startInterpreter_worker(cfg);
         } else {
-            x = await this._startInterpreter_main(cfg);
+            wrapped_remote_interpreter = await this._startInterpreter_main(cfg);
         }
-        const { remote_interpreter, wrapped_remote_interpreter } = x;
 
         this.interpreter = new InterpreterClient(
             this.config,
             this._stdioMultiplexer,
             wrapped_remote_interpreter as Synclink.Remote<RemoteInterpreter>,
-            remote_interpreter,
         );
         await this.afterInterpreterLoad(this.interpreter);
     }
@@ -413,11 +410,9 @@ export class PyScriptApp {
         // TODO: This is very specific to Pyodide API and will not work for other interpreters,
         //       when we add support for other interpreters we will need to move this to the
         //       interpreter API level and allow each one to implement it in its own way
-
+        const module = await interpreter.pyimport(modulename);
-        // eventually replace with interpreter.pyimport(modulename);
-        const module = interpreter._unwrapped_remote.pyimport(modulename);
         if (typeof (await module.plugin) !== 'undefined') {
-            const py_plugin = module.plugin as PythonPlugin;
+            const py_plugin = (await module.plugin) as PythonPlugin;
             py_plugin.init(this);
             this.plugins.addPythonPlugin(py_plugin);
         } else {

pyscriptjs/src/plugin.ts

@@ -4,9 +4,11 @@ import { UserError, ErrorCode } from './exceptions';
 import { getLogger } from './logger';
 import { make_PyScript } from './components/pyscript';
 import { InterpreterClient } from './interpreter_client';
+import { make_PyRepl } from './components/pyrepl';
 
 const logger = getLogger('plugin');
 type PyScriptTag = InstanceType<ReturnType<typeof make_PyScript>>;
+type PyReplTag = InstanceType<ReturnType<typeof make_PyRepl>>;
 
 export class Plugin {
     /** Validate the configuration of the plugin and handle default values.
@@ -84,7 +86,12 @@ export class Plugin {
      * @param options.outEl The element that the result of the REPL evaluation will be output to.
      * @param options.pyReplTag The <py-repl> HTML tag the originated the evaluation
      */
-    beforePyReplExec(options: { interpreter: InterpreterClient; src: string; outEl: HTMLElement; pyReplTag: any }) {
+    beforePyReplExec(options: {
+        interpreter: InterpreterClient;
+        src: string;
+        outEl: HTMLElement;
+        pyReplTag: PyReplTag;
+    }) {
         /* empty */
     }
 
@@ -100,7 +107,7 @@ export class Plugin {
         interpreter: InterpreterClient;
         src: string;
         outEl: HTMLElement;
-        pyReplTag: HTMLElement;
+        pyReplTag: PyReplTag;
         result: any;
     }) {
         /* empty */
@@ -126,10 +133,26 @@ export type PythonPlugin = {
     configure?: (config: AppConfig) => Promise<void>;
     afterSetup?: (interpreter: InterpreterClient) => Promise<void>;
     afterStartup?: (interpreter: InterpreterClient) => Promise<void>;
-    beforePyScriptExec?: { callKwargs: (options: any) => Promise<void> };
+    beforePyScriptExec?: (interpreter: InterpreterClient, src: string, pyScriptTag: PyScriptTag) => Promise<void>;
-    afterPyScriptExec?: { callKwargs: (options: any) => Promise<void> };
+    afterPyScriptExec?: (
-    beforePyReplExec?: { callKwargs: (options: any) => Promise<void> };
+        interpreter: InterpreterClient,
-    afterPyReplExec?: { callKwargs: (options: any) => Promise<void> };
+        src: string,
+        pyScriptTag: PyScriptTag,
+        result: any,
+    ) => Promise<void>;
+    beforePyReplExec?: (
+        interpreter: InterpreterClient,
+        src: string,
+        outEl: HTMLElement,
+        pyReplTag: PyReplTag,
+    ) => Promise<void>;
+    afterPyReplExec?: (
+        interpreter: InterpreterClient,
+        src: string,
+        outEl: HTMLElement,
+        pyReplTag: PyReplTag,
+        result: any,
+    ) => Promise<void>;
     onUserError?: (error: UserError) => Promise<void>;
 };
 
@@ -188,7 +211,9 @@ export class PluginManager {
 
     async beforePyScriptExec(options: { interpreter: InterpreterClient; src: string; pyScriptTag: PyScriptTag }) {
         await Promise.all(this._plugins.map(p => p.beforePyScriptExec?.(options)));
-        await Promise.all(this._pythonPlugins.map(p => p.beforePyScriptExec.callKwargs(options)));
+        await Promise.all(
+            this._pythonPlugins.map(p => p.beforePyScriptExec?.(options.interpreter, options.src, options.pyScriptTag)),
+        );
     }
 
     async afterPyScriptExec(options: {
@@ -198,22 +223,47 @@ export class PluginManager {
         result: any;
     }) {
         await Promise.all(this._plugins.map(p => p.afterPyScriptExec?.(options)));
-        await Promise.all(this._pythonPlugins.map(p => p.afterPyScriptExec.callKwargs(options)));
+        await Promise.all(
+            this._pythonPlugins.map(
+                p => p.afterPyScriptExec?.(options.interpreter, options.src, options.pyScriptTag, options.result),
+            ),
+        );
     }
 
     async beforePyReplExec(options: {
         interpreter: InterpreterClient;
         src: string;
         outEl: HTMLElement;
-        pyReplTag: any;
+        pyReplTag: PyReplTag;
     }) {
         await Promise.all(this._plugins.map(p => p.beforePyReplExec?.(options)));
-        await Promise.all(this._pythonPlugins.map(p => p.beforePyReplExec.callKwargs(options)));
+        await Promise.all(
+            this._pythonPlugins.map(
+                p => p.beforePyReplExec?.(options.interpreter, options.src, options.outEl, options.pyReplTag),
+            ),
+        );
     }
 
-    async afterPyReplExec(options: { interpreter: InterpreterClient; src: string; outEl; pyReplTag; result }) {
+    async afterPyReplExec(options: {
+        interpreter: InterpreterClient;
+        src: string;
+        outEl: HTMLElement;
+        pyReplTag: PyReplTag;
+        result: any;
+    }) {
         await Promise.all(this._plugins.map(p => p.afterPyReplExec?.(options)));
-        await Promise.all(this._pythonPlugins.map(p => p.afterPyReplExec.callKwargs(options)));
+        await Promise.all(
+            this._pythonPlugins.map(
+                p =>
+                    p.afterPyReplExec?.(
+                        options.interpreter,
+                        options.src,
+                        options.outEl,
+                        options.pyReplTag,
+                        options.result,
+                    ),
+            ),
+        );
     }
 
     async onUserError(error: UserError) {

pyscriptjs/tests/integration/test_plugins.py

@@ -63,12 +63,10 @@ class ExecTestLogger(Plugin):
     async def beforePyScriptExec(self, interpreter, src, pyScriptTag):
         console.log(f'beforePyScriptExec called')
         console.log(f'before_src:{src}')
-        console.log(f'before_id:{pyScriptTag.id}')
 
     async def afterPyScriptExec(self, interpreter, src, pyScriptTag, result):
         console.log(f'afterPyScriptExec called')
         console.log(f'after_src:{src}')
-        console.log(f'after_id:{pyScriptTag.id}')
         console.log(f'result:{result}')
 
 
@@ -88,12 +86,10 @@ class PyReplTestLogger(Plugin):
     def beforePyReplExec(self, interpreter, src, outEl, pyReplTag):
         console.log(f'beforePyReplExec called')
         console.log(f'before_src:{src}')
-        console.log(f'before_id:{pyReplTag.id}')
 
     def afterPyReplExec(self, interpreter, src, outEl, pyReplTag, result):
         console.log(f'afterPyReplExec called')
         console.log(f'after_src:{src}')
-        console.log(f'after_id:{pyReplTag.id}')
         console.log(f'result:{result}')
 
 
@@ -261,9 +257,7 @@ class TestPlugin(PyScriptTest):
         # These could be made better with a utility function that found log lines
         # that match a filter function, or start with something
         assert "before_src:x=2; x" in log_lines
-        assert "before_id:pyid" in log_lines
         assert "after_src:x=2; x" in log_lines
-        assert "after_id:pyid" in log_lines
         assert "result:2" in log_lines
 
     @skip_worker("FIXME: relative paths")
@@ -286,9 +280,7 @@ class TestPlugin(PyScriptTest):
         # These could be made better with a utility function that found log lines
         # that match a filter function, or start with something
         assert "before_src:x=2; x" in log_lines
-        assert "before_id:pyid" in log_lines
         assert "after_src:x=2; x" in log_lines
-        assert "after_id:pyid" in log_lines
         assert "result:2" in log_lines
 
     @skip_worker("FIXME: relative paths")

pyscriptjs/tests/unit/pyodide.test.ts

@@ -29,7 +29,6 @@ describe('RemoteInterpreter', () => {
             config,
             stdio,
             wrapped_remote_interpreter as Synclink.Remote<RemoteInterpreter>,
-            remote_interpreter,
         );
 
         /**
@@ -69,10 +68,6 @@ describe('RemoteInterpreter', () => {
         expect(interpreter).toBeInstanceOf(InterpreterClient);
     });
 
-    it('should check if interpreter is an instance of RemoteInterpreter', async () => {
-        expect(interpreter._unwrapped_remote).toBeInstanceOf(RemoteInterpreter);
-    });
-
     it('should check if interpreter can run python code asynchronously', async () => {
         expect((await interpreter.run('2+3')).result).toBe(5);
     });
@@ -85,7 +80,7 @@ describe('RemoteInterpreter', () => {
 
     it('should check if interpreter is able to load a package', async () => {
         stdio.reset();
-        await interpreter._unwrapped_remote.loadPackage('numpy');
+        await interpreter._remote.loadPackage('numpy');
         await interpreter.run('import numpy as np');
         await interpreter.run('x = np.ones((10,))');
         await interpreter.run('print(x)');