SQUASH: store module loader instead of hook functionsesm/static-imports

this way, the entire set of module loading behaviours is associated with
one object.

8 files changed, 77 insertions, 130 deletions

diff --git a/gjs/global.cpp b/gjs/global.cpp
index aeb11ad7..6e0dac3b 100644
--- a/gjs/global.cpp
+++ b/gjs/global.cpp
@@ -273,10 +273,9 @@ class GjsInternalGlobal : GjsBaseGlobal {
         JS_FN("parseURI", gjs_internal_parse_uri, 1, 0),
         JS_FN("resolveRelativeResourceOrFile",
               gjs_internal_resolve_relative_resource_or_file, 2, 0),
-        JS_FN("setModuleLoadHook", gjs_internal_global_set_module_hook, 3, 0),
+        JS_FN("setGlobalModuleLoader", gjs_internal_set_global_module_loader, 2,
+              0),
         JS_FN("setModulePrivate", gjs_internal_set_module_private, 2, 0),
-        JS_FN("setModuleResolveHook",
-              gjs_internal_global_set_module_resolve_hook, 2, 0),
         JS_FN("uriExists", gjs_internal_uri_exists, 1, 0),
         JS_FS_END};
 
diff --git a/gjs/global.h b/gjs/global.h
index f7c39ad3..569a8ce1 100644
--- a/gjs/global.h
+++ b/gjs/global.h
@@ -37,10 +37,8 @@ enum class GjsDebuggerGlobalSlot : uint32_t {
 
 enum class GjsGlobalSlot : uint32_t {
     IMPORTS = static_cast<uint32_t>(GjsBaseGlobalSlot::LAST),
-    // Stores a function which resolves imports
-    IMPORT_HOOK,
-    // Stores a function which creates new module objects
-    MODULE_HOOK,
+    // Stores an object with methods to resolve and load modules
+    MODULE_LOADER,
     // Stores the module registry (a Map object)
     MODULE_REGISTRY,
     NATIVE_REGISTRY,
diff --git a/gjs/internal.cpp b/gjs/internal.cpp
index 388f2406..779e0864 100644
--- a/gjs/internal.cpp
+++ b/gjs/internal.cpp
@@ -106,68 +106,30 @@ bool gjs_load_internal_module(JSContext* cx, const char* identifier) {
 }
 
 /**
- * Asserts the correct arguments for a hook setting function.
+ * gjs_internal_set_global_module_loader:
  *
- * Asserts: (arg0: object, arg1: Function) => void
- */
-static void set_module_hook(JS::CallArgs args, GjsGlobalSlot slot) {
-    JS::Value v_global = args[0];
-    JS::Value v_hook = args[1];
-
-    g_assert(v_global.isObject());
-    g_assert(v_hook.isObject());
-
-    g_assert(JS::IsCallable(&v_hook.toObject()));
-    gjs_set_global_slot(&v_global.toObject(), slot, v_hook);
-
-    args.rval().setUndefined();
-}
-
-/**
- * gjs_internal_global_set_module_hook:
- *
- * @brief Sets the MODULE_HOOK slot of the passed global object.
- * Asserts that the second argument must be callable (e.g. Function)
- * The passed callable is called by gjs_module_load.
- *
- * @example (in JavaScript)
- * setModuleLoadHook(globalThis, (id, uri) => {
- *   id // the module's identifier
- *   uri // the URI to load from
- * });
+ * @brief Sets the MODULE_LOADER slot of the passed global object.
+ * The second argument should be an instance of ModuleLoader or
+ * InternalModuleLoader. Its moduleResolveHook and moduleLoadHook properties
+ * will be called.
  *
  * @returns guaranteed to return true or assert.
  */
-bool gjs_internal_global_set_module_hook([[maybe_unused]] JSContext* cx,
-                                         unsigned argc, JS::Value* vp) {
+bool gjs_internal_set_global_module_loader(JSContext*, unsigned argc,
+                                           JS::Value* vp) {
     JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
-    g_assert(args.length() == 2 && "setModuleLoadHook takes 2 arguments");
+    g_assert(args.length() == 2 && "setGlobalModuleLoader takes 2 arguments");
 
-    set_module_hook(args, GjsGlobalSlot::MODULE_HOOK);
-    return true;
-}
+    JS::Value v_global = args[0];
+    JS::Value v_loader = args[1];
 
-/**
- * gjs_internal_global_set_module_resolve_hook:
- *
- * @brief Sets the IMPORT_HOOK slot of the passed global object.
- * Asserts that the second argument must be callable (e.g. Function)
- * The passed callable is called by gjs_module_resolve.
- *
- * @example (in JavaScript)
- * setModuleResolveHook(globalThis, (module, specifier) => {
- *   module // the importing module object
- *   specifier // the import specifier
- * });
- *
- * @returns guaranteed to return true or assert.
- */
-bool gjs_internal_global_set_module_resolve_hook([[maybe_unused]] JSContext* cx,
-                                                 unsigned argc, JS::Value* vp) {
-    JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
-    g_assert(args.length() == 2 && "setModuleResolveHook takes 2 arguments");
+    g_assert(v_global.isObject() && "first argument must be an object");
+    g_assert(v_loader.isObject() && "second argument must be an object");
 
-    set_module_hook(args, GjsGlobalSlot::IMPORT_HOOK);
+    gjs_set_global_slot(&v_global.toObject(), GjsGlobalSlot::MODULE_LOADER,
+                        v_loader);
+
+    args.rval().setUndefined();
     return true;
 }
 
diff --git a/gjs/internal.h b/gjs/internal.h
index 888b9033..212e5458 100644
--- a/gjs/internal.h
+++ b/gjs/internal.h
@@ -25,18 +25,14 @@ GJS_JSAPI_RETURN_CONVENTION
 bool gjs_internal_get_registry(JSContext* cx, unsigned argc, JS::Value* vp);
 
 GJS_JSAPI_RETURN_CONVENTION
-bool gjs_internal_global_set_module_hook(JSContext* cx, unsigned argc,
-                                         JS::Value* vp);
+bool gjs_internal_set_global_module_loader(JSContext* cx, unsigned argc,
+                                           JS::Value* vp);
 
 GJS_JSAPI_RETURN_CONVENTION
 bool gjs_internal_set_module_private(JSContext* cx, unsigned argc,
                                      JS::Value* vp);
 
 GJS_JSAPI_RETURN_CONVENTION
-bool gjs_internal_global_set_module_resolve_hook(JSContext* cx, unsigned argc,
-                                                 JS::Value* vp);
-
-GJS_JSAPI_RETURN_CONVENTION
 bool gjs_internal_parse_uri(JSContext* cx, unsigned argc, JS::Value* vp);
 
 GJS_JSAPI_RETURN_CONVENTION
diff --git a/gjs/module.cpp b/gjs/module.cpp
index 37fdd3af..0a9effab 100644
--- a/gjs/module.cpp
+++ b/gjs/module.cpp
@@ -319,8 +319,10 @@ JSObject* gjs_module_load(JSContext* cx, const char* identifier,
              "globals.");
 
     JS::RootedObject global(cx, JS::CurrentGlobalOrNull(cx));
-    JS::RootedValue hook(
-        cx, gjs_get_global_slot(global, GjsGlobalSlot::MODULE_HOOK));
+    JS::RootedValue v_loader(
+        cx, gjs_get_global_slot(global, GjsGlobalSlot::MODULE_LOADER));
+    g_assert(v_loader.isObject());
+    JS::RootedObject loader(cx, &v_loader.toObject());
 
     JS::ConstUTF8CharsZ id_chars(identifier, strlen(identifier));
     JS::ConstUTF8CharsZ uri_chars(file_uri, strlen(file_uri));
@@ -336,7 +338,7 @@ JSObject* gjs_module_load(JSContext* cx, const char* identifier,
     args[1].setString(uri);
 
     JS::RootedValue result(cx);
-    if (!JS_CallFunctionValue(cx, nullptr, hook, args, &result))
+    if (!JS::Call(cx, loader, "moduleLoadHook", args, &result))
         return nullptr;
 
     g_assert(result.isObject() && "Module hook failed to return an object!");
@@ -439,29 +441,34 @@ bool gjs_populate_module_meta(JSContext* cx, JS::HandleValue private_ref,
  *
  * Hook SpiderMonkey calls to resolve import specifiers.
  *
- * @param priv the private value of the #Module object initiating the import, or
- *   undefined.
+ * @param importingModulePriv the private value of the #Module object initiating
+ *   the import.
  * @param specifier the import specifier to resolve
  *
  * @returns whether an error occurred while resolving the specifier.
  */
-JSObject* gjs_module_resolve(JSContext* cx, JS::HandleValue priv,
+JSObject* gjs_module_resolve(JSContext* cx, JS::HandleValue importingModulePriv,
                              JS::HandleString specifier) {
     g_assert((gjs_global_is_type(cx, GjsGlobalType::DEFAULT) ||
               gjs_global_is_type(cx, GjsGlobalType::INTERNAL)) &&
              "gjs_module_resolve can only be called from module-enabled "
              "globals.");
+    g_assert(importingModulePriv.isObject() &&
+             "the importing module can't be null, don't add import to the "
+             "bootstrap script");
 
     JS::RootedObject global(cx, JS::CurrentGlobalOrNull(cx));
-    JS::RootedValue v_hook(
-        cx, gjs_get_global_slot(global, GjsGlobalSlot::IMPORT_HOOK));
+    JS::RootedValue v_loader(
+        cx, gjs_get_global_slot(global, GjsGlobalSlot::MODULE_LOADER));
+    g_assert(v_loader.isObject());
+    JS::RootedObject loader(cx, &v_loader.toObject());
 
     JS::RootedValueArray<2> args(cx);
-    args[0].set(priv);
+    args[0].set(importingModulePriv);
     args[1].setString(specifier);
 
     JS::RootedValue result(cx);
-    if (!JS_CallFunctionValue(cx, nullptr, v_hook, args, &result))
+    if (!JS::Call(cx, loader, "moduleResolveHook", args, &result))
         return nullptr;
 
     g_assert(result.isObject() && "resolve hook failed to return an object!");
diff --git a/modules/internal/.eslintrc.yml b/modules/internal/.eslintrc.yml
index 6bcff33c..b06bc212 100644
--- a/modules/internal/.eslintrc.yml
+++ b/modules/internal/.eslintrc.yml
@@ -22,7 +22,6 @@ globals:
   parseURI: readonly
   uriExists: readonly
   resolveRelativeResourceOrFile: readonly
-  setModuleResolveHook: readonly
-  setModuleLoadHook: readonly
+  setGlobalModuleLoader: readonly
   setModulePrivate: readonly
   getRegistry: readonly
diff --git a/modules/internal/internalLoader.js b/modules/internal/internalLoader.js
index e59e5c5e..09acc16c 100644
--- a/modules/internal/internalLoader.js
+++ b/modules/internal/internalLoader.js
@@ -117,15 +117,16 @@ export class InternalModuleLoader {
      * the parentURI isn't valid.
      *
      * @param {string} relativePath the relative path to resolve against the base URI
-     * @param {string} parentURI the parent URI
+     * @param {string} importingModuleURI the URI of the module triggering this
+     *   resolve
      * @returns {Uri}
      */
-    resolveRelativePath(relativePath, parentURI) {
+    resolveRelativePath(relativePath, importingModuleURI) {
         // Ensure the parent URI is valid.
-        parseURI(parentURI);
+        parseURI(importingModuleURI);
 
         // Handle relative imports from URI-based modules.
-        const relativeURI = resolveRelativeResourceOrFile(parentURI, relativePath);
+        const relativeURI = resolveRelativeResourceOrFile(importingModuleURI, relativePath);
         if (!relativeURI)
             throw new ImportError('File does not have a valid parent!');
         return parseURI(relativeURI);
@@ -149,11 +150,12 @@ export class InternalModuleLoader {
 
     /**
      * @param {string} specifier the specifier (e.g. relative path, root package) to resolve
-     * @param {string | null} parentURI the URI of the module triggering this resolve
+     * @param {string | null} importingModuleURI the URI of the module
+     *   triggering this resolve
      *
      * @returns {import("../types").Module | null}
      */
-    resolveModule(specifier, parentURI) {
+    resolveModule(specifier, importingModuleURI) {
         const registry = getRegistry(this.global);
 
         // Check if the module has already been loaded
@@ -162,7 +164,7 @@ export class InternalModuleLoader {
             return module;
 
         // 1) Resolve path and URI-based imports.
-        const uri = this.resolveSpecifier(specifier, parentURI);
+        const uri = this.resolveSpecifier(specifier, importingModuleURI);
         if (uri) {
             module = registry.get(uri.uri);
 
@@ -188,30 +190,33 @@ export class InternalModuleLoader {
 
         return null;
     }
-}
 
-export const internalModuleLoader = new InternalModuleLoader(globalThis);
+    moduleResolveHook(importingModulePriv, specifier) {
+        const resolved = this.resolveModule(specifier, importingModulePriv.uri ?? null);
+        if (!resolved)
+            throw new ImportError(`Module not found: ${specifier}`);
 
-setModuleResolveHook(globalThis, (priv, specifier) => {
-    const resolved = internalModuleLoader.resolveModule(specifier, priv.uri ?? null);
-    if (!resolved)
-        throw new ImportError(`Module not found: ${specifier}`);
+        return resolved;
+    }
 
-    return resolved;
-});
+    moduleLoadHook(id, uri) {
+        const priv = new ModulePrivate(id, uri);
 
-setModuleLoadHook(globalThis, (id, uri) => {
-    const priv = new ModulePrivate(id, uri);
+        const result = this.loadURI(parseURI(uri));
+        // result can only be null if `this` is InternalModuleLoader. If `this`
+        // is ModuleLoader, then loadURI() will have thrown
+        if (!result)
+            throw new ImportError(`URI not found: ${uri}`);
 
-    const result = internalModuleLoader.loadURI(parseURI(uri));
-    if (!result)
-        throw new ImportError(`URI not found: ${uri}`);
+        const [text] = result;
+        const compiled = this.compileModule(priv, text);
 
-    const [text] = result;
-    const compiled = internalModuleLoader.compileModule(priv, text);
+        const registry = getRegistry(this.global);
+        registry.set(id, compiled);
 
-    const registry = getRegistry(globalThis);
-    registry.set(uri, compiled);
+        return compiled;
+    }
+}
 
-    return compiled;
-});
+export const internalModuleLoader = new InternalModuleLoader(globalThis);
+setGlobalModuleLoader(globalThis, internalModuleLoader);
diff --git a/modules/internal/loader.js b/modules/internal/loader.js
index 2c13d7a5..a44e8127 100644
--- a/modules/internal/loader.js
+++ b/modules/internal/loader.js
@@ -82,14 +82,15 @@ class ModuleLoader extends InternalModuleLoader {
 
     /**
      * Resolves a module import with optional handling for relative imports.
-     * Overrides InternalModuleLoader.resolveModule
+     * Overrides InternalModuleLoader.moduleResolveHook
      *
+     * @param {import("./internalLoader.js").ModulePrivate} importingModulePriv
+     *   the private object of the module initiating the import
      * @param {string} specifier the module specifier to resolve for an import
-     * @param {string | null} moduleURI the importing module's URI or null if importing from the entry point
      * @returns {import("../types").Module}
      */
-    resolveModule(specifier, moduleURI) {
-        const module = super.resolveModule(specifier, moduleURI);
+    moduleResolveHook(importingModulePriv, specifier) {
+        const module = this.resolveModule(specifier, importingModulePriv.uri);
         if (module)
             return module;
 
@@ -119,6 +120,7 @@ class ModuleLoader extends InternalModuleLoader {
 }
 
 const moduleLoader = new ModuleLoader(moduleGlobalThis);
+setGlobalModuleLoader(moduleGlobalThis, moduleLoader);
 
 const giVersionMap = new Map([
     ['GLib', '2.0'],
@@ -161,24 +163,3 @@ moduleLoader.registerScheme('gi', {
         return [generateGIModule(namespace, version), true];
     },
 });
-
-/**
- * @param {string} id
- * @param {string} uri
- */
-setModuleLoadHook(moduleGlobalThis, (id, uri) => {
-    const priv = new ModulePrivate(id, uri);
-
-    const [text] = moduleLoader.loadURI(parseURI(uri));
-    const compiled = moduleLoader.compileModule(priv, text);
-
-    const registry = getRegistry(moduleGlobalThis);
-
-    registry.set(id, compiled);
-
-    return compiled;
-});
-
-setModuleResolveHook(moduleGlobalThis, (priv, specifier) => {
-    return moduleLoader.resolveModule(specifier, priv.uri);
-});