search

rscarson / rustyscript

Effortless JS integration for rust
MIT License
168 stars 18 forks source link

Allow Module::load to accept AsRef<Path> #188

Closed martsokha closed 2 months ago

martsokha commented 2 months ago

Is your feature request related to a problem? Please describe. It allows to avoid PathBuf::to_str and the subsequent .unwrap() in my use case. Idk if the change of the Module::filename to the OsString works.

Describe the solution you'd like

    pub fn load(filename: impl AsRef<Path>) -> Result<Self, std::io::Error> {
        let contents = read_to_string(filename)?;
        Ok(Self::new(filename, &contents))
    }

Describe alternatives you've considered Nothing.

Additional context Nothing.

rscarson commented 2 months ago

The reason it currently isn't supported is because deno_core doesn't accept it upstream so Module could accept it, but then the runtime may be unable to load them.

It seems wise to move that limitation as far up in the call chain as possible

rscarson commented 2 months ago

It actually seems like it may no longer be the case

I will look into this further

rscarson commented 2 months ago

Does in fact seem to work!

I have opened a PR on deno_core to fix resolve_path upstream, but have it working on my end already

Since it does require changing the signature of Module::filename incompatibly I will go from 0.7 to 0.8 for this change