Improve module path handling

[playXE]

Current implementation simply stores Rc<String> inside each bytecode block and this module path is not always properly updated.

[andrieshiemstra]

Does starlight also have something like a custom module_path_resolve method to resolve the absolute path of a module?

and/or an actual load_module(path) method?

[playXE]

No for both. I haven't gotten to this and that is why almost all test262 module tests fail now. :( I guess for test262 module tests some special handling is required.

[andrieshiemstra]

well, other engines like quickjs and spidermonkey leave module resolution up to the implementor of the engine and just provide two methods for loading and resolution

in quickjs it's JS_SetModuleLoaderFunc which acepts two functions

    ctx: *mut q::JSContext,
    module_base_name: *const ::std::os::raw::c_char,
    module_name: *const ::std::os::raw::c_char,
    _opaque: *mut ::std::os::raw::c_void,
 ) -> *mut ::std::os::raw::c_char

and

* fn js_module_loader(
    ctx: *mut q::JSContext,
    module_name_raw: *const ::std::os::raw::c_char,
    _opaque: *mut ::std::os::raw::c_void,
) -> *mut q::JSModuleDef 

the first translates the relative module name or path to an absolute path

the second actually loads the module (either from script or by instantiating a native module)

Spidermonkey pretty much does the same with SetModuleResolveHook() and SetModuleDynamicImportHook()

Maybe for starlight we could implement something more fancy like a ModuleLoader trait? for quickjs i built this (https://github.com/HiRoFa/quickjs_es_runtime/blob/master/src/quickjsruntime.rs#L25) but i have to differentiate between native and script based modules, i don;t think we'd need that in starlight? maybe something like this?

pub trait ModuleLoader {
    fn normalize_path(&self, vm: &mut Runtime, ref_path: &str, path: &str) -> Option<String>;
    fn load_module(
        &self,
        vm: &mut Runtime
        absolute_path: &str,
    ) -> Result<JsValue, JsValue>;
}

[playXE]

ModuleLoader trait sounds nice but where do we store currently running module path so ref_path is always up to date?

[andrieshiemstra]

most engines simply add that info to a CallFrame, if the current frame does not have the info, check the parent frame etc.

this is the quickjs impl, i know rhino pretty much does the same, i haven't checked spidermonkey

(from https://raw.githubusercontent.com/theduke/quickjs-rs/master/libquickjs-sys/embed/quickjs/quickjs.c)

/* return JS_ATOM_NULL if the name cannot be found. Only works with
   not striped bytecode functions. */
JSAtom JS_GetScriptOrModuleName(JSContext *ctx, int n_stack_levels)
{
    JSStackFrame *sf;
    JSFunctionBytecode *b;
    JSObject *p;
    /* XXX: currently we just use the filename of the englobing
       function. It does not work for eval(). Need to add a
       ScriptOrModule info in JSFunctionBytecode */
    sf = ctx->rt->current_stack_frame;
    if (!sf)
        return JS_ATOM_NULL;
    while (n_stack_levels-- > 0) {
        sf = sf->prev_frame;
        if (!sf)
            return JS_ATOM_NULL;
    }
    if (JS_VALUE_GET_TAG(sf->cur_func) != JS_TAG_OBJECT)
        return JS_ATOM_NULL;
    p = JS_VALUE_GET_OBJ(sf->cur_func);
    if (!js_class_has_bytecode(p->class_id))
        return JS_ATOM_NULL;
    b = p->u.func.function_bytecode;
    if (!b->has_debug)
        return JS_ATOM_NULL;
    return JS_DupAtom(ctx, b->debug.filename);
}

So when import() is called you just call vm.getCurrentScriptOrModuleName (whioch does pretty much the same as the above C code) and that's your ref_path

When you eval a module or a script you add the info to the current CallFrame....

I'd be glad to help out with this but i'm kinda in the middle of playing with Promises first (i have some question about starting async (not parallel) jobs for you later :) )

[andrieshiemstra]

Hmm i see your CallFrame already has a CodeBlock Option... why does ther Rc inside it need updating?

[playXE]

I thought about Rc a little bit more and it seems that's not a bad option actually. And obtaining module path from current call frame does not sound bad either.

i have some question about starting async (not parallel) jobs for you later :) )

Generators and async_func_resume in src/vm/function.rs is direct rewrite of QuickJS code so I assume you could just rewrite how QuickJS implement Async fns.