Skip to content

Commit d4ebe0e

Browse files
authored
Set turtle graphics inside runCode to avoid null.firstChild (#1428)
Closes [Intermittent ExternalError when running Turtle projects after saving #1261](RaspberryPiFoundation/digital-editor-issues#1261) ### Findings - The 'firstChild' error is not at all reproducible locally - In staging, sometimes it errors and sometimes not. Couldn't see any pattern 🤷‍♀️ - Upon inspecting `turtle.js` within the skupt module, it seemed that the type error could be coming from the `target` being null while checking `target.firstChild` for the while loop: ```js // turtle.js ... function getConfiguredTarget() { var selector, target; selector = (Sk.TurtleGraphics && Sk.TurtleGraphics.target) || "turtle", target = typeof selector === "string" ? document.getElementById(selector) : selector; // ensure that the canvas container is empty while (target.firstChild) { target.removeChild(target.firstChild); } return target; } ... ``` - `SkulptRunner` runs runCode() in a parent useEffect. `VisualOutputPane` sets `Sk.TurtleGraphics.target` in a child useEffect. React runs parent effects before child effects, so from turtle import * can run before the child has set `Sk.TurtleGraphics.target`. ### Potential fix - Ensured `Sk.TurtleGraphics.target` points at `#turtleOutput` before turtle runs by setting it within the SkulpRunner's runCode()
1 parent a1c2ace commit d4ebe0e

5 files changed

Lines changed: 116 additions & 8 deletions

File tree

src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.jsx

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import { SettingsContext } from "../../../../../utils/settings";
2525
import RunnerControls from "../../../../RunButton/RunnerControls";
2626
import { MOBILE_MEDIA_QUERY } from "../../../../../utils/mediaQueryBreakpoints";
2727
import { getPythonImports } from "../../../../../utils/getPythonImports";
28+
import { configureTurtleGraphics } from "../../../../../utils/configureTurtleGraphics";
2829

2930
const externalLibraries = {
3031
"./pygal/__init__.js": {
@@ -409,6 +410,18 @@ const SkulptRunner = ({ active, outputPanels = ["text", "visual"] }) => {
409410
}
410411
dispatch(setSenseHatEnabled(false));
411412

413+
// runCode runs from the current runner's useEffect before VisualOutputPane’s useEffect
414+
// so Sk.TurtleGraphics.target / assets are set here so they exist before import
415+
const host = document.querySelector("editor-wc");
416+
const turtleOutputElement =
417+
host?.shadowRoot?.getElementById("turtleOutput") ||
418+
document.getElementById("turtleOutput");
419+
420+
configureTurtleGraphics({
421+
targetEl: turtleOutputElement,
422+
projectImages: project.image_list || [],
423+
});
424+
412425
var prog = mainComponent?.content || "";
413426

414427
if (prog.includes(`# ${t("input.comment.py5")}`)) {

src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.test.js

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1329,3 +1329,59 @@ describe("When not active and a code run has been triggered", () => {
13291329
expect(Sk.misceval.asyncToPromise).not.toHaveBeenCalled();
13301330
});
13311331
});
1332+
1333+
describe("Turtle graphics is wired before Skulpt import", () => {
1334+
const asyncToPromise = Sk.misceval.asyncToPromise;
1335+
1336+
afterEach(() => {
1337+
Sk.misceval.asyncToPromise = asyncToPromise;
1338+
});
1339+
1340+
test("sets Sk.TurtleGraphics.target and assets before asyncToPromise runs", () => {
1341+
Sk.misceval.asyncToPromise = jest.fn((_computeFn) => {
1342+
const turtleElement = document.getElementById("turtleOutput");
1343+
expect(turtleElement).not.toBeNull();
1344+
expect(Sk.TurtleGraphics.target).toBe(turtleElement);
1345+
expect(Sk.TurtleGraphics.assets["pic.png"]).toBe(
1346+
"https://example.com/img.png",
1347+
);
1348+
return Promise.resolve();
1349+
});
1350+
1351+
const middlewares = [];
1352+
const mockStore = configureStore(middlewares);
1353+
const initialState = {
1354+
editor: {
1355+
project: {
1356+
components: [
1357+
{
1358+
name: "main",
1359+
extension: "py",
1360+
content: "print('hello')",
1361+
},
1362+
],
1363+
image_list: [
1364+
{
1365+
name: "pic",
1366+
extension: "png",
1367+
url: "https://example.com/img.png",
1368+
},
1369+
],
1370+
},
1371+
codeRunTriggered: true,
1372+
isEmbedded: false,
1373+
},
1374+
auth: {
1375+
user,
1376+
},
1377+
};
1378+
const store = mockStore(initialState);
1379+
render(
1380+
<Provider store={store}>
1381+
<SkulptRunner active={true} />
1382+
</Provider>,
1383+
);
1384+
1385+
expect(Sk.misceval.asyncToPromise).toHaveBeenCalled();
1386+
});
1387+
});

src/components/Editor/Runners/PythonRunner/VisualOutputPane.jsx

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { useDispatch, useSelector } from "react-redux";
55
import Sk from "skulpt";
66
import AstroPiModel from "../../../AstroPiModel/AstroPiModel";
77
import { codeRunHandled, setError } from "../../../../redux/EditorSlice";
8+
import { configureTurtleGraphics } from "../../../../utils/configureTurtleGraphics";
89

910
const VisualOutputPane = () => {
1011
const codeRunTriggered = useSelector(
@@ -43,14 +44,10 @@ const VisualOutputPane = () => {
4344

4445
(Sk.pygal || (Sk.pygal = {})).outputCanvas = pygalOutput.current;
4546

46-
(Sk.TurtleGraphics || (Sk.TurtleGraphics = {})).target =
47-
turtleOutput.current;
48-
Sk.TurtleGraphics.assets = Object.assign(
49-
{},
50-
...projectImages.map((image) => ({
51-
[`${image.name}.${image.extension}`]: image.url,
52-
})),
53-
);
47+
configureTurtleGraphics({
48+
targetEl: turtleOutput.current,
49+
projectImages,
50+
});
5451
}
5552
}, [codeRunTriggered, projectImages]);
5653

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
import Sk from "skulpt";
2+
3+
/**
4+
* Wires Skulpt turtle to a DOM container and project image URLs for stamps.
5+
* Asset keys follow `${name}.${extension}` → url (Skulpt turtle convention).
6+
*/
7+
export function configureTurtleGraphics({ targetEl, projectImages = [] }) {
8+
if (!targetEl) {
9+
return;
10+
}
11+
(Sk.TurtleGraphics || (Sk.TurtleGraphics = {})).target = targetEl;
12+
Sk.TurtleGraphics.assets = Object.assign(
13+
{},
14+
...projectImages.map((image) => ({
15+
[`${image.name}.${image.extension}`]: image.url,
16+
})),
17+
);
18+
}
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
import Sk from "skulpt";
2+
import { configureTurtleGraphics } from "./configureTurtleGraphics";
3+
4+
describe("configureTurtleGraphics", () => {
5+
test("sets Sk.TurtleGraphics target and assets map from project images", () => {
6+
const el = document.createElement("div");
7+
configureTurtleGraphics({
8+
targetEl: el,
9+
projectImages: [
10+
{ name: "a", extension: "png", url: "https://x.test/a.png" },
11+
{ name: "b", extension: "gif", url: "https://x.test/b.gif" },
12+
],
13+
});
14+
expect(Sk.TurtleGraphics.target).toBe(el);
15+
expect(Sk.TurtleGraphics.assets["a.png"]).toBe("https://x.test/a.png");
16+
expect(Sk.TurtleGraphics.assets["b.gif"]).toBe("https://x.test/b.gif");
17+
});
18+
19+
test("no-ops when targetEl is null", () => {
20+
expect(() =>
21+
configureTurtleGraphics({ targetEl: null, projectImages: [] }),
22+
).not.toThrow();
23+
});
24+
});

0 commit comments

Comments
 (0)