From 14914210ee4d3aefd7f6c0a27602fa1b3c0af304 Mon Sep 17 00:00:00 2001 From: Luca Casonato Date: Wed, 27 Sep 2023 14:23:04 +0900 Subject: [PATCH 1/5] fix(cli): panic with __runtime_js_sources Also a drive-by cleanup elsewhere (removing unused enum). --- cli/ops/mod.rs | 1 + cli/tools/test/mod.rs | 7 ------- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/cli/ops/mod.rs b/cli/ops/mod.rs index ad4e4bd4e65b42..ab3a554687b46b 100644 --- a/cli/ops/mod.rs +++ b/cli/ops/mod.rs @@ -29,6 +29,7 @@ deno_core::extension!(cli, esm = [ dir "js", "40_testing.js", + "40_jupyter.js", "99_main.js" ], options = { diff --git a/cli/tools/test/mod.rs b/cli/tools/test/mod.rs index 550ec9f9bb8471..bf466579faa739 100644 --- a/cli/tools/test/mod.rs +++ b/cli/tools/test/mod.rs @@ -173,13 +173,6 @@ pub struct TestDescription { pub location: TestLocation, } -#[derive(Debug, Clone, Eq, PartialEq, Deserialize)] -#[serde(rename_all = "camelCase")] -pub enum TestOutput { - String(String), - Bytes(Vec), -} - #[allow(clippy::derive_partial_eq_without_eq)] #[derive(Debug, Clone, PartialEq, Deserialize)] #[serde(rename_all = "camelCase")] From 9821a0d0762c2f70ab6f4ef46d414e9547ecf9ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Wed, 27 Sep 2023 12:34:15 +0200 Subject: [PATCH 2/5] fix reassigning the namespace --- cli/js/40_jupyter.js | 4 +--- cli/tests/testdata/jupyter/integration_test.ipynb | 11 ++++++----- runtime/js/99_main.js | 7 +++++++ 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/cli/js/40_jupyter.js b/cli/js/40_jupyter.js index 8cdf0789a93e2f..7414d29d58afa2 100644 --- a/cli/js/40_jupyter.js +++ b/cli/js/40_jupyter.js @@ -3,10 +3,8 @@ const core = globalThis.Deno.core; const internals = globalThis.__bootstrap.internals; -import { denoNsUnstable } from "ext:runtime/90_deno_ns.js"; - function enableJupyter() { - denoNsUnstable.jupyter = { + globalThis.Deno.jupyter = { async broadcast(msgType, content) { await core.opAsync("op_jupyter_broadcast", msgType, content); }, diff --git a/cli/tests/testdata/jupyter/integration_test.ipynb b/cli/tests/testdata/jupyter/integration_test.ipynb index 1d48c8fc9c95a8..d92fd8fa30170f 100644 --- a/cli/tests/testdata/jupyter/integration_test.ipynb +++ b/cli/tests/testdata/jupyter/integration_test.ipynb @@ -665,18 +665,19 @@ }, { "cell_type": "code", - "execution_count": 1, + "execution_count": 2, "id": "8e93df23-06eb-414b-98d4-51fbebb53d1f", "metadata": {}, "outputs": [ { - "ename": "TypeError", - "evalue": "Cannot read properties of undefined (reading 'broadcast')", + "ename": "Error", + "evalue": "Deno.jupyter is only available in `deno jupyter` subcommand.", "output_type": "error", "traceback": [ "Stack trace:", - "TypeError: Cannot read properties of undefined (reading 'broadcast')", - " at :2:20" + "Error: Deno.jupyter is only available in `deno jupyter` subcommand.", + " at Object.get (ext:cli/runtime/js/99_main.js:550:15)", + " at :2:12" ] } ], diff --git a/runtime/js/99_main.js b/runtime/js/99_main.js index 15e4936b137de0..d2a28838a89df9 100644 --- a/runtime/js/99_main.js +++ b/runtime/js/99_main.js @@ -545,12 +545,19 @@ function bootstrapMainRuntime(runtimeOptions) { // TODO(bartlomieju): this is not ideal, but because we use `ObjectAssign` // above any properties that are defined elsewhere using `Object.defineProperty` // are lost. + let jupyterNs = undefined; ObjectDefineProperty(finalDenoNs, "jupyter", { get() { + if (jupyterNs) { + return jupyterNs; + } throw new Error( "Deno.jupyter is only available in `deno jupyter` subcommand.", ); }, + set(val) { + jupyterNs = val; + }, }); } From f89c50fe5e799d358b4622c36102b0a1c2bb031c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Wed, 27 Sep 2023 12:36:24 +0200 Subject: [PATCH 3/5] ensure fast ops, update test --- cli/js/40_jupyter.js | 6 +- .../testdata/jupyter/integration_test.ipynb | 108 +++++++++--------- 2 files changed, 62 insertions(+), 52 deletions(-) diff --git a/cli/js/40_jupyter.js b/cli/js/40_jupyter.js index 7414d29d58afa2..ae257f1c1f54fb 100644 --- a/cli/js/40_jupyter.js +++ b/cli/js/40_jupyter.js @@ -4,9 +4,13 @@ const core = globalThis.Deno.core; const internals = globalThis.__bootstrap.internals; function enableJupyter() { + const { + op_jupyter_broadcast, + } = core.ensureFastOps(); + globalThis.Deno.jupyter = { async broadcast(msgType, content) { - await core.opAsync("op_jupyter_broadcast", msgType, content); + await op_jupyter_broadcast(msgType, content); }, }; } diff --git a/cli/tests/testdata/jupyter/integration_test.ipynb b/cli/tests/testdata/jupyter/integration_test.ipynb index d92fd8fa30170f..76064cea2a07f1 100644 --- a/cli/tests/testdata/jupyter/integration_test.ipynb +++ b/cli/tests/testdata/jupyter/integration_test.ipynb @@ -44,7 +44,7 @@ }, { "cell_type": "code", - "execution_count": 21, + "execution_count": 1, "id": "a5d38758", "metadata": { "hidden": true @@ -52,7 +52,7 @@ "outputs": [ { "data": {}, - "execution_count": 21, + "execution_count": 1, "metadata": {}, "output_type": "execute_result" }, @@ -81,7 +81,7 @@ }, { "cell_type": "code", - "execution_count": 22, + "execution_count": 2, "id": "f7fa885a", "metadata": { "hidden": true @@ -89,7 +89,7 @@ "outputs": [ { "data": {}, - "execution_count": 22, + "execution_count": 2, "metadata": {}, "output_type": "execute_result" }, @@ -120,7 +120,7 @@ }, { "cell_type": "code", - "execution_count": 23, + "execution_count": 3, "id": "08a17340", "metadata": { "hidden": true @@ -132,7 +132,7 @@ "{ color: \u001b[32m\"red\"\u001b[39m, area: \u001b[33m10000\u001b[39m }" ] }, - "execution_count": 23, + "execution_count": 3, "metadata": {}, "output_type": "execute_result" } @@ -177,7 +177,7 @@ }, { "cell_type": "code", - "execution_count": 24, + "execution_count": 4, "id": "bbf2c09b", "metadata": { "hidden": true @@ -185,7 +185,7 @@ "outputs": [ { "data": {}, - "execution_count": 24, + "execution_count": 4, "metadata": {}, "output_type": "execute_result" } @@ -207,7 +207,7 @@ }, { "cell_type": "code", - "execution_count": 25, + "execution_count": 5, "id": "d9801d80", "metadata": { "hidden": true @@ -219,7 +219,7 @@ "\u001b[1mnull\u001b[22m" ] }, - "execution_count": 25, + "execution_count": 5, "metadata": {}, "output_type": "execute_result" } @@ -241,7 +241,7 @@ }, { "cell_type": "code", - "execution_count": 26, + "execution_count": 6, "id": "cfaac330", "metadata": { "hidden": true @@ -253,7 +253,7 @@ "\u001b[33mtrue\u001b[39m" ] }, - "execution_count": 26, + "execution_count": 6, "metadata": {}, "output_type": "execute_result" } @@ -275,7 +275,7 @@ }, { "cell_type": "code", - "execution_count": 27, + "execution_count": 7, "id": "ec3be2da", "metadata": { "hidden": true @@ -287,7 +287,7 @@ "\u001b[33m42\u001b[39m" ] }, - "execution_count": 27, + "execution_count": 7, "metadata": {}, "output_type": "execute_result" } @@ -309,7 +309,7 @@ }, { "cell_type": "code", - "execution_count": 28, + "execution_count": 8, "id": "997cf2d7", "metadata": { "hidden": true @@ -321,7 +321,7 @@ "\u001b[32m\"this is a test of the emergency broadcast system\"\u001b[39m" ] }, - "execution_count": 28, + "execution_count": 8, "metadata": {}, "output_type": "execute_result" } @@ -343,7 +343,7 @@ }, { "cell_type": "code", - "execution_count": 29, + "execution_count": 9, "id": "44b63807", "metadata": { "hidden": true @@ -355,7 +355,7 @@ "\u001b[33m31337n\u001b[39m" ] }, - "execution_count": 29, + "execution_count": 9, "metadata": {}, "output_type": "execute_result" } @@ -377,7 +377,7 @@ }, { "cell_type": "code", - "execution_count": 30, + "execution_count": 10, "id": "e10c0d31", "metadata": { "hidden": true @@ -389,7 +389,7 @@ "\u001b[32mSymbol(foo)\u001b[39m" ] }, - "execution_count": 30, + "execution_count": 10, "metadata": {}, "output_type": "execute_result" } @@ -411,7 +411,7 @@ }, { "cell_type": "code", - "execution_count": 31, + "execution_count": 11, "id": "81c99233", "metadata": { "hidden": true @@ -423,7 +423,7 @@ "{ foo: \u001b[32m\"bar\"\u001b[39m }" ] }, - "execution_count": 31, + "execution_count": 11, "metadata": {}, "output_type": "execute_result" } @@ -445,7 +445,7 @@ }, { "cell_type": "code", - "execution_count": 32, + "execution_count": 12, "id": "43c1581b", "metadata": { "hidden": true @@ -457,7 +457,7 @@ "Promise { \u001b[32m\"it worked!\"\u001b[39m }" ] }, - "execution_count": 32, + "execution_count": 12, "metadata": {}, "output_type": "execute_result" } @@ -468,7 +468,7 @@ }, { "cell_type": "code", - "execution_count": 33, + "execution_count": 13, "id": "9a34b725", "metadata": { "hidden": true @@ -483,7 +483,7 @@ "}" ] }, - "execution_count": 33, + "execution_count": 13, "metadata": {}, "output_type": "execute_result" } @@ -494,7 +494,7 @@ }, { "cell_type": "code", - "execution_count": 34, + "execution_count": 14, "id": "b5c7b819", "metadata": { "scrolled": true @@ -520,7 +520,7 @@ }, { "cell_type": "code", - "execution_count": 35, + "execution_count": 15, "id": "14844fc9-536e-4121-a9bd-fc2d3f7b6395", "metadata": {}, "outputs": [ @@ -541,7 +541,7 @@ }, { "cell_type": "code", - "execution_count": 36, + "execution_count": 16, "id": "72d01fdd", "metadata": {}, "outputs": [ @@ -549,13 +549,15 @@ "data": { "text/plain": [ "Promise {\n", - " \u001b[36m\u001b[39m TypeError: Expected string at position 1\n", - " at Object.readFile (ext:deno_fs/30_fs.js:716:29)\n", - " at :2:6\n", + " \u001b[36m\u001b[39m NotFound: No such file or directory (os error 2): readfile ''\n", + " at async Object.readFile (ext:deno_fs/30_fs.js:716:18) {\n", + " name: \u001b[32m\"NotFound\"\u001b[39m,\n", + " code: \u001b[32m\"ENOENT\"\u001b[39m\n", + " }\n", "}" ] }, - "execution_count": 36, + "execution_count": 16, "metadata": {}, "output_type": "execute_result" } @@ -566,13 +568,13 @@ }, { "cell_type": "code", - "execution_count": 37, + "execution_count": 17, "id": "28cf59d0-6908-4edc-bb10-c325beeee362", "metadata": {}, "outputs": [ { "data": {}, - "execution_count": 37, + "execution_count": 17, "metadata": {}, "output_type": "execute_result" }, @@ -590,13 +592,13 @@ }, { "cell_type": "code", - "execution_count": 38, + "execution_count": 18, "id": "8d5485c3-0da3-43fe-8ef5-a61e672f5e81", "metadata": {}, "outputs": [ { "data": {}, - "execution_count": 38, + "execution_count": 18, "metadata": {}, "output_type": "execute_result" }, @@ -614,7 +616,7 @@ }, { "cell_type": "code", - "execution_count": 39, + "execution_count": 19, "id": "1401d9d5-6994-4c7b-b55a-db3c16a1e2dc", "metadata": {}, "outputs": [ @@ -624,7 +626,7 @@ "\u001b[32m\"Cool 🫡\"\u001b[39m" ] }, - "execution_count": 39, + "execution_count": 19, "metadata": {}, "output_type": "execute_result" } @@ -635,13 +637,13 @@ }, { "cell_type": "code", - "execution_count": 40, + "execution_count": 20, "id": "7afdaa0a-a2a0-4f52-8c7d-b6c5f237aa0d", "metadata": {}, "outputs": [ { "data": {}, - "execution_count": 40, + "execution_count": 20, "metadata": {}, "output_type": "execute_result" }, @@ -665,20 +667,24 @@ }, { "cell_type": "code", - "execution_count": 2, + "execution_count": 21, "id": "8e93df23-06eb-414b-98d4-51fbebb53d1f", "metadata": {}, "outputs": [ { - "ename": "Error", - "evalue": "Deno.jupyter is only available in `deno jupyter` subcommand.", - "output_type": "error", - "traceback": [ - "Stack trace:", - "Error: Deno.jupyter is only available in `deno jupyter` subcommand.", - " at Object.get (ext:cli/runtime/js/99_main.js:550:15)", - " at :2:12" - ] + "data": { + "text/html": [ + "Complete ✅" + ] + }, + "metadata": {}, + "output_type": "display_data" + }, + { + "data": {}, + "execution_count": 21, + "metadata": {}, + "output_type": "execute_result" } ], "source": [ From 5f29a3d59c1b6953de707f0970ddf1b971481f85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Wed, 27 Sep 2023 13:00:30 +0200 Subject: [PATCH 4/5] don't send reply if mime bundle is empty --- cli/tools/jupyter/server.rs | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/cli/tools/jupyter/server.rs b/cli/tools/jupyter/server.rs index c54dcd2756a56e..2489fcd047191e 100644 --- a/cli/tools/jupyter/server.rs +++ b/cli/tools/jupyter/server.rs @@ -391,15 +391,18 @@ impl JupyterServer { let output = get_jupyter_display_or_eval_value(&mut self.repl_session, &result) .await?; - msg - .new_message("execute_result") - .with_content(json!({ - "execution_count": self.execution_count, - "data": output, - "metadata": {}, - })) - .send(&mut *self.iopub_socket.lock().await) - .await?; + // Don't bother sending `execute_result` reply if the MIME bundle is empty + if !output.is_empty() { + msg + .new_message("execute_result") + .with_content(json!({ + "execution_count": self.execution_count, + "data": output, + "metadata": {}, + })) + .send(&mut *self.iopub_socket.lock().await) + .await?; + } msg .new_reply() .with_content(json!({ From 1c3485f3d02313b7863cd49feed77104bcb4785c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Wed, 27 Sep 2023 13:17:08 +0200 Subject: [PATCH 5/5] reset CI