Skip to content

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

Merged
cocomarine merged 6 commits intomainfrom
hj/fix-turle-import-external-error
Apr 17, 2026
Merged

Set turtle graphics inside runCode to avoid null.firstChild#1428
cocomarine merged 6 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.

Comment thread src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.jsx Outdated
@cocomarine cocomarine temporarily deployed to previews/1428/merge April 10, 2026 15:09 — with GitHub Actions Inactive
@zetter-rpf zetter-rpf requested a review from DNR500 April 13, 2026 08:53
@cocomarine cocomarine temporarily deployed to previews/1428/merge April 16, 2026 07:31 — with GitHub Actions Inactive
@cocomarine cocomarine temporarily deployed to previews/1428/merge April 16, 2026 08:29 — with GitHub Actions Inactive
Copy link
Copy Markdown
Contributor

@DNR500 DNR500 left a comment

Choose a reason for hiding this comment

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

Nice! I can see how this should avoid a race condition between turtle setup and code execution in VisualOutputPane. Also, thanks for adding the comments and tests

@cocomarine cocomarine enabled auto-merge (squash) April 17, 2026 08:49
@cocomarine cocomarine merged commit d4ebe0e into main Apr 17, 2026
6 checks passed
@cocomarine cocomarine deleted the hj/fix-turle-import-external-error branch April 17, 2026 08:52
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.

3 participants