fix: scan_external_changes never reloads script-bearing modules

.changeset/hot-reload-script-modules.md

@@ -0,0 +1,5 @@
+---
+"@godot-js/editor": patch
+---
+
+**Fix:** Editing a script on disk now reloads script-bearing modules and rebinds live `GodotJSScript` instances. `scan_external_changes()` no longer skips script modules, cascades the reload through transitive dependents, and resets `module.exports` so ES-module-style default exports survive a second run.

bridge/jsb_bridge_module_loader.cpp

@@ -3,6 +3,7 @@
 #include "jsb_editor_utility_funcs.h"
 #include "jsb_callable.h"
 #include "jsb_object_bindings.h"
+#include "../weaver/jsb_script_language.h"
 
 namespace jsb
 {
@@ -480,6 +481,28 @@ namespace jsb
             }
         }
 
+        // function scan_external_changes(): void
+        // Re-scan loaded modules, reloading any whose mtime + hash drifted since last load.
+        // Mirrors the editor's app-focus-driven scan so headless tests (and other non-editor
+        // callers) can drive a reload without an EditorFileSystem.
+        void _scan_external_changes(const v8::FunctionCallbackInfo<v8::Value>& info)
+        {
+            Environment* env = Environment::wrap(info.GetIsolate());
+            jsb_check(env);
+            // Prefer the language wrapper so script-bearing modules get their
+            // live instances rebound after the env reload. Fall back to env
+            // alone if the language singleton isn't initialised (unusual,
+            // headless-jsb-only builds).
+            if (GodotJSScriptLanguage* lang = GodotJSScriptLanguage::get_singleton())
+            {
+                lang->scan_external_changes();
+            }
+            else
+            {
+                (void) env->scan_external_changes();
+            }
+        }
+
         void _add_module(const v8::FunctionCallbackInfo<v8::Value>& info)
         {
             v8::Isolate* isolate = info.GetIsolate();
@@ -596,6 +619,7 @@ namespace jsb
 
                 internal_obj->Set(context, impl::Helper::new_string_ascii(isolate, "find_module"), JSB_NEW_FUNCTION(context, _find_module, {})).Check();
                 internal_obj->Set(context, impl::Helper::new_string_ascii(isolate, "add_module"), JSB_NEW_FUNCTION(context, _add_module, {})).Check();
+                internal_obj->Set(context, impl::Helper::new_string_ascii(isolate, "scan_external_changes"), JSB_NEW_FUNCTION(context, _scan_external_changes, {})).Check();
 
                 internal_obj->Set(context, impl::Helper::new_string_ascii(isolate, "add_script_signal"), JSB_NEW_FUNCTION(context, _add_script_signal, {})).Check();
                 internal_obj->Set(context, impl::Helper::new_string_ascii(isolate, "add_script_property"), JSB_NEW_FUNCTION(context, _add_script_property, {})).Check();

bridge/jsb_environment.cpp

@@ -1089,26 +1089,83 @@ namespace jsb
         return new_id;
     }
 
-    void Environment::scan_external_changes()
+    Vector<StringName> Environment::scan_external_changes()
     {
         check_internal_state();
+        v8::Isolate* isolate = isolate_;
+        v8::Isolate::Scope isolate_scope(isolate);
+        v8::HandleScope handle_scope(isolate);
+        v8::Local<v8::Context> context = context_.Get(isolate);
+        v8::Context::Scope context_scope(context);
+
         Vector<StringName> requested_modules;
+        HashSet<StringName> dirty_set;
         for (const KeyValue<StringName, JavaScriptModule*>& kv : module_cache_.modules_)
         {
             JavaScriptModule* module = kv.value;
-            // skip script modules which are managed by the godot editor
-            if (module->script_class_id) continue;
+            // Script-bearing modules are no longer skipped. The reload path
+            // inside _load_module already handles re-execution and
+            // ScriptClassInfo::_parse_script_class; GodotJSScriptLanguage
+            // uses the returned ids to rebind live instances.
             if (module->mark_as_reloading())
             {
+                JSB_LOG(Verbose, "[reload] marked dirty: %s (is_script=%d)", module->id, (int)(bool)module->script_class_id);
                 requested_modules.append(module->id);
+                dirty_set.insert(module->id);
+            }
+        }
+
+        // Transitive invalidation: any module whose `children` array contains
+        // a dirty module is also dirty. Fixed-point iterate over the module
+        // graph. `children` is the v8 Array built during initial load (see the
+        // jsb_name(this, children) Set in _load_module); each entry is another
+        // module object with an `id` property.
+        const v8::Local<v8::Name> children_name = jsb_name(this, children);
+        const v8::Local<v8::Name> id_name = jsb_name(this, id);
+        bool changed = true;
+        while (changed)
+        {
+            changed = false;
+            for (const KeyValue<StringName, JavaScriptModule*>& kv : module_cache_.modules_)
+            {
+                JavaScriptModule* module = kv.value;
+                if (dirty_set.has(module->id)) continue;
+                const v8::Local<v8::Object> module_obj = module->module.Get(isolate);
+                v8::Local<v8::Value> children_val;
+                if (!module_obj->Get(context, children_name).ToLocal(&children_val) || !children_val->IsArray()) continue;
+                const v8::Local<v8::Array> children = children_val.As<v8::Array>();
+                const uint32_t count = children->Length();
+                bool depends_on_dirty = false;
+                for (uint32_t i = 0; i < count; i++)
+                {
+                    v8::Local<v8::Value> child_val;
+                    if (!children->Get(context, i).ToLocal(&child_val) || !child_val->IsObject()) continue;
+                    v8::Local<v8::Value> child_id_val;
+                    if (!child_val.As<v8::Object>()->Get(context, id_name).ToLocal(&child_id_val) || !child_id_val->IsString()) continue;
+                    const String child_id_str = impl::Helper::to_string(isolate, child_id_val);
+                    if (dirty_set.has(StringName(child_id_str)))
+                    {
+                        depends_on_dirty = true;
+                        break;
+                    }
+                }
+                if (depends_on_dirty)
+                {
+                    JSB_LOG(Verbose, "[reload] cascaded dirty: %s (depends on a dirty module)", module->id);
+                    module->force_mark_as_reloading();
+                    requested_modules.append(module->id);
+                    dirty_set.insert(module->id);
+                    changed = true;
+                }
             }
         }
 
         for (const StringName& id : requested_modules)
         {
-            JSB_LOG(Verbose, "changed module check: %s", id);
+            JSB_LOG(Verbose, "[reload] reloading via load(): %s", id);
             load(id);
         }
+        return requested_modules;
     }
 
     ModuleReloadResult::Type Environment::mark_as_reloading(const StringName& p_name)
@@ -1242,6 +1299,20 @@ namespace jsb
 
                     JSB_LOG(VeryVerbose, "reload module %s", module_id);
                     resolved_module->mark_as_reloaded();
+
+                    // Reset `exports` to a fresh Object before re-running the
+                    // wrapped source. Compilers targeting CJS for ES `export default`
+                    // emit `Object.defineProperty(exports, "default", { configurable: false, ... })`,
+                    // which throws "Cannot redefine property" on the second run
+                    // when the OLD non-configurable "default" still sits on
+                    // exports. Without this reset, the reload silently fails
+                    // for any module using ES-module-style default exports.
+                    {
+                        v8::Local<v8::Object> fresh_exports = v8::Object::New(isolate);
+                        const v8::Local<v8::Object> module_obj_local = resolved_module->module.Get(isolate);
+                        module_obj_local->Set(context, jsb_name(this, exports), fresh_exports).Check();
+                        resolved_module->exports.Reset(isolate, fresh_exports);
+                    }
                     if (!resolver->load(this, source_info.source_filepath, *resolved_module))
                     {
                         return nullptr;

bridge/jsb_environment.h

@@ -399,9 +399,11 @@ namespace jsb
         JavaScriptModule* _load_module(const String& p_parent_id, const String& p_module_id);
 
         // manually scan changes of modules,
-        // will reload IMMEDIATELY
-        // (modules not attached with GodotJS script are not automatically reloaded by resource manager)
-        void scan_external_changes();
+        // will reload IMMEDIATELY and return the list of module ids whose
+        // sources changed on disk and were re-executed. Script-bearing modules
+        // are included; callers can use the list to rebind live GodotJSScript
+        // instances.
+        Vector<StringName> scan_external_changes();
 
         // request to reload a module,
         // will reload until next load.

bridge/jsb_module.cpp

@@ -46,6 +46,21 @@ namespace jsb
 #endif
     }
 
+    void JavaScriptModule::force_mark_as_reloading()
+    {
+#if JSB_SUPPORT_RELOAD && defined(TOOLS_ENABLED)
+        // Bypass the mtime/md5 gate — used by transitive invalidation when a
+        // dependency reloaded but our own source bytes didn't change. Bumping
+        // time_modified to the current disk mtime keeps the cache coherent
+        // with the next mark_as_reloading() probe.
+        if (is_reloadable())
+        {
+            time_modified = FileAccess::get_modified_time(source_info.source_filepath);
+        }
+        reload_requested = true;
+#endif
+    }
+
     JavaScriptModule& JavaScriptModuleCache::insert(v8::Isolate* isolate, const v8::Local<v8::Context>& context, const StringName& p_name, bool p_main_candidate, bool p_init_loaded)
     {
         jsb_checkf(!((String) p_name).is_empty(), "empty module name is not allowed");

bridge/jsb_module.h

@@ -58,6 +58,11 @@ namespace jsb
         bool mark_as_reloading();
         void mark_as_reloaded();
 
+        // Force reload even when mtime/md5 haven't changed (transitive
+        // invalidation cascades dependent modules). Safe to call any time;
+        // matches mark_as_reloaded()'s reset semantics for hash/time fields.
+        void force_mark_as_reloading();
+
     };
 
     struct JavaScriptModuleCache

tests/project/tests/reload/Reload.tscn

@@ -0,0 +1,11 @@
+[gd_scene load_steps=2 format=3]
+
+[ext_resource type="Script" path="res://tests/reload/test-reload.ts" id="1_rl1ad"]
+
+[node name="Reload" type="Node2D"]
+script = ExtResource("1_rl1ad")
+
+[node name="Label" type="Label" parent="."]
+offset_right = 40.0
+offset_bottom = 23.0
+text = "Reload"

tests/project/tests/reload/ScriptClassReload.tscn

@@ -0,0 +1,11 @@
+[gd_scene load_steps=2 format=3]
+
+[ext_resource type="Script" path="res://tests/reload/test-script-class-reload.ts" id="1_scrl1"]
+
+[node name="ScriptClassReload" type="Node2D"]
+script = ExtResource("1_scrl1")
+
+[node name="Label" type="Label" parent="."]
+offset_right = 40.0
+offset_bottom = 23.0
+text = "ScriptClassReload"

tests/project/tests/reload/script-class-target.js

@@ -0,0 +1,14 @@
+const godot = require("godot");
+
+class ScriptClassTarget extends godot.Node {
+    getValue() {
+        return 1;
+    }
+}
+
+Object.defineProperty(module.exports, "default", {
+    configurable: false,
+    enumerable: true,
+    writable: false,
+    value: ScriptClassTarget,
+});

tests/project/tests/reload/target.js

@@ -0,0 +1 @@
+module.exports = { getValue: () => 1 };

tests/project/tests/reload/test-reload.ts

@@ -0,0 +1,58 @@
+import { FileAccess, Node } from "godot";
+import { beginAsyncTest, endAsyncTest, reportTestFailure } from "../test-status";
+
+declare function require(id: string): any;
+
+const jsb = require("godot-jsb") as { internal: { scan_external_changes: () => void } };
+
+const TARGET_PATH = "res://tests/reload/target.js";
+
+async function runReloadTest(): Promise<void> {
+    const reader = FileAccess.open(TARGET_PATH, FileAccess.ModeFlags.READ);
+    if (!reader) throw new Error("failed to open target.js for read");
+    const original = reader.get_as_text();
+    reader.close();
+
+    try {
+        const initial = require("./target");
+        const initialValue = initial.getValue();
+        if (initialValue !== 1) {
+            throw new Error(`expected initial value 1, got ${initialValue}`);
+        }
+
+        // FileAccess::get_modified_time is 1-second-granular on most platforms;
+        // wait so mark_as_reloading() sees a distinct mtime on the second write.
+        await new Promise<void>((resolve) => setTimeout(resolve, 1100));
+
+        const writer = FileAccess.open(TARGET_PATH, FileAccess.ModeFlags.WRITE);
+        if (!writer) throw new Error("failed to open target.js for write");
+        writer.store_string("module.exports = { getValue: () => 2 };\n");
+        writer.close();
+
+        const t0 = Date.now();
+        jsb.internal.scan_external_changes();
+        const reloaded = require("./target");
+        const elapsed = Date.now() - t0;
+
+        const newValue = reloaded.getValue();
+        if (newValue !== 2) {
+            throw new Error(`expected reloaded value 2, got ${newValue}`);
+        }
+        console.log(`TestReload: module hot reload OK (${elapsed}ms)`);
+    } finally {
+        const restorer = FileAccess.open(TARGET_PATH, FileAccess.ModeFlags.WRITE);
+        if (restorer) {
+            restorer.store_string(original);
+            restorer.close();
+        }
+    }
+}
+
+export default class TestReload extends Node {
+    _ready(): void {
+        beginAsyncTest();
+        runReloadTest()
+            .catch((error) => reportTestFailure("reload", error))
+            .finally(() => endAsyncTest());
+    }
+}