[gjs/esm/dynamic-imports: 190/192] Refactor for static changes.

[Evan Welsh <ewlsh src gnome org>]

commit a2cb614898bded22fd82aa1f7793993691e69eb3
Author: Evan Welsh <contact evanwelsh com>
Date:   Fri Feb 5 20:16:30 2021 -0800

    Refactor for static changes.

 gjs/global.cpp                     |   1 -
 gjs/internal.cpp                   |  22 -------
 gjs/internal.h                     |   2 -
 gjs/module.cpp                     |  18 +++--
 modules/internal/internalLoader.js |  69 --------------------
 modules/internal/loader.js         | 130 +++++++++++++++++++++++--------------
 6 files changed, 91 insertions(+), 151 deletions(-)
---
diff --git a/gjs/global.cpp b/gjs/global.cpp
index dfe05133..a1a25ddf 100644
--- a/gjs/global.cpp
+++ b/gjs/global.cpp
@@ -279,7 +279,6 @@ class GjsInternalGlobal : GjsBaseGlobal {
               gjs_internal_resolve_relative_resource_or_file, 2, 0),
         JS_FN("setGlobalModuleLoader", gjs_internal_set_global_module_loader, 2,
               0),
-        JS_FN("setModuleDynamicImportHook", SetModuleDynamicImportHook, 2, 0),
         JS_FN("setModulePrivate", gjs_internal_set_module_private, 2, 0),
         JS_FN("uriExists", gjs_internal_uri_exists, 1, 0),
         JS_FS_END};
diff --git a/gjs/internal.cpp b/gjs/internal.cpp
index ccbfc1ea..61f400e5 100644
--- a/gjs/internal.cpp
+++ b/gjs/internal.cpp
@@ -106,28 +106,6 @@ bool gjs_load_internal_module(JSContext* cx, const char* identifier) {
     return true;
 }
 
-bool SetModuleDynamicImportHook(JSContext* cx, unsigned argc, JS::Value* vp) {
-    JS::CallArgs args = CallArgsFromVp(argc, vp);
-    if (!args.requireAtLeast(cx, "setModuleDynamicImportHook", 2)) {
-        return false;
-    }
-
-    JS::RootedValue gv(cx, args[0]);
-    JS::RootedValue mv(cx, args[1]);
-
-    g_assert(gv.isObject());
-
-    // The hook is stored in the internal global.
-    JS::RootedObject global(cx, &gv.toObject());
-
-    // The dynamic hook is stored in the internal global.
-
-    gjs_set_global_slot(global, GjsGlobalSlot::DYNAMIC_IMPORT_HOOK, mv);
-
-    args.rval().setUndefined();
-    return true;
-}
-
 bool FinishDynamicModuleImport(JSContext* cx, unsigned argc, JS::Value* vp) {
     JS::CallArgs args = CallArgsFromVp(argc, vp);
 
diff --git a/gjs/internal.h b/gjs/internal.h
index bf742652..6b9d525a 100644
--- a/gjs/internal.h
+++ b/gjs/internal.h
@@ -41,8 +41,6 @@ bool gjs_internal_resolve_relative_resource_or_file(JSContext* cx,
                                                     JS::Value* vp);
 
 bool InitAndEval(JSContext* cx, unsigned argc, JS::Value* vp);
-bool SetModuleDynamicImportHook(JSContext* cx, unsigned argc, JS::Value* vp);
-
 bool FinishDynamicModuleImport(JSContext* cx, unsigned argc, JS::Value* vp);
 
 GJS_JSAPI_RETURN_CONVENTION
diff --git a/gjs/module.cpp b/gjs/module.cpp
index bf5a41c6..71e654da 100644
--- a/gjs/module.cpp
+++ b/gjs/module.cpp
@@ -479,19 +479,17 @@ bool gjs_dynamic_module_resolve(JSContext* cx,
                                 JS::Handle<JS::Value> aReferencingPrivate,
                                 JS::Handle<JSString*> aSpecifier,
                                 JS::Handle<JSObject*> aPromise) {
-    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.");
-
-    GjsContextPrivate* gjs_cx = GjsContextPrivate::from_cx(cx);
+    g_assert(gjs_global_is_type(cx, GjsGlobalType::DEFAULT) &&
+             "gjs_dynamic_module_resolve can only be called from the default "
+             "global.");
 
     JS::RootedObject global(cx, JS::CurrentGlobalOrNull(cx));
-
     JSAutoRealm ar(cx, global);
 
-    JS::RootedValue hookValue(
-        cx, gjs_get_global_slot(global, GjsGlobalSlot::DYNAMIC_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<3> args(cx);
     args[0].set(aReferencingPrivate);
@@ -499,5 +497,5 @@ bool gjs_dynamic_module_resolve(JSContext* cx,
     args[2].setObject(*aPromise);
 
     JS::RootedValue result(cx);
-    return JS_CallFunctionValue(cx, nullptr, hookValue, args, &result);
+    return JS::Call(cx, loader, "moduleDynamicImportHook", args, &result);
 }
diff --git a/modules/internal/internalLoader.js b/modules/internal/internalLoader.js
index bd9933e6..09acc16c 100644
--- a/modules/internal/internalLoader.js
+++ b/modules/internal/internalLoader.js
@@ -84,21 +84,6 @@ export class InternalModuleLoader {
 
         return null;
     }
-    
-    /**
-     * Loads a file or resource URI synchronously
-     *
-     * @param {Uri} uri the file or resource URI to load
-     * @returns {Promise<[string] | [string, boolean] | null>}
-     */
-    async loadURIAsync(uri) {
-        if (uri.scheme === 'file' || uri.scheme === 'resource') {
-            const result = await loadResourceOrFileAsync(uri.uri);
-            return [result];
-        }
-
-        return null;
-    }
 
     /**
      * Resolves an import specifier given an optional parent importer.
@@ -206,60 +191,6 @@ export class InternalModuleLoader {
         return null;
     }
 
-    /**
-     * @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
-     *
-     * @returns {Promise<import("../types").Module | null>}
-     */
-    async resolveModuleAsync(specifier, parentURI) {
-        const registry = getRegistry(this.global);
-
-
-        // Check if the module has already been loaded
-
-        let module = registry.get(specifier);
-
-        if (module)
-            return module;
-
-
-        // 1) Resolve path and URI-based imports.
-
-        const uri = this.resolveSpecifier(specifier, parentURI);
-
-        if (uri) {
-            module = registry.get(uri.uri);
-            //
-            // Check if module is already loaded (relative handling)
-            if (module)
-                return module;
-
-
-            const result = await this.loadURIAsync(uri);
-
-            if (!result)
-                return null;
-
-
-            const [text, internal = false] = result;
-
-            const esmodule = new ESModule(uri.uri, uri.uri, internal);
-
-            const compiled = this.compileModule(esmodule, text);
-
-            if (!compiled)
-                throw new ImportError(`Failed to register module: ${uri}`);
-
-
-            registry.set(uri.uri, compiled);
-            return compiled;
-        }
-
-        return null;
-    }
-
-
     moduleResolveHook(importingModulePriv, specifier) {
         const resolved = this.resolveModule(specifier, importingModulePriv.uri ?? null);
         if (!resolved)
diff --git a/modules/internal/loader.js b/modules/internal/loader.js
index dad19b58..5f112e84 100644
--- a/modules/internal/loader.js
+++ b/modules/internal/loader.js
@@ -80,20 +80,23 @@ class ModuleLoader extends InternalModuleLoader {
         throw new ImportError(`Unable to load module from invalid URI: ${uri.uri}`);
     }
 
+    moduleDynamicImportHook(module, specifier, promise) {
+        this.resolveModuleAsync(specifier, module.uri).then((m) => {
+            initAndEval(m);
+    
+            finishDynamicModuleImport(module, specifier, promise);
+        }).catch(err => {
+            throw err;
+        });
+    }
+
     /**
-     * Resolves a module import with optional handling for relative imports.
-     * Overrides InternalModuleLoader.moduleResolveHook
+     * Resolves specifiers like 'system'.
      *
-     * @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
      * @returns {import("../types").Module}
      */
-    moduleResolveHook(importingModulePriv, specifier) {
-        const module = this.resolveModule(specifier, importingModulePriv.uri);
-        if (module)
-            return module;
-
+    resolveBareSpecifier(specifier) {
         // 2) Resolve internal imports.
 
         const uri = this.buildInternalURIs(specifier).find(uriExists);
@@ -118,51 +121,89 @@ class ModuleLoader extends InternalModuleLoader {
         return compiled;
     }
 
-      /**
-     * Resolves a module import with optional handling for relative imports asynchronously.
+    /**
+     * Resolves a module import with optional handling for relative imports.
+     * 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 {Promise<import("../types").Module>}
+     * @returns {import("../types").Module}
      */
-    async resolveModuleAsync(specifier, moduleURI) {
-        const module = await super.resolveModuleAsync(specifier, moduleURI);
-
+    moduleResolveHook(importingModulePriv, specifier) {
+        const module = this.resolveModule(specifier, importingModulePriv.uri);
         if (module)
             return module;
 
+        return this.resolveBareSpecifier(specifier);
+    }
 
-        // 2) Resolve internal imports.
+    /**
+     * Loads a file or resource URI asynchronously
+     *
+     * @param {Uri} uri the file or resource URI to load
+     * @returns {Promise<[string] | [string, boolean] | null>}
+     */
+    async loadURIAsync(uri) {
+        if (uri.scheme) {
+            const loader = this.schemeHandlers.get(uri.scheme);
 
-        const uri = this.buildInternalURIs(specifier).find(u => {
-            let file = Gio.File.new_for_uri(u);
+            if (loader)
+                return loader.loadAsync(uri);
+        }
 
-            return file && file.query_exists(null);
-        });
+        if (uri.scheme === 'file' || uri.scheme === 'resource') {
+            const result = await loadResourceOrFileAsync(uri.uri);
+            return [result];
+        }
 
-        if (!uri)
-            throw new ImportError(`Attempted to load unregistered global module: ${specifier}`);
+        return null;
+    }
 
-        const parsed = parseURI(uri);
+    /**
+     * Resolves a module import with optional handling for relative imports asynchronously.
+     *
+     * @param {string} specifier the module specifier to resolve for an import
+     * @param {string | null} importingModuleURI the importing module's URI or null if importing from the 
entry point
+     * @returns {Promise<import("../types").Module>}
+     */
+    async resolveModuleAsync(specifier, importingModuleURI) {
+        const registry = getRegistry(this.global);
 
-        if (parsed.scheme !== 'file' && parsed.scheme !== 'resource')
-            throw new ImportError('Only file:// and resource:// URIs are currently supported.');
+        // Check if the module has already been loaded
+        let module = registry.get(specifier);
+        if (module)
+            return module;
 
-        const text = loadResourceOrFile(parsed.uri);
 
-        const priv = new ESModule(specifier, uri, true);
+        // 1) Resolve path and URI-based imports.
+        const uri = this.resolveSpecifier(specifier, importingModuleURI);
+        if (uri) {
+            module = registry.get(uri.uri);
 
-        const compiled = this.compileModule(priv, text);
+            // Check if module is already loaded (relative handling)
+            if (module)
+                return module;
 
-        if (!compiled)
-            throw new ImportError(`Failed to register module: ${uri}`);
+            const result = await this.loadURIAsync(uri);
+            if (!result)
+                return null;
 
-        const registry = getRegistry(this.global);
+            const [text, internal = false] = result;
 
-        if (!registry.has(specifier))
-            registry.set(specifier, compiled);
+            const priv = new ModulePrivate(uri.uri, uri.uri, internal);
+            const compiled = this.compileModule(priv, text);
 
-        return compiled;
+            if (!compiled)
+                throw new ImportError(`Failed to register module: ${uri}`);
+
+            registry.set(uri.uri, compiled);
+            return compiled;
+        }
+
+        // 2) Resolve internal imports.
+
+        return this.resolveBareSpecifier(specifier);
     }
 }
 
@@ -209,16 +250,11 @@ moduleLoader.registerScheme('gi', {
 
         return [generateGIModule(namespace, version), true];
     },
-});
-
-setModuleDynamicImportHook(moduleGlobalThis, (module, specifier, promise) => {
-    moduleLoader.resolveModuleAsync(specifier, module.uri).then((m) => {
-        initAndEval(m);
-
-        finishDynamicModuleImport(module, specifier, promise);
-    }).catch(err => {
-        debug(err);
-        debug(err.stack);
-        throw new Error(`Dynamic module import failed: ${err}`);
-    });
+    /**
+     * @param {import("./internalLoader.js").Uri} uri the URI to load asynchronously
+     */
+    loadAsync(uri) {
+        // gi: only does string manipulation, so it is safe to use the same code for sync and async.
+        return this.load(uri);
+    }
 });