Skip to content

Set turtle graphics inside runCode to avoid null.firstChild#1428

Open
cocomarine wants to merge 3 commits intomainfrom
hj/fix-turle-import-external-error
Open

Set turtle graphics inside runCode to avoid null.firstChild#1428
cocomarine wants to merge 3 commits intomainfrom
hj/fix-turle-import-external-error

Conversation

@cocomarine
Copy link
Copy Markdown
Contributor

@cocomarine cocomarine commented Apr 10, 2026

Closes Intermittent ExternalError when running Turtle projects after saving #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:
// 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()

Copilot AI review requested due to automatic review settings April 10, 2026 14:40
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses an intermittent Skulpt Turtle graphics failure (reported as null.firstChild) by ensuring the Turtle graphics target/asset wiring happens before Skulpt imports/executes user code, particularly in the web component (shadow DOM) context.

Changes:

  • Initialize Sk.TurtleGraphics.target and Sk.TurtleGraphics.assets inside runCode() before invoking Sk.misceval.asyncToPromise / importing the main module.
  • Add a regression test asserting Turtle wiring occurs before asyncToPromise is entered.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.jsx Wires Turtle graphics target/assets earlier (and supports shadowRoot lookup) to prevent null access during turtle import/runtime init.
src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.test.js Adds a unit test ensuring Turtle configuration is set before Skulpt execution begins.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants