From ffb328fab02640fa3db4d77ed8f703233fb27d4e Mon Sep 17 00:00:00 2001 From: Fons van der Plas Date: Mon, 12 Aug 2024 15:41:28 +0200 Subject: [PATCH 1/5] variable renames --- frontend/components/Editor.js | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/frontend/components/Editor.js b/frontend/components/Editor.js index b33f0e3acd..c01acd67c9 100644 --- a/frontend/components/Editor.js +++ b/frontend/components/Editor.js @@ -812,7 +812,7 @@ patch: ${JSON.stringify( } } apply_promise.finally(set_waiting).then(() => { - this.send_queued_bond_changes() + this.maybe_send_queued_bond_changes() }) break @@ -971,18 +971,18 @@ patch: ${JSON.stringify( // Not completely happy with this yet, but it will do for now - DRAL /** Patches that are being delayed until all cells have finished running. */ - this.bonds_changes_to_apply_when_done = [] - this.send_queued_bond_changes = () => { - if (this.notebook_is_idle() && this.bonds_changes_to_apply_when_done.length !== 0) { - // console.log("Applying queued bond changes!", this.bonds_changes_to_apply_when_done) - let bonds_patches = this.bonds_changes_to_apply_when_done - this.bonds_changes_to_apply_when_done = [] + this.bond_changes_to_apply_when_done = [] + this.maybe_send_queued_bond_changes = () => { + if (this.notebook_is_idle() && this.bond_changes_to_apply_when_done.length !== 0) { + // console.log("Applying queued bond changes!", this.bond_changes_to_apply_when_done) + let bonds_patches = this.bond_changes_to_apply_when_done + this.bond_changes_to_apply_when_done = [] this.update_notebook((notebook) => { applyPatches(notebook, bonds_patches) }) } } - /** Whether we just set a bond value which will trigger a cell to run, but we are still waiting for the server to process the bond value (and run the cell). During this time, we won't send new bond values. See https://github.com/fonsp/Pluto.jl/issues/1891 for more info. */ + /** This tracks whether we just set a bond value which will trigger a cell to run, but we are still waiting for the server to process the bond value (and run the cell). During this time, we won't send new bond values. See https://github.com/fonsp/Pluto.jl/issues/1891 for more info. */ this.waiting_for_bond_to_trigger_execution = false /** Number of local updates that have not yet been applied to the server's state. */ this.pending_local_updates = 0 @@ -992,7 +992,7 @@ patch: ${JSON.stringify( */ this.js_init_set = new SetWithEmptyCallback(() => { // console.info("All scripts finished!") - this.send_queued_bond_changes() + this.maybe_send_queued_bond_changes() }) // @ts-ignore This is for tests @@ -1060,7 +1060,7 @@ patch: ${JSON.stringify( let is_idle = this.notebook_is_idle() let changes_involving_bonds = changes.filter((x) => x.path[0] === "bonds") if (!is_idle) { - this.bonds_changes_to_apply_when_done = [...this.bonds_changes_to_apply_when_done, ...changes_involving_bonds] + this.bond_changes_to_apply_when_done = [...this.bond_changes_to_apply_when_done, ...changes_involving_bonds] changes = changes.filter((x) => x.path[0] !== "bonds") } @@ -1430,7 +1430,7 @@ The notebook file saves every time you run a cell.` document.title = "🎈 " + new_state.notebook.shortpath + " — Pluto.jl" } - this.send_queued_bond_changes() + this.maybe_send_queued_bond_changes() if (old_state.backend_launch_phase !== this.state.backend_launch_phase && this.state.backend_launch_phase != null) { const phase = Object.entries(BackendLaunchPhase).find(([k, v]) => v == this.state.backend_launch_phase)?.[0] From 3c633ec447b4c39df3bc900c26ad310ec4bbd1ff Mon Sep 17 00:00:00 2001 From: Fons van der Plas Date: Mon, 12 Aug 2024 15:42:40 +0200 Subject: [PATCH 2/5] Check patches to ignore status_tree updates --- frontend/components/Editor.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/frontend/components/Editor.js b/frontend/components/Editor.js index c01acd67c9..7070c992fe 100644 --- a/frontend/components/Editor.js +++ b/frontend/components/Editor.js @@ -805,7 +805,9 @@ patch: ${JSON.stringify( const set_waiting = () => { let from_update = message?.response?.update_went_well != null let is_just_acknowledgement = from_update && message.patches.length === 0 - // console.log("Received patches!", message.patches, message.response, is_just_acknowledgement) + let is_relevant_for_bonds = message.patches.some(({ path }) => path.length === 0 || path[0] !== "status_tree") + + // console.debug("Received patches!", is_just_acknowledgement, is_relevant_for_bonds, message.patches, message.response) if (!is_just_acknowledgement) { this.waiting_for_bond_to_trigger_execution = false From 103da2d1fcea999403cfcf20ade2f7f372a02cab Mon Sep 17 00:00:00 2001 From: Fons van der Plas Date: Mon, 12 Aug 2024 15:43:10 +0200 Subject: [PATCH 3/5] delay throttle from status tree updates at run start --- src/evaluation/Run.jl | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/evaluation/Run.jl b/src/evaluation/Run.jl index 81be9f736b..ae1d223578 100644 --- a/src/evaluation/Run.jl +++ b/src/evaluation/Run.jl @@ -57,6 +57,9 @@ function run_reactive_core!( old_workspace_name, _ = WorkspaceManager.bump_workspace_module((session, notebook)) + # A state sync will come soon from this function, so let's delay anything coming from the status_tree listener, see https://github.com/fonsp/Pluto.jl/issues/2978 + Throttled.force_throttle_without_run(notebook.status_tree.update_listener_ref[]) + run_status = Status.report_business_started!(notebook.status_tree, :run) Status.report_business_started!(run_status, :resolve_topology) cell_status = Status.report_business_planned!(run_status, :evaluate) From b79b7eefcd520b40dcab220c65fdf5987fb8329f Mon Sep 17 00:00:00 2001 From: Fons van der Plas Date: Mon, 12 Aug 2024 15:49:04 +0200 Subject: [PATCH 4/5] test --- sample/test_bind_dynamics.jl | 91 ++++++++++++++++++++++++++++++++++++ 1 file changed, 91 insertions(+) create mode 100644 sample/test_bind_dynamics.jl diff --git a/sample/test_bind_dynamics.jl b/sample/test_bind_dynamics.jl new file mode 100644 index 0000000000..7096c475db --- /dev/null +++ b/sample/test_bind_dynamics.jl @@ -0,0 +1,91 @@ +### A Pluto.jl notebook ### +# v0.19.45 + +using Markdown +using InteractiveUtils + +# This Pluto notebook uses @bind for interactivity. When running this notebook outside of Pluto, the following 'mock version' of @bind gives bound variables a default value (instead of an error). +macro bind(def, element) + quote + local iv = try Base.loaded_modules[Base.PkgId(Base.UUID("6e696c72-6542-2067-7265-42206c756150"), "AbstractPlutoDingetjes")].Bonds.initial_value catch; b -> missing; end + local el = $(esc(element)) + global $(esc(def)) = Core.applicable(Base.get, el) ? Base.get(el) : iv(el) + el + end +end + +# ╔═╡ a0fe4e4d-eee5-4420-bd58-3f12749a9ed1 +@bind reset_xs html"" + +# ╔═╡ 58db6bd4-58a6-11ef-3795-fd6e57eceb68 +@bind x html"""
+ + +
""" + +# ╔═╡ 8568c646-0233-4a95-8332-2351e9c56027 +@bind withsleep html"" + +# ╔═╡ 8a20fa4a-ac02-4a37-a54e-e4224628db66 +x + +# ╔═╡ 29f1d840-574e-463c-87d3-4b938e123493 +begin + reset_xs + xs = [] +end + +# ╔═╡ 3155b6e0-8e19-4583-b2ab-4ab2db1f10b9 +md""" +Click **reset_xs**. + +Click **Start**. + +The cell below should give: `1,done`. + +Not: `1,2,done` or something like that. That means that an intermediate bond value (`2`) found its way through: [https://github.com/fonsp/Pluto.jl/issues/1891](https://github.com/fonsp/Pluto.jl/issues/1891) +""" + +# ╔═╡ 029e1d1c-bf42-4e2c-a141-1e2eecc0800d +begin + withsleep && sleep(1.5) + push!(xs, x) + xs_done = true + + join(xs[2:end], ",") |> Text +end + +# ╔═╡ Cell order: +# ╟─a0fe4e4d-eee5-4420-bd58-3f12749a9ed1 +# ╟─58db6bd4-58a6-11ef-3795-fd6e57eceb68 +# ╟─8568c646-0233-4a95-8332-2351e9c56027 +# ╠═8a20fa4a-ac02-4a37-a54e-e4224628db66 +# ╠═29f1d840-574e-463c-87d3-4b938e123493 +# ╟─3155b6e0-8e19-4583-b2ab-4ab2db1f10b9 +# ╠═029e1d1c-bf42-4e2c-a141-1e2eecc0800d From 4520f194e3067582fcdeaf6051c4b92833f90ac6 Mon Sep 17 00:00:00 2001 From: Fons van der Plas Date: Mon, 12 Aug 2024 16:43:18 +0200 Subject: [PATCH 5/5] puppeteer test --- test/frontend/__tests__/bonds.js | 66 ++++++++++++++++++- .../frontend/fixtures}/test_bind_dynamics.jl | 6 +- 2 files changed, 66 insertions(+), 6 deletions(-) rename {sample => test/frontend/fixtures}/test_bind_dynamics.jl (95%) diff --git a/test/frontend/__tests__/bonds.js b/test/frontend/__tests__/bonds.js index af633a3400..0050429d45 100644 --- a/test/frontend/__tests__/bonds.js +++ b/test/frontend/__tests__/bonds.js @@ -1,9 +1,18 @@ import puppeteer from "puppeteer" -import { saveScreenshot, createPage, paste } from "../helpers/common" -import { createNewNotebook, getPlutoUrl, runAllChanged, setupPlutoBrowser, shutdownCurrentNotebook, waitForPlutoToCalmDown } from "../helpers/pluto" +import { saveScreenshot, createPage, paste, waitForContentToBecome, waitForContent } from "../helpers/common" +import { + createNewNotebook, + getPlutoUrl, + importNotebook, + runAllChanged, + setupPlutoBrowser, + shutdownCurrentNotebook, + waitForCellOutput, + waitForPlutoToCalmDown, +} from "../helpers/pluto" // https://github.com/fonsp/Pluto.jl/issues/928 -describe("Bonds should run once when refreshing page", () => { +describe("@bind", () => { /** * Launch a shared browser instance for all tests. * I don't use jest-puppeteer because it takes away a lot of control and works buggy for me, @@ -78,4 +87,55 @@ numberoftimes = Ref(0) }) expect(output_after_reload).toBe(output_after_running_bonds) }) + + it("should ignore intermediate bond values while the notebook is running", async () => { + const wait = (ms) => new Promise((resolve) => setTimeout(resolve, ms)) + + const chill = async () => { + await wait(300) + await waitForPlutoToCalmDown(page) + await wait(1500) + await waitForPlutoToCalmDown(page) + } + + await importNotebook(page, "test_bind_dynamics.jl") + await chill() + await chill() + + const id = `029e1d1c-bf42-4e2c-a141-1e2eecc0800d` + const output_selector = `pluto-cell[id="${id}"] pluto-output` + + //page.click is stupid + const click = async (sel) => { + await page.waitForSelector(sel) + await page.evaluate((sel) => document.querySelector(sel).click(), sel) + } + + const reset = async () => { + await click(`#reset_xs_button`) + await wait(300) + await waitForPlutoToCalmDown(page) + await waitForContentToBecome(page, output_selector, "") + await wait(300) + await waitForPlutoToCalmDown(page) + await waitForContentToBecome(page, output_selector, "") + await wait(300) + } + + const start = async () => { + await click(`#add_x_button`) + await chill() + + return await waitForContent(page, output_selector) + } + + await reset() + await start() + + await chill() + + await reset() + const val = await start() + expect(val).toBe("1,done") + }) }) diff --git a/sample/test_bind_dynamics.jl b/test/frontend/fixtures/test_bind_dynamics.jl similarity index 95% rename from sample/test_bind_dynamics.jl rename to test/frontend/fixtures/test_bind_dynamics.jl index 7096c475db..0746d5c805 100644 --- a/sample/test_bind_dynamics.jl +++ b/test/frontend/fixtures/test_bind_dynamics.jl @@ -15,7 +15,7 @@ macro bind(def, element) end # ╔═╡ a0fe4e4d-eee5-4420-bd58-3f12749a9ed1 -@bind reset_xs html"" +@bind reset_xs html"" # ╔═╡ 58db6bd4-58a6-11ef-3795-fd6e57eceb68 @bind x html"""
@@ -46,7 +46,7 @@ btn.onclick = () => { go() } - +
""" # ╔═╡ 8568c646-0233-4a95-8332-2351e9c56027 @@ -86,6 +86,6 @@ end # ╟─58db6bd4-58a6-11ef-3795-fd6e57eceb68 # ╟─8568c646-0233-4a95-8332-2351e9c56027 # ╠═8a20fa4a-ac02-4a37-a54e-e4224628db66 -# ╠═29f1d840-574e-463c-87d3-4b938e123493 # ╟─3155b6e0-8e19-4583-b2ab-4ab2db1f10b9 # ╠═029e1d1c-bf42-4e2c-a141-1e2eecc0800d +# ╠═29f1d840-574e-463c-87d3-4b938e123493